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

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

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



##########
File path: docs/apache-airflow/plugins.rst
##########
@@ -268,6 +285,7 @@ definitions in Airflow.
         operator_extra_links = [
             S3LogLink(),
         ]
+        listeners = [PrintingRunningListener]

Review comment:
       Generally, in pluggy, we need to register `plugin` that's a namespace that consists of possibly multiple hookimpls, not single functions. 
   By passing just a list of functions, we'd later need to construct "synthetic" namespace for it to register.
   
   I think requiring a class, or, possibly a python module, is a good idea - this is what pluggy docs present:
   
   - class https://pluggy.readthedocs.io/en/stable/index.html#a-toy-example
   - module https://pluggy.readthedocs.io/en/stable/index.html#the-plugin
   
   Now, it does not need to look like 
   ```python
           listeners = [PrintingRunningListener]
   ```
   
   We could require plugin authors to directly implement hookimpls in plugins, for example
   
   ```python
   class SomeListenerPlugin(AirflowPlugin):
       name = 'some-plugin'
       @hookimpl
       def on_task_instance_running(...):
           print("running")
   ``` 
   
   or, we could require it to add single pluggy plugin - as class or module - if for some reason we don't want them to implement multiple listener functions in single plugin.
   
   Note, this is separate discussion from https://github.com/apache/airflow/pull/20443#discussion_r778047132




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