You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "amoghrajesh (via GitHub)" <gi...@apache.org> on 2025/12/29 12:38:44 UTC
[PR] Extract airflow listeners as a shared library [airflow]
amoghrajesh opened a new pull request, #59883:
URL: https://github.com/apache/airflow/pull/59883
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
## Why?
Simply stating, for client server separation.
The airflow.listeners module was used between airflow-core and task-sdk and is still needed that way. By extracting it to shared library, I intend to solve the problem of cross dependency between airflow core and task-sdk.
## What's done?
1. Created shared/listeners/ library containing:
- ListenerManager class (manages plugin registration and hook dispatching)
- Hook specifications (lifecycle, taskinstance, dagrun, asset, importerrors)
- hookimpl marker for implementing listeners
2. For both airflow-core and task-sdk now:
- Import shared listener infrastructure via symlinks (`_shared/listeners/`)
- Provide thin wrappers that call `integrate_listener_plugins()` to load Airflow plugins and register the listeners with airflow.
## Open Questions and Direction to proceed
1. Plugin Discovery on workers
- Current state even before this change: task SDK workers use `integrate_listener_plugins()` from airflow-core to discover listeners via the Airflow plugins.
- Proposal for thisis that we can create a lightweight plugin discovery mechanism for task-sdk which will use entry points (importlib.metadata) to discover provider listeners like the plugins code already does. The benefit of this is that it will allow us / providers to register listeners without full Airflow plugins needed in task sdk: https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/plugins_manager.py#L508-L515 (cc @potiuk)
2. What do providers do?
- My proposal is that they use the task sdk building blocks present in `sdk.listeners` to write plugins while we figure out (1) from above which registers those written listeners with airflow.
## TODO
- [ ] Update providers using listeners manager to consume from task sdk once 1, 2 qns are clarified from above
- [ ] Deprecation warning for `airflow.listeners.spec`? I think its ok to delete as-is since its an internal module
- [ ] Update documentation
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2668031581
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
Sure that sounds good as well
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667200580
##########
airflow-core/src/airflow/listeners/spec/asset.py:
##########
Review Comment:
I think I have it at a state now where I am mostly happy with it. Investigated all call sites of the specs and arrived at this conclusion:
| Spec | Location | Called from SDK? | Called from Core? |
|---------------|----------|------------------|-------------------|
| lifecycle | Shared | Yes | Yes |
| taskinstance | Shared | Yes | Yes |
| dagrun | Core | No | Yes |
| asset | Core | No | Yes |
| importerrors | Core | No | Yes |
The specs have been moved to the relevant sites now. For example, sdk only has lifecycle and taskinstance which it consumes from shared because its also used in core, a better example would probably be `asset` - only used in core, stays in core
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671341853
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
Or maybe use hook functions to set/unset this on load/unload? https://docs.pytest.org/en/7.1.x/how-to/writing_hook_functions.html
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667587980
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
Review Comment:
Unfortunately this is intentional because we use `DagMaker` in `create_task_instance` fixture. That uses task sdk to make dags and hence the sdk path is needed here.
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
+ listener_manager(full_listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
- listener_logs = [r for r in caplog.record_tuples if r[0] == "airflow.listeners.listener"]
- assert all(r[:-1] == ("airflow.listeners.listener", logging.DEBUG) for r in listener_logs)
+ listener_logs = [r for r in caplog.record_tuples if r[0] == "airflow.sdk._shared.listeners.listener"]
+ assert all(r[:-1] == ("airflow.sdk._shared.listeners.listener", logging.DEBUG) for r in listener_logs)
Review Comment:
Same as above
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667194938
##########
shared/listeners/src/airflow_shared/listeners/listener.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+import pluggy
+import structlog
+
+if TYPE_CHECKING:
+ from pluggy._hooks import _HookRelay
+
+log = structlog.get_logger(__name__)
+
+
+def _before_hookcall(hook_name, hook_impls, kwargs):
+ log.debug("Calling %r with %r", hook_name, kwargs)
+ log.debug("Hook impls: %s", hook_impls)
+
+
+def _after_hookcall(outcome, hook_name, hook_impls, kwargs):
+ log.debug("Result from %r: %s", hook_name, outcome.get_result())
+
+
+class ListenerManager:
+ """
+ Manage listener registration and provides hook property for calling them.
+
+ This class provides base infra for listener system. The consumers / components
+ wanting to register listeners should initialise its own ListenerManager and
+ register the hook specs relevant to that component using add_hookspecs.
+ """
+
+ def __init__(self):
+ self.pm = pluggy.PluginManager("airflow")
+ self.pm.add_hookcall_monitoring(_before_hookcall, _after_hookcall)
+
+ def add_hookspecs(self, spec_module) -> None:
+ """
+ Register hook specs from a module.
+
+ :param spec_module: A module containing functions decorated with @hookspec.
+ """
+ self.pm.add_hookspecs(spec_module)
Review Comment:
cc @potiuk WDYT?
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2668039256
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
Review Comment:
Umm yeah I guess, but since its tests, is there something you worry about?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2660959708
##########
task-sdk/src/airflow/sdk/listeners/__init__.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.sdk._shared.listeners import hookimpl
+
+__all__ = ["hookimpl"]
Review Comment:
Do we need this? You can probably argue Core does for compatibility, but SDK can just import directly from the canonical location, can it not?
I also kind of wonder if we should do some additional logic here to avoid having two `hookimpl` objects when both Core and SDK are available. Would it cause issues?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665926112
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
Review Comment:
Or maybe tests should be moved to sdk?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2653089623
##########
providers/openlineage/src/airflow/providers/openlineage/plugins/listener.py:
##########
@@ -28,9 +28,8 @@
from openlineage.client.serde import Serde
from airflow import settings
-from airflow.listeners import hookimpl
from airflow.models import DagRun, TaskInstance
-from airflow.providers.common.compat.sdk import Stats, timeout, timezone
+from airflow.providers.common.compat.sdk import Stats, hookimpl, timeout, timezone
Review Comment:
You need to bump the version required (via an indicator which will be used by Providers's release manager) otherwise this will break OL:
https://github.com/apache/airflow/blob/cf80ae19840f1d03e16323e7bca819550633db97/contributing-docs/13_airflow_dependencies_and_extras.rst?plain=1#L216-L223
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59883:
URL: https://github.com/apache/airflow/pull/59883#issuecomment-3717786325
Also - there are two remaining questions about tests -> I am not sure why we have task.sdk in airflow-core unit tests ?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2664149543
##########
shared/listeners/src/airflow_shared/listeners/listener.py:
##########
@@ -0,0 +1,77 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import logging
+from typing import TYPE_CHECKING
+
+import pluggy
+
+if TYPE_CHECKING:
+ from pluggy._hooks import _HookRelay
+
+log = logging.getLogger(__name__)
Review Comment:
Good call, let's do it. We would normally not do it otherwise.
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665920756
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
Review Comment:
Should it be `airflow._shared.listeners.listener` ?
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
+ listener_manager(full_listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
- listener_logs = [r for r in caplog.record_tuples if r[0] == "airflow.listeners.listener"]
- assert all(r[:-1] == ("airflow.listeners.listener", logging.DEBUG) for r in listener_logs)
+ listener_logs = [r for r in caplog.record_tuples if r[0] == "airflow.sdk._shared.listeners.listener"]
+ assert all(r[:-1] == ("airflow.sdk._shared.listeners.listener", logging.DEBUG) for r in listener_logs)
Review Comment:
same here?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2660949996
##########
shared/listeners/src/airflow_shared/listeners/listener.py:
##########
@@ -0,0 +1,77 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import logging
+from typing import TYPE_CHECKING
+
+import pluggy
+
+if TYPE_CHECKING:
+ from pluggy._hooks import _HookRelay
+
+log = logging.getLogger(__name__)
Review Comment:
Do we want to use this chance to switch to structlog here?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2664088189
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
We have a type checking block in here:
```python
if TYPE_CHECKING:
from airflow.serialization.definitions.assets import SerializedAsset, SerializedAssetAlias
```
which is an airflow-core block. There is no runtime impact here, but easing this with `Any` or removing typing doesn't seem right as providers implementing this could lose mypy warnings. I could also define a protocol in shared library but that would be an unwanted maintenance burden to keep this protocol and actual structure in sync.
cc @potiuk / @uranusjr do you have any thoughts on this? BTW, this is for all files in spec, I am just pointing assets spec.
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665948983
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
Same as in the other libraries -> I think you should inject specs to the shared listener from the "user" - so in airflow core you should keep the specs only used in the core,m and you should pass the list of specs when initializing listener manager.
```
self.pm.add_hookspecs(lifecycle)
self.pm.add_hookspecs(dagrun)
self.pm.add_hookspecs(asset)
self.pm.add_hookspecs(taskinstance)
self.pm.add_hookspecs(importerrors)
```
This has the nice effect that in each component you will have only those listeners injected that make sense in the context of that component.
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665960399
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
Also as a "side-effect"- the compnent-specific listener specifications will be defined in the components that use them, which will allow us to have for example separate documentation generated for core and sdk describing listeners where they are used - simply because the right list of specs will be in the right package and distribution.
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671228046
##########
airflow-core/src/airflow/listeners/spec/asset.py:
##########
@@ -15,6 +15,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/listeners/spec/importerrors.py:
##########
@@ -15,6 +15,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665122890
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -92,6 +92,8 @@
TaskDeferred as TaskDeferred,
XComNotFound as XComNotFound,
)
+ from airflow.sdk.listeners import hookimpl as hookimpl
+ from airflow.sdk.listeners.listener import get_listener_manager as get_listener_manager
Review Comment:
Yes, you are right here
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665118797
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -257,6 +259,10 @@
"airflow.utils.log.secrets_masker",
),
# ============================================================================
+ # Listeners
+ # ============================================================================
+ "hookimpl": ("airflow.sdk.listeners", "airflow.listeners"),
Review Comment:
Good catch. Fixed in 6a21b5f7b0
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665955256
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
This is really the common pattern we should adopt:
* have comon APIs that are usable on both
* whenever you have a component-specific 'thingie" make sure to keep it in the component and inject at explicit initialization time.
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
Same as in the other libraries -> I think you should inject specs to the shared listener from the "user" - so in airflow core you should keep the specs only used in the core,m and you should pass the list of specs when initializing listener manager.
```python
self.pm.add_hookspecs(lifecycle)
self.pm.add_hookspecs(dagrun)
self.pm.add_hookspecs(asset)
self.pm.add_hookspecs(taskinstance)
self.pm.add_hookspecs(importerrors)
```
This has the nice effect that in each component you will have only those listeners injected that make sense in the context of that component.
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671309493
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
These are used only for shared library tests and those are run separately per library. So do you think this still is an issue?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #59883:
URL: https://github.com/apache/airflow/pull/59883#issuecomment-3702254069
Precursor PR to this one: https://github.com/apache/airflow/pull/59956, this will be needed first
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2672081389
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
This will require getting rid of all the import side effects though
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667114084
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
Actually yes, thats a good idea. Let me take a stab at it
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2650906013
##########
shared/listeners/tests/listeners/test_listener_manager.py:
##########
@@ -0,0 +1,133 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow_shared.listeners import hookimpl
+from airflow_shared.listeners.listener import ListenerManager
+
+
+class TestListenerManager:
Review Comment:
Was able to add some basic tests using cursor's help. It tests basics of a listeners manager like initialisation, adding hooks, calling hooks, etc
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2652743386
##########
task-sdk/src/airflow/sdk/listeners/listener.py:
##########
@@ -0,0 +1,36 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from functools import cache
+
+from airflow.plugins_manager import integrate_listener_plugins
+from airflow.sdk._shared.listeners.listener import ListenerManager
+
+
+@cache
+def get_listener_manager() -> ListenerManager:
+ """Get singleton listener manager."""
+ _listener_manager = ListenerManager()
+ # TODO: For the time being, task sdk will use airflow's plugin mechanism to discover listeners and load them.
+ # We need to work out a better way to do this in the future.
+ integrate_listener_plugins(_listener_manager) # type: ignore[arg-type]
Review Comment:
I don't love this. I can think of a way through entrypoints, wondering what others think.
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2668019399
##########
airflow-core/tests/unit/listeners/test_listeners.py:
##########
@@ -159,27 +151,25 @@ def test_listener_captures_longrunning_taskinstances(create_task_instance_of_ope
@provide_session
-def test_class_based_listener(create_task_instance, session):
- lm = get_listener_manager()
+def test_class_based_listener(create_task_instance, session, listener_manager):
listener = class_listener.ClassBasedListener()
- lm.add_listener(listener)
+ listener_manager(listener)
ti = create_task_instance(session=session, state=TaskInstanceState.QUEUED)
ti.run()
assert listener.state == [TaskInstanceState.RUNNING, TaskInstanceState.SUCCESS, DagRunState.SUCCESS]
-def test_listener_logs_call(caplog, create_task_instance, session):
- caplog.set_level(logging.DEBUG, logger="airflow.listeners.listener")
- lm = get_listener_manager()
- lm.add_listener(full_listener)
+def test_listener_logs_call(caplog, create_task_instance, session, listener_manager):
+ caplog.set_level(logging.DEBUG, logger="airflow.sdk._shared.listeners.listener")
Review Comment:
Hmm. I think this should be fixed and we should separate those somehow - because if we do it this way, we will never be able to run tests on airflow-core without task-sdk being installed - which should be the mai guardrail for us to not accidentally depend on task-sdk.
But - we can deal with it later.
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2664198426
##########
task-sdk/src/airflow/sdk/listeners/__init__.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.sdk._shared.listeners import hookimpl
+
+__all__ = ["hookimpl"]
Review Comment:
> I also kind of wonder if we should do some additional logic here to avoid having two hookimpl objects when both Core and SDK are available. Would it cause issues?
We would need sdk and core objects to co-exist I think in cases when listener needs to run on server as well as workers?
##########
shared/listeners/src/airflow_shared/listeners/listener.py:
##########
@@ -0,0 +1,77 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import logging
+from typing import TYPE_CHECKING
+
+import pluggy
+
+if TYPE_CHECKING:
+ from pluggy._hooks import _HookRelay
+
+log = logging.getLogger(__name__)
Review Comment:
Handled
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667570167
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
Oh bad rebase from earlier
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2668020636
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
yes
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671413856
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
Let me just do it after this in a follow up: https://github.com/apache/airflow/issues/60253
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667583774
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
Actually seems this was missed from earlier @potiuk, it would be beneficial to have dagnode in there for shared library tests for dagnode right?
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #59883:
URL: https://github.com/apache/airflow/pull/59883#issuecomment-3718421500
@kaxil there's a request changes from you over here, could you take a look again? Or remove the request change if your veto was handled?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665107283
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
Called from AssetManager: https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/assets/manager.py#L289-L295 and it gets a `SerializedAsset`
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667200580
##########
airflow-core/src/airflow/listeners/spec/asset.py:
##########
Review Comment:
I think I have it at a state now where I am mostly happy with it. Investigated all call sites of the specs and arrived at this conclusion:
| Spec | Location | Called from SDK? | Called from Core? |
|---------------|----------|------------------|-------------------|
| lifecycle | Shared | ✅ | ✅ |
| taskinstance | Shared | ✅ | ✅ |
| dagrun | Core | ❌ | ✅ |
| asset | Core | ❌ | ✅ |
| importerrors | Core | ❌ | ✅ |
The specs have been moved to the relevant sites now. For example, sdk only has lifecycle and taskinstance which it consumes from shared because its also used in core, a better example would probably be `asset` - only used in core, stays in core
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2672495383
##########
task-sdk/src/airflow/sdk/listeners/__init__.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.sdk._shared.listeners import hookimpl
+
+__all__ = ["hookimpl"]
Review Comment:
Done in a different way now, so this comment is not valid
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671228046
##########
airflow-core/src/airflow/listeners/spec/asset.py:
##########
@@ -15,6 +15,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/listeners/spec/importerrors.py:
##########
@@ -15,6 +15,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh merged PR #59883:
URL: https://github.com/apache/airflow/pull/59883
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2668034212
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
https://github.com/apache/airflow/pull/60211
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2664112356
##########
providers/openlineage/src/airflow/providers/openlineage/plugins/listener.py:
##########
@@ -28,9 +28,8 @@
from openlineage.client.serde import Serde
from airflow import settings
-from airflow.listeners import hookimpl
from airflow.models import DagRun, TaskInstance
-from airflow.providers.common.compat.sdk import Stats, timeout, timezone
+from airflow.providers.common.compat.sdk import Stats, hookimpl, timeout, timezone
Review Comment:
Thanks kaxil, OL already has it present: https://github.com/apache/airflow/blob/main/providers/openlineage/pyproject.toml#L63, was added few days ago for some other PR
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665004311
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -92,6 +92,8 @@
TaskDeferred as TaskDeferred,
XComNotFound as XComNotFound,
)
+ from airflow.sdk.listeners import hookimpl as hookimpl
+ from airflow.sdk.listeners.listener import get_listener_manager as get_listener_manager
Review Comment:
```py
from airflow.sdk._shared.listeners import hookimpl as hookimpl
from airflow.sdk.listener import get_listener_manager as get_listener_manager
```
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -257,6 +259,10 @@
"airflow.utils.log.secrets_masker",
),
# ============================================================================
+ # Listeners
+ # ============================================================================
+ "hookimpl": ("airflow.sdk.listeners", "airflow.listeners"),
+ "get_listener_manager": ("airflow.sdk.listeners.listener", "airflow.listeners.listener"),
Review Comment:
Should these 2 be?
```py
"hookimpl": ("airflow.sdk._shared.listeners", "airflow.listeners"),
"get_listener_manager": ("airflow.sdk.listener", "airflow.listeners.listener"),
}
```
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -92,6 +92,8 @@
TaskDeferred as TaskDeferred,
XComNotFound as XComNotFound,
)
+ from airflow.sdk.listeners import hookimpl as hookimpl
+ from airflow.sdk.listeners.listener import get_listener_manager as get_listener_manager
Review Comment:
(_I think_)
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -257,6 +259,10 @@
"airflow.utils.log.secrets_masker",
),
# ============================================================================
+ # Listeners
+ # ============================================================================
+ "hookimpl": ("airflow.sdk.listeners", "airflow.listeners"),
Review Comment:
After @uranusjr's feedback, it looks like this was changed from listeners/ (module) to listener.py (file), but this wasn't updated
##########
shared/listeners/src/airflow_shared/listeners/spec/asset.py:
##########
Review Comment:
This Listener for Asset: where are these called from? and what objects do they receive?
`SerializedAsset`, `Asset` or both?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665019758
##########
providers/openlineage/src/airflow/providers/openlineage/plugins/listener.py:
##########
@@ -28,9 +28,8 @@
from openlineage.client.serde import Serde
from airflow import settings
-from airflow.listeners import hookimpl
from airflow.models import DagRun, TaskInstance
-from airflow.providers.common.compat.sdk import Stats, timeout, timezone
+from airflow.providers.common.compat.sdk import Stats, hookimpl, timeout, timezone
Review Comment:
yup got added after the comment :)
https://github.com/apache/airflow/commit/a09ed06a2b59057d1e397b8e28821b26038ea2ab
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667470275
##########
airflow-core/src/airflow/listeners/spec/asset.py:
##########
Review Comment:
Cool. Let me take a look. Nice table :)
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2667476889
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
dagnode here?
##########
pyproject.toml:
##########
@@ -1470,6 +1473,8 @@ members = [
"providers-summary-docs",
"docker-stack-docs",
"shared/configuration",
+ "shared/dagnode",
Review Comment:
And here?
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2664170611
##########
task-sdk/src/airflow/sdk/listeners/__init__.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.sdk._shared.listeners import hookimpl
+
+__all__ = ["hookimpl"]
Review Comment:
Good call again, it is needed for core, but sdk doesn't _need_ it, in that process I also realised that we can just have a listeners.py instead of a module in sdk
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665122890
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -92,6 +92,8 @@
TaskDeferred as TaskDeferred,
XComNotFound as XComNotFound,
)
+ from airflow.sdk.listeners import hookimpl as hookimpl
+ from airflow.sdk.listeners.listener import get_listener_manager as get_listener_manager
Review Comment:
Yes, you are right here, fixed in: eca2451e1a
--
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
Re: [PR] Extract airflow listeners as a shared library [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2665121168
##########
providers/common/compat/src/airflow/providers/common/compat/sdk.py:
##########
@@ -257,6 +259,10 @@
"airflow.utils.log.secrets_masker",
),
# ============================================================================
+ # Listeners
+ # ============================================================================
+ "hookimpl": ("airflow.sdk.listeners", "airflow.listeners"),
+ "get_listener_manager": ("airflow.sdk.listeners.listener", "airflow.listeners.listener"),
Review Comment:
Yes, pushed a fix in 842c32fc9c
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2668021179
##########
pyproject.toml:
##########
@@ -1289,6 +1289,8 @@ dev = [
"apache-airflow-ctl",
"apache-airflow-ctl-tests",
"apache-airflow-shared-configuration",
+ "apache-airflow-shared-dagnode",
Review Comment:
But maybe a separate PR - let me add it
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671243273
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
Not a fan with this global side effect on import, especially since this leaks out of the tests (if you run multiple directories together)
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2671339443
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
The thing I don’t like is if you run the shared lib and core tests in the same process (say locally in breeze) it would mysteriously fail. I wonder if this would work in a module-scoped 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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59883:
URL: https://github.com/apache/airflow/pull/59883#discussion_r2672082675
##########
shared/listeners/tests/conftest.py:
##########
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import os
+
+os.environ["_AIRFLOW__AS_LIBRARY"] = "true"
Review Comment:
But other than that - ABSOLUTELY YES
--
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
Re: [PR] Move listeners module to shared library for client server separation [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #59883:
URL: https://github.com/apache/airflow/pull/59883#issuecomment-3882958150
cc: @atul-astronomer another good candidate to run some listeners in existing test suite to ensure no regressions.
--
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