Repository navigation
Honor wait_for_past_depends_before_skipping when an upstream skips - #75028
luc-pimentel wants to merge 2 commits into
Conversation
|
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 |
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.
013ae7c to
99b9a77
Compare
|
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 |
|
Just to add a bit more context here: it looks like The option came in with #27710 in 2.6.0. In 2.x the dependency checks only read it from I ran a Dag like the one in #55146 on the released 2.x images with
Drafted-by: Claude Code (Opus 5.5); reviewed by @luc-pimentel before posting |
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