Skip to content

Commit f7a3398

Browse files
committed
Honour wait_for_past_depends_before_skipping when a task is told to skip
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.
1 parent fe3aa8e commit f7a3398

6 files changed

Lines changed: 50 additions & 21 deletions

File tree

‎airflow-core/src/airflow/serialization/definitions/mappedoperator.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@ def ignore_first_depends_on_past(self) -> bool:
223223
def wait_for_downstream(self) -> bool:
224224
return self._get_partial_kwargs_or_operator_default("wait_for_downstream")
225225

226+
@property
227+
def wait_for_past_depends_before_skipping(self) -> bool:
228+
return self._get_partial_kwargs_or_operator_default("wait_for_past_depends_before_skipping")
229+
226230
@property
227231
def retries(self) -> int:
228232
return self._get_partial_kwargs_or_operator_default("retries")

‎airflow-core/src/airflow/ti_deps/deps/not_previously_skipped_dep.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,9 @@ def _skip(
130130
# If the parent SkipMixin has run, and the XCom result stored indicates this
131131
# ti should be skipped, set ti.state to SKIPPED and fail the rule so that the
132132
# ti does not execute.
133-
if dep_context.wait_for_past_depends_before_skipping:
133+
if TYPE_CHECKING:
134+
assert ti.task
135+
if dep_context.wait_for_past_depends_before_skipping or ti.task.wait_for_past_depends_before_skipping:
134136
past_depends_met = ti.xcom_pull(
135137
task_ids=ti.task_id, key=PAST_DEPENDS_MET, session=session, default=False
136138
)

‎airflow-core/src/airflow/ti_deps/deps/prev_dagrun_dep.py‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,12 @@ class PrevDagrunDep(BaseTIDep):
5050
IS_TASK_DEP = True
5151

5252
@staticmethod
53-
def _push_past_deps_met_xcom_if_needed(ti: TI, dep_context):
54-
if dep_context.wait_for_past_depends_before_skipping:
55-
ti.xcom_push(key=PAST_DEPENDS_MET, value=True)
53+
def _push_past_deps_met_xcom_if_needed(ti: TI, dep_context, *, session: Session):
54+
if TYPE_CHECKING:
55+
assert ti.task
56+
if dep_context.wait_for_past_depends_before_skipping or ti.task.wait_for_past_depends_before_skipping:
57+
# The scheduler's session: a fresh one would commit inside the scheduling loop.
58+
ti.xcom_push(key=PAST_DEPENDS_MET, value=True, session=session)
5659

5760
@staticmethod
5861
def _has_tis(dagrun: DagRun, task_id: str, *, session: Session) -> bool:
@@ -134,13 +137,13 @@ def _get_dep_statuses(self, ti: TI, dep_context, *, session: Session):
134137
if TYPE_CHECKING:
135138
assert ti.task
136139
if dep_context.ignore_depends_on_past:
137-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
140+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)
138141
reason = "The context specified that the state of past DAGs could be ignored."
139142
yield self._passing_status(reason=reason)
140143
return
141144

142145
if not ti.task.depends_on_past:
143-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
146+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)
144147
yield self._passing_status(reason="The task did not have depends_on_past set.")
145148
return
146149

@@ -157,7 +160,7 @@ def _get_dep_statuses(self, ti: TI, dep_context, *, session: Session):
157160
return
158161

159162
if not dr:
160-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
163+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)
161164
yield self._passing_status(reason="This task instance does not belong to a DAG.")
162165
return
163166

@@ -169,7 +172,7 @@ def _get_dep_statuses(self, ti: TI, dep_context, *, session: Session):
169172
last_dagrun = DagRun.get_previous_dagrun(dr, session=session)
170173
# First ever run for this DAG.
171174
if not last_dagrun:
172-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
175+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)
173176
yield self._passing_status(reason="This task instance was the first task instance for its task.")
174177
return
175178

@@ -180,14 +183,14 @@ def _get_dep_statuses(self, ti: TI, dep_context, *, session: Session):
180183
and last_dagrun.logical_date is not None
181184
and last_dagrun.logical_date < ti.task.start_date
182185
):
183-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
186+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)
184187
yield self._passing_status(reason="This task instance was the first task instance for its task.")
185188
return
186189

187190
if not self._has_tis(last_dagrun, ti.task_id, session=session):
188191
if ti.task.ignore_first_depends_on_past:
189192
if not self._has_any_prior_tis(ti, session=session):
190-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
193+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)
191194
yield self._passing_status(
192195
reason="ignore_first_depends_on_past is true for this task "
193196
"and it is the first task instance for its task."
@@ -220,4 +223,4 @@ def _get_dep_statuses(self, ti: TI, dep_context, *, session: Session):
220223
)
221224
return
222225

