henry3260 commented on code in PR #74548:
URL: https://github.com/apache/airflow/pull/74548#discussion_r4241495499


##########
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:
   I think the same issue exists in the loop-aware path? When `loop_aware` is
   true, the `not loop_aware` check here skips the branch this PR fixes, and
   `select_loop_clear_scope` grows `retry` in place the same way 
(`taskinstance.py:788-793`).



##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -1090,19 +1090,19 @@ def post_clear_task_instances(
         # Unmapped tasks are expressed in their task_ids (without map_indexes)
         normal_task_ids = {t for t in task_markers_to_clear if not 
isinstance(t, tuple)}
 
-        def _collect_relatives(run_id: str, direction: Literal["upstream", 
"downstream"]) -> None:
+        def _find_relatives(
+            run_id: str, direction: Literal["upstream", "downstream"]
+        ) -> Collection[str | tuple[str, int]]:

Review Comment:
   nit: `_find_relatives` still reads `normal_task_ids` and 
`mapped_tasks_tuples`
   from the enclosing scope, and both sets are mutable. Could we pass them in
   explicitly?



-- 
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]

Reply via email to