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 2026/02/06 13:24:05 UTC
[PR] Use attribute based trigger queue detection for triggers [airflow]
amoghrajesh opened a new pull request, #61531:
URL: https://github.com/apache/airflow/pull/61531
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
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 (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [x] No
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
Related: https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1770376076210459
Noticed these imports while working on client server separation and ensuring minimal import pattern from airflow core in task sdk. I am replacing the `isinstance` checks with a class based attribute to determine whether triggers types support trigger queues or not vs using isinstance to assert types which added runtime imports for `BaseEventTrigger` and `CallbackTrigger`
This enables task SDK to check queue support without importing trigger classes from core.
Triggers declare queue support via the `uses_triggerer_queue` attribute on the class itself.
- BaseTrigger defaults to True (supports queues)
- BaseEventTrigger and CallbackTrigger set to False (no queue support)
The config here looks like it is meant to be a feature flag to enable/disable the entire queue routing feature, which is retained.
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#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](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{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] Use attribute based trigger queue detection for triggers [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #61531:
URL: https://github.com/apache/airflow/pull/61531#issuecomment-3860442881
cc @zach-overflow correct me if I am wrong 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] Use attribute based trigger queue detection for triggers [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh merged PR #61531:
URL: https://github.com/apache/airflow/pull/61531
--
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] Use attribute based trigger queue detection for triggers [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #61531:
URL: https://github.com/apache/airflow/pull/61531#discussion_r2779176292
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1097,9 +1095,9 @@ def _defer_task(
classpath, trigger_kwargs = defer.trigger.serialize()
queue: str | None = None
# Currently, only task-associated BaseTrigger instances may have a non-None queue,
- # and only when triggerer.queues_enabled is True.
- if not isinstance(defer.trigger, (BaseEventTrigger, CallbackTrigger)) and conf.getboolean(
- "triggerer", "queues_enabled", fallback=False
+ # and only when triggerer.queues_enabled conf is True.
+ if conf.getboolean("triggerer", "queues_enabled", fallback=False) and getattr(
+ defer.trigger, "uses_triggerer_queue", True
Review Comment:
I retained the original behaviour. Earlier it checked for:
- Trigger is NOT BaseEventTrigger AND NOT CallbackTrigger
Now the logic does this:
- BaseEventTrigger.uses_triggerer_queue = False
- CallbackTrigger.uses_triggerer_queue = False
- BaseTrigger.uses_triggerer_queue = True (default for all other triggers)
Hence BaseEventTrigger and CallbackTrigger won't have triggers and ALL others will have it. Same as original logic
--
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] Use attribute based trigger queue detection for triggers [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #61531:
URL: https://github.com/apache/airflow/pull/61531#discussion_r2778869231
##########
airflow-core/src/airflow/triggers/base.py:
##########
@@ -63,6 +63,8 @@ class BaseTrigger(abc.ABC, LoggingMixin):
let them be re-instantiated elsewhere.
"""
+ uses_triggerer_queue: bool = True
Review Comment:
How about renaming as `supports_triggerer_queue` to align `BaseExecutor`?
https://github.com/apache/airflow/blob/a11ec7296ce52ae3693e46835a49d8c5e3677341/airflow-core/src/airflow/executors/base_executor.py#L145-L146
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1097,9 +1095,9 @@ def _defer_task(
classpath, trigger_kwargs = defer.trigger.serialize()
queue: str | None = None
# Currently, only task-associated BaseTrigger instances may have a non-None queue,
- # and only when triggerer.queues_enabled is True.
- if not isinstance(defer.trigger, (BaseEventTrigger, CallbackTrigger)) and conf.getboolean(
- "triggerer", "queues_enabled", fallback=False
+ # and only when triggerer.queues_enabled conf is True.
+ if conf.getboolean("triggerer", "queues_enabled", fallback=False) and getattr(
+ defer.trigger, "uses_triggerer_queue", True
Review Comment:
Should we fallback as `False` if `uses_triggerer_queue` not found ?
```suggestion
defer.trigger, "uses_triggerer_queue", False
```
--
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] Use attribute based trigger queue detection for triggers [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #61531:
URL: https://github.com/apache/airflow/pull/61531#discussion_r2779172571
##########
airflow-core/src/airflow/triggers/base.py:
##########
@@ -63,6 +63,8 @@ class BaseTrigger(abc.ABC, LoggingMixin):
let them be re-instantiated elsewhere.
"""
+ uses_triggerer_queue: bool = True
Review Comment:
Makes sense, let me update that
--
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] Use attribute based trigger queue detection for triggers [airflow]
Posted by "zach-overflow (via GitHub)" <gi...@apache.org>.
zach-overflow commented on PR #61531:
URL: https://github.com/apache/airflow/pull/61531#issuecomment-3872897206
Missed your DM in slack, but this looks very reasonable and clean to me 👍. Left a much more detailed followup in Slack to some of your other questions around non-TI related triggers.
--
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