mistercrunch commented on a change in pull request #13346:
URL: https://github.com/apache/superset/pull/13346#discussion_r583844440



##########
File path: superset/databases/commands/create.py
##########
@@ -69,8 +64,9 @@ def run(self) -> Model:
             security_manager.add_permission_view_menu("database_access", 
database.perm)
             db.session.commit()
         except DAOCreateFailedError as ex:
-            logger.exception(ex.exception)
-            raise DatabaseCreateFailedError()
+            with 
event_logger.log_context(action=f"db_creation_failed.{ex.exception}"):

Review comment:
       Quick clarification that `enrich_with_request_context ` would be false 
by default to protect backwards compatibity. But maybe function composition is 
better than a parameter here, I'm ok with with either pattern. 
   
   There's a need to really clarify/improve the method in `AbstractEventLogger` 
with good/clear names here between functions we can call, content manager(s) 
and decorator(s)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to