Skip to content

Fix plugin registration returning early on duplicate names in Task SDK - #72253

Merged
potiuk merged 1 commit into
apache:mainfrom
olmaks:fix-task-sdk-duplicate-plugin-registration
Aug 29, 2026
Merged

potiuk merged 1 commit into
apache:mainfrom
olmaks:fix-task-sdk-duplicate-plugin-registration

Conversation

@olmaks

@olmaks olmaks commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

__register_plugins in task-sdk/src/airflow/sdk/plugins_manager.py uses return where it means
continue:

if plugin_instance.name in loaded_plugins:
    return

The first duplicate plugin name therefore abandons the rest of the batch, and every plugin after it is
dropped — silently, since nothing is logged and _get_plugins() still returns an empty import_errors.

airflow-core carries a copy of this function and fixed it in #60498, then hardened it in #66649 (record
the duplicate in import_errors). Neither fix reached the Task SDK copy. This PR applies both, so the
block becomes byte-identical to the core one.

It matters because the Task SDK copy is the one the task-running process uses. While the two disagree,
plugins load for the scheduler and API server but go missing inside tasks — most sharply, a plugin
registering on_task_instance_running / on_task_instance_failed after a duplicate name never fires,
with no error anywhere to attribute it to. Duplicate names are not exotic: the same plugin shipped both
flattened and as a package, or present in the plugins folder and as an entry point, is enough.

Covered by a new task-sdk/tests/task_sdk/test_plugins_manager.py carrying the two cases the core suite
already has for this branch; both fail on main and pass with the change.


Was generative AI tooling used to co-author this PR?
  • Yes

Generated-by: Claude Code following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

__register_plugins used `return` instead of `continue` when encountering
a duplicate plugin name, so the first duplicate abandoned the rest of the
batch and every plugin after it was silently dropped.

airflow-core carries a duplicate of this function and was fixed twice: in
apache#60498 (`return` became a warning plus `continue`) and in apache#66649 (also
record the duplicate in import_errors). Neither fix reached the Task SDK
copy. Because that copy is the one used by the task-running process,
plugins would load for the scheduler and API server but silently go
missing inside tasks - most visibly, task instance listeners registered
by a plugin would never fire.

The __register_plugins body is now byte-identical to the airflow-core one.
@boring-cyborg

boring-cyborg Bot commented Aug 29, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@potiuk
potiuk merged commit 49789f2 into apache:main Aug 29, 2026
107 checks passed
@boring-cyborg

boring-cyborg Bot commented Aug 29, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@olmaks
olmaks deleted the fix-task-sdk-duplicate-plugin-registration branch August 29, 2026 17:36
imrichardwu pushed a commit to imrichardwu/airflow that referenced this pull request Sep 11, 2026
apache#72253)

__register_plugins used `return` instead of `continue` when encountering
a duplicate plugin name, so the first duplicate abandoned the rest of the
batch and every plugin after it was silently dropped.

airflow-core carries a duplicate of this function and was fixed twice: in
apache#60498 (`return` became a warning plus `continue`) and in apache#66649 (also
record the duplicate in import_errors). Neither fix reached the Task SDK
copy. Because that copy is the one used by the task-running process,
plugins would load for the scheduler and API server but silently go
missing inside tasks - most visibly, task instance listeners registered
by a plugin would never fire.

The __register_plugins body is now byte-identical to the airflow-core one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants