-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fix TriggerDagRunOperator deferring when wait_for_completion=False
#60052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e487a8f to
3215f5f
Compare
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py
Outdated
Show resolved
Hide resolved
The bug was in the task runner's DagRunTriggerException handler, which checked the deferrable flag before checking wait_for_completion. This caused tasks with deferrable=True to incorrectly enter DEFERRED state even in fire-and-forget mode (wait_for_completion=False). Fixed by nesting the deferrable check inside the wait_for_completion check, ensuring tasks only defer when BOTH conditions are true. Also updated test expectations to validate the correct behaviour.
1891ea3 to
2b7eb2c
Compare
|
Is it ever useful for |
|
@uranusjr Setting |
|
Sorry I mistyped. I meant |
|
@uranusjr Yes, that's exactly the change I am proposing! Initially I modified the operators |
uranusjr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logically no problem. I’d recommend adding a log message (warning or at leat info) when deferrable=True is ignored so the user has better idea what happened.
Ok, will do! |
2b7eb2c to
551f04e
Compare
|
@uranusjr Thanks for approving! |
Backport failed to create: v3-1-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 2daadf4 v3-1-testThis should apply the commit to the v3-1-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
apache#60052) The bug was in the task runner's DagRunTriggerException handler, which checked the deferrable flag before checking wait_for_completion. This caused tasks with deferrable=True to incorrectly enter DEFERRED state even in fire-and-forget mode (wait_for_completion=False). Fixed by nesting the deferrable check inside the wait_for_completion check, ensuring tasks only defer when BOTH conditions are true. Also updated test expectations to validate the correct behaviour.
| ), | ||
| method_name="execute_complete", | ||
| ) | ||
| return _defer_task(defer, ti, log) | ||
| if drte.wait_for_completion: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @nathadfield
apache#60052) The bug was in the task runner's DagRunTriggerException handler, which checked the deferrable flag before checking wait_for_completion. This caused tasks with deferrable=True to incorrectly enter DEFERRED state even in fire-and-forget mode (wait_for_completion=False). Fixed by nesting the deferrable check inside the wait_for_completion check, ensuring tasks only defer when BOTH conditions are true. Also updated test expectations to validate the correct behaviour.
Summary
Closes #60049
The bug was in the task runner's
DagRunTriggerExceptionhandler, which checked thedeferrableflag before checkingwait_for_completion. This caused tasks withdeferrable=Trueto incorrectly enterDEFERREDstate even in fire-and-forget mode (wait_for_completion=False).Root Cause
In
task_runner.py, the handler forDagRunTriggerExceptionwas structured like:This meant that tasks would defer based solely on the deferrable flag, ignoring wait_for_completion.
Fix
Nested the deferrable check inside the wait_for_completion check:
Now tasks only defer when BOTH
deferrable=TrueANDwait_for_completion=True.Changes
task-sdk/src/airflow/sdk/execution_time/task_runner.py: Fixed the exception handler logictask-sdk/tests/task_sdk/execution_time/test_task_runner.py: Updated test expectations to validate correct behavior