amoghrajesh commented on code in PR #69989:
URL: https://github.com/apache/airflow/pull/69989#discussion_r3615360420
##########
task-sdk/src/airflow/sdk/plugins_manager.py:
##########
@@ -128,9 +128,17 @@ def integrate_macros_plugins() -> None:
)
-def integrate_listener_plugins(listener_manager: ListenerManager) -> None:
- """Add listeners from plugins."""
+def integrate_listener_plugins(listener_manager: ListenerManager, team_name:
str | None = None) -> None:
+ """
+ Add listeners from plugins to the given listener manager.
+
+ Only listeners from global plugins (``team_name is None``) and plugins
belonging
+ to ``team_name`` are registered. On the worker, ``team_name`` comes from
the
+ server-provided task instance context; filtering here is always applied
and does
+ not depend on the worker reading ``core.multi_team`` from its own
configuration.
+ """
plugins, _ = _get_plugins()
+ plugins = [plugin for plugin in plugins if plugin.team_name in (None,
team_name)]
Review Comment:
This filters `plugin.team_name` in `(None, team_name)` unconditionally, but
the core equivalent only applies this filter when `core.multi_team` is enabled
otherwise it registers every plugin regardless of team_name.
That difference creates an asymmetry when multi team mode is disabled on a
deployment that still has team_name tagged plugins:
- Core hooks correctly go back to firing for every plugin, since core's
filtering is gated on the flag.
- Worker-triggered hooks stay silently scoped, because `ti.team_name`
resolves to `None` when the server has multi-team disabled, and this function's
filter then excludes any plugin with a non-None team_name with no flag-based
escape hatch.
Net result: disabling multi-team mode does not fully restore "every plugin
sees every event" - a team-scoped plugin quietly stops getting its most
important events (the worker-side ones) with no error or log line to explain
why.
##########
task-sdk/src/airflow/sdk/plugins_manager.py:
##########
@@ -128,9 +128,17 @@ def integrate_macros_plugins() -> None:
)
-def integrate_listener_plugins(listener_manager: ListenerManager) -> None:
- """Add listeners from plugins."""
+def integrate_listener_plugins(listener_manager: ListenerManager, team_name:
str | None = None) -> None:
+ """
+ Add listeners from plugins to the given listener manager.
+
+ Only listeners from global plugins (``team_name is None``) and plugins
belonging
+ to ``team_name`` are registered. On the worker, ``team_name`` comes from
the
+ server-provided task instance context; filtering here is always applied
and does
+ not depend on the worker reading ``core.multi_team`` from its own
configuration.
+ """
plugins, _ = _get_plugins()
+ plugins = [plugin for plugin in plugins if plugin.team_name in (None,
team_name)]
Review Comment:
The core version of this filters by `conf.getboolean("core", "multi_team")`,
why the drift?
##########
devel-common/src/tests_common/pytest_plugin.py:
##########
@@ -3107,7 +3107,7 @@ def test_something(listener_manager):
get_sdk_lm = None
core_lm = get_core_lm()
- sdk_lm = get_sdk_lm() if get_sdk_lm else None
+ sdk_lm = get_sdk_lm() if get_sdk_lm is not None else None
Review Comment:
Seems like a no-op change?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
