[GitHub] [airflow] uranusjr commented on a change in pull request #20443: Add Listener Plugin API that tracks TaskInstance state changes

[GitBox <gi...@apache.org>]

uranusjr commented on a change in pull request #20443:
URL: https://github.com/apache/airflow/pull/20443#discussion_r773555651



##########
File path: airflow/utils/orm_event_handlers.py
##########
@@ -86,3 +86,7 @@ def after_cursor_execute(conn, cursor, statement, parameters, context, executema
                 stack_info,
                 statement.replace("\n", " "),
             )
+
+    if conf.getboolean('core', 'execute_listeners_on_scheduler', fallback=False):
+        # On import, register sqlalchemy event handlers which call airflow.listeners
+        import airflow.listeners.events  # noqa

Review comment:
       IMO this is generally bad practice. I’d much prefer the import to not have side effect, and an explicit init function call registers the handlers. This would make tests much easier to write as well since you can do the init in a module-scoped pytest fixture instead.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org