From 253f80ab6d90d6c3c8e9337be2545779ead2c943 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 8 Mar 2022 17:31:19 -0800 Subject: [PATCH] chore: log multiple errors (#14064) * log all errors from db create * return unique set of errors * sort set for exceptions list * run black (cherry picked from commit c143b3712800d2d74c395525e39e7b87beb12c53) --- superset/commands/exceptions.py | 3 ++ superset/databases/commands/create.py | 5 ++- .../databases/commands_tests.py | 39 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/superset/commands/exceptions.py b/superset/commands/exceptions.py index 40c059765b6..8b4b717f31d 100644 --- a/superset/commands/exceptions.py +++ b/superset/commands/exceptions.py @@ -66,6 +66,9 @@ class CommandInvalidError(CommandException): def add_list(self, exceptions: List[ValidationError]) -> None: self._invalid_exceptions.extend(exceptions) + def get_list_classnames(self) -> List[str]: + return list(sorted({ex.__class__.__name__ for ex in self._invalid_exceptions})) + def normalized_messages(self) -> Dict[Any, Any]: errors: Dict[Any, Any] = {} for exception in self._invalid_exceptions: diff --git a/superset/databases/commands/create.py b/superset/databases/commands/create.py index ffcb018c067..e91ccec45c5 100644 --- a/superset/databases/commands/create.py +++ b/superset/databases/commands/create.py @@ -92,6 +92,9 @@ class CreateDatabaseCommand(BaseCommand): exception = DatabaseInvalidError() exception.add_list(exceptions) event_logger.log_with_context( - action=f"db_connection_failed.{exception.__class__.__name__}" + action="db_connection_failed.{}.{}".format( + exception.__class__.__name__, + ".".join(exception.get_list_classnames()), + ) ) raise exception diff --git a/tests/integration_tests/databases/commands_tests.py b/tests/integration_tests/databases/commands_tests.py index 199b08ce108..c90550ed699 100644 --- a/tests/integration_tests/databases/commands_tests.py +++ b/tests/integration_tests/databases/commands_tests.py @@ -26,7 +26,9 @@ from superset import db, event_logger, security_manager from superset.commands.exceptions import CommandInvalidError from superset.commands.importers.exceptions import IncorrectVersionError from superset.connectors.sqla.models import SqlaTable +from superset.databases.commands.create import CreateDatabaseCommand from superset.databases.commands.exceptions import ( + DatabaseInvalidError, DatabaseNotFoundError, DatabaseSecurityUnsafeError, DatabaseTestConnectionDriverError, @@ -64,6 +66,43 @@ from tests.integration_tests.fixtures.importexport import ( ) +class TestCreateDatabaseCommand(SupersetTestCase): + @mock.patch( + "superset.databases.commands.test_connection.event_logger.log_with_context" + ) + def test_create_duplicate_error(self, mock_logger): + example_db = get_example_database() + command = CreateDatabaseCommand( + security_manager.find_user("admin"), + {"database_name": example_db.database_name}, + ) + with pytest.raises(DatabaseInvalidError) as excinfo: + command.run() + assert str(excinfo.value) == ("Database parameters are invalid.") + # logger should list classnames of all errors + mock_logger.assert_called_with( + action="db_connection_failed." + "DatabaseInvalidError." + "DatabaseExistsValidationError." + "DatabaseRequiredFieldValidationError" + ) + + @mock.patch( + "superset.databases.commands.test_connection.event_logger.log_with_context" + ) + def test_multiple_error_logging(self, mock_logger): + command = CreateDatabaseCommand(security_manager.find_user("admin"), {}) + with pytest.raises(DatabaseInvalidError) as excinfo: + command.run() + assert str(excinfo.value) == ("Database parameters are invalid.") + # logger should list a unique set of errors with no duplicates + mock_logger.assert_called_with( + action="db_connection_failed." + "DatabaseInvalidError." + "DatabaseRequiredFieldValidationError" + ) + + class TestExportDatabasesCommand(SupersetTestCase): @skip("Flaky") @patch("superset.security.manager.g")