sjyangkevin commented on code in PR #74548:
URL: https://github.com/apache/airflow/pull/74548#discussion_r4241954784
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -1119,10 +1119,15 @@ def _collect_relatives(run_id: str, direction:
Literal["upstream", "downstream"]
)
normal_task_ids.update(partial_dag.task_dict)
else:
+ # Both directions must start from the requested tasks only.
Merging upstream results
+ # before the downstream pass would also select every descendant of
those upstream tasks.
+ relatives: set[str | tuple[str, int]] = set()
if upstream:
- _collect_relatives(dag_run_id, "upstream")
+ relatives.update(_find_relatives(dag_run_id, "upstream"))
if downstream:
- _collect_relatives(dag_run_id, "downstream")
+ relatives.update(_find_relatives(dag_run_id, "downstream"))
+ normal_task_ids.update(t for t in relatives if not isinstance(t,
tuple))
+ mapped_tasks_tuples.update(t for t in relatives if isinstance(t,
tuple))
Review Comment:
`select_loop_clear_scope` now captures the selection before the upstream and
downstream passes, and both passes walk from it, as the non-loop path now does.
That function is also used for Mark as success/failed on a task inside a loop,
so the fix covers that too.
What it looked like before, checked in Breeze:
- The issue's Dag plus any unrelated loop goes through the loop-aware path,
so clearing `a` still selected `other`, and clearing `a[0]` still selected
`a[1]` and `a[2]`.
- For a task inside a loop with an upstream task outside it, the downstream
pass from that outside task reached every iteration's gate. Clearing a task in
iteration 1 then replaced iterations 1 and 2, including the selected task, and
re-ran the loop from iteration 0. Marking it as failed marked every task of
every iteration.
On compatibility: loops have not been released yet, and `v3-4-test` is
fast-forwarded from `main` during the 3.4.0 betas, so no compatibility handling
is needed.
Two notes on the loop part:
- The `upstream=True` case of
`test_outside_predecessor_selects_all_mapped_loop_occurrences` expected one map
index inside a loop, cleared with both options, to select every task instance
of the run. That depended on the downstream pass starting from the outside
predecessor, so I updated its expectation.
- Clearing an exact execution inside a loop does not pair setups with
teardowns (`include_setups_and_teardowns=False` for `task_instance_ids`). If an
upstream setup's teardown is not downstream of the selected task, it is no
longer selected; before, it was selected only through this flaw, together with
unrelated tasks. That now matches Upstream-only, which already behaves this
way. I left the pairing unchanged; the PR description has the details.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