223-
self._push_past_deps_met_xcom_if_needed(ti, dep_context)
226+
self._push_past_deps_met_xcom_if_needed(ti, dep_context, session=session)

‎airflow-core/src/airflow/ti_deps/deps/trigger_rule_dep.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,9 @@ def _evaluate_setup_constraint(
366366
new_state = TaskInstanceState.REMOVED
367367

368368
if new_state is not None:
369-
if (
370-
new_state == TaskInstanceState.SKIPPED
371-
and dep_context.wait_for_past_depends_before_skipping
369+
if new_state == TaskInstanceState.SKIPPED and (
370+
dep_context.wait_for_past_depends_before_skipping
371+
or task.wait_for_past_depends_before_skipping
372372
):
373373
past_depends_met = ti.xcom_pull(
374374
task_ids=ti.task_id, key=PAST_DEPENDS_MET, session=session, default=False
@@ -527,9 +527,9 @@ def _evaluate_direct_relatives() -> Iterator[TIDepStatus]:
527527
# if at least one setup ran, we'll let it run
528528
new_state = TaskInstanceState.UPSTREAM_FAILED
529529
if new_state is not None:
530-
if (
531-
new_state == TaskInstanceState.SKIPPED
532-
and dep_context.wait_for_past_depends_before_skipping
530+
if new_state == TaskInstanceState.SKIPPED and (
531+
dep_context.wait_for_past_depends_before_skipping
532+
or task.wait_for_past_depends_before_skipping
533533
):
534534
past_depends_met = ti.xcom_pull(
535535
task_ids=ti.task_id, key=PAST_DEPENDS_MET, session=session, default=False

‎airflow-core/tests/unit/ti_deps/deps/test_prev_dagrun_dep.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ def test_dagrun_dep(mock_get_previous_dagrun, mock_get_previous_scheduled_dagrun
330330

331331
assert actual_dep_met == expected_dep_met
332332
if past_depends_met_xcom_sent:
333-
ti.xcom_push.assert_called_with(key="past_depends_met", value=True)
333+
ti.xcom_push.assert_called_with(key="past_depends_met", value=True, session=ANY)
334334
else:
335335
ti.xcom_push.assert_not_called()
336336

@@ -346,4 +346,4 @@ def test_past_depends_met_xcom_sent_when_the_task_asks_to_wait():
346346
ti = Mock(task=task, task_id=task.task_id, **{"xcom_push.return_value": None})
347347

348348
assert PrevDagrunDep().is_met(ti=ti, dep_context=DepContext(ignore_depends_on_past=True))
349-
ti.xcom_push.assert_called_once_with(key="past_depends_met", value=True)
349+
ti.xcom_push.assert_called_once_with(key="past_depends_met", value=True, session=ANY)

‎task-sdk/src/airflow/sdk/bases/skipmixin.py‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,24 @@ def _set_state_to_skipped(
5555
if tasks and map_index == -1:
5656
raise DownstreamTasksSkipped(tasks=tasks)
5757

58+
def _tasks_to_skip_now(self, tasks: Sequence[Operator]) -> list[Operator]:
59+
"""
60+
Drop the direct downstream tasks that wait for their past depends before being skipped.
61+
62+
They stay in the XCom decision, so the scheduler skips them once the previous run's task
63+
instance has succeeded or been skipped (``NotPreviouslySkippedDep``).
64+
"""
65+
direct_downstream_ids: set[str] = getattr(self, "downstream_task_ids", set())
66+
return [
67+
t
68+
for t in tasks
69+
if not (
70+
t.task_id in direct_downstream_ids
71+
and t.depends_on_past
72+
and t.wait_for_past_depends_before_skipping
73+
)
74+
]
75+
5876
def skip(
5977
self,
6078
ti: RuntimeTaskInstanceProtocol,
@@ -79,7 +97,7 @@ def skip(
7997
value={XCOM_SKIPMIXIN_SKIPPED: task_ids_list},
8098
)
8199

82-
self._set_state_to_skipped(task_ids_list, ti.map_index)
100+
self._set_state_to_skipped([t.task_id for t in self._tasks_to_skip_now(task_list)], ti.map_index)
83101

84102
def skip_all_except(
85103
self,
@@ -156,7 +174,9 @@ def skip_all_except(
156174
branch_task_id_set.update(dag.get_task(branch_task_id).get_flat_relative_ids(upstream=False))
157175

158176
skip_tasks = [
159-
(t.task_id, ti.map_index) for t in downstream_tasks if t.task_id not in branch_task_id_set
177+
(t.task_id, ti.map_index)
178+
for t in self._tasks_to_skip_now(downstream_tasks)
179+
if t.task_id not in branch_task_id_set
160180
]
161181

162182
follow_task_ids = [t.task_id for t in downstream_tasks if t.task_id in branch_task_id_set]

0 commit comments

Comments
 (0)