Repository navigation
Honor wait_for_past_depends_before_skipping when an upstream skips - #75028
Open
luc-pimentel wants to merge 2 commits into
Open
luc-pimentel wants to merge 2 commits into
luc-pimentel wants to merge 2 commits into
Conversation
luc-pimentel
requested review from
amoghrajesh,
ashb and
bolkedebruin
as code owners
October 19, 2026 01:24
1 of 2 tasks
Contributor
Author
|
Rebased on main after #72718 landed. The three deps now read the past-depends mark through its new Drafted-by: Claude Code (Fable 5.1); reviewed by @luc-pimentel before posting |
luc-pimentel
force-pushed
the
fix/55146-wait-for-past-depends-before-skipping
branch
from
October 19, 2026 19:08
f7a3398 to
013ae7c
Compare
…t once A task with depends_on_past=True and wait_for_past_depends_before_skipping=True is documented to stay unscheduled until its previous run's task instance has succeeded or been skipped, and only then be skipped. Today the task attribute is never read: SkipMixin lets the execution API skip the task at once, and the scheduler deps only honour the DepContext flag, which nothing sets. These tests fail on main and pass with the fix: the scheduling decision for a short-circuit and for a branch decision, the SkipMixin lists, the three deps reading the task attribute, and the serialized mapped operator exposing it.
A task with depends_on_past=True and wait_for_past_depends_before_skipping=True was skipped at once when an upstream ShortCircuitOperator or branch operator decided to skip it, even though its previous run's task instance had failed. Two things stood between the parameter and the scheduler. SkipMixin raised DownstreamTasksSkipped for every direct downstream task, so the execution API wrote SKIPPED before any dependency ran. And the scheduler deps only honoured DepContext.wait_for_past_depends_before_skipping, which nothing sets in Airflow 3; the task attribute was never read, and SerializedMappedOperator did not expose it. SkipMixin now leaves the direct downstream tasks that set both flags out of the state change, keeping them in the XCom decision, so NotPreviouslySkippedDep decides. The deps read the task attribute as well as the context flag, and PrevDagrunDep writes the past-depends-met mark through the scheduler's session instead of a fresh one that commits inside the scheduling loop. A task that sets only one of the two flags keeps today's immediate skip, as its docstring says, and tasks reached through another skipped task are still skipped at once.
luc-pimentel
force-pushed
the
fix/55146-wait-for-past-depends-before-skipping
branch
from
October 19, 2026 23:13
013ae7c to
99b9a77
Compare
Contributor
Author
|
Rebased on main to pick up #75126 and #75138. All six previously failing API tests now pass locally, along with the scoped scheduler/SDK tests and prek. Drafted-by: OpenCode (openai/gpt-6-astra); reviewed by @luc-pimentel before posting |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A task with
depends_on_past=Trueandwait_for_past_depends_before_skipping=Trueis skipped immediately when its upstream short-circuits or branches away, even if the previous run’s task failed. It should stay unscheduled until its past dependency is met.The SDK skips tasks before scheduler dependencies run, and those dependencies only read the unused context flag. This change leaves flagged direct downstream tasks for the scheduler, preserves the XCom skip decision, and makes the dependencies honor the task flag. It also exposes the flag on serialized mapped operators and uses the scheduler’s session when writing the past-dependency XCom.
Both flags are required. Indirect descendants force-skipped by
ignore_downstream_trigger_rules=Trueretain their current behavior.Testing
With
airflow standaloneon SQLite, run 1 fails the downstream task and run 2 short-circuits:skippedskippedNone— waitingskippedAfter marking run 1’s task successful, the waiting task is skipped within 10 seconds.
Regression tests cover short-circuit and branch decisions, dependency checks and mapped serialization; the new failure cases fail without the fix. Scoped tests and prek pass.
Builds on the SkipMixin approach in #73470 and the design in discussion #26460.
closes: #55146
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Fable 5.1) and OpenCode (openai/gpt-6-astra) following the guidelines