dheerajturaga commented on code in PR #73724:
URL: https://github.com/apache/airflow/pull/73724#discussion_r4211161659
##########
airflow-core/tests/unit/utils/test_task_group.py:
##########
@@ -1351,6 +1351,88 @@ def spy(self, nodes, projected):
assert position[f"r{i}"] < position[f"r{i + 1}"]
+def _make_sibling_groups_cycle():
+ with DAG("sibling_groups_cycle", schedule=None, start_date=DEFAULT_DATE)
as dag:
+ start = EmptyOperator(task_id="start")
+ with TaskGroup("group1"):
+ a1 = EmptyOperator(task_id="a1")
+ a2 = EmptyOperator(task_id="a2")
+ with TaskGroup("group2"):
+ b1 = EmptyOperator(task_id="b1")
+ b2 = EmptyOperator(task_id="b2")
+ end = EmptyOperator(task_id="end")
+ start >> [a1, b2]
+ a1 >> b1
+ b2 >> a2
+ [a2, b1] >> end
+ return dag
+
+
+def _make_group_bridged_by_outside_task():
+ with DAG("group_bridged_by_outside_task", schedule=None,
start_date=DEFAULT_DATE) as dag:
+ with TaskGroup("group"):
+ a = EmptyOperator(task_id="a")
+ b = EmptyOperator(task_id="b")
+ bridge = EmptyOperator(task_id="bridge")
+ a >> bridge >> b
+ return dag
+
+
+def _make_three_group_ring():
+ with DAG("three_group_ring", schedule=None, start_date=DEFAULT_DATE) as
dag:
+ groups = {}
+ for group_id in ("g0", "g1", "g2"):
+ with TaskGroup(group_id):
+ groups[group_id] = (EmptyOperator(task_id="first"),
EmptyOperator(task_id="second"))
+ groups["g1"][0] >> groups["g0"][1]
+ groups["g2"][0] >> groups["g1"][1]
+ groups["g0"][0] >> groups["g2"][1]
+ return dag
+
+
[email protected](
+ ("make_dag", "expected"),
+ [
+ pytest.param(
+ _make_sibling_groups_cycle,
+ {
+ None: ["start", "group1", "group2", "end"],
+ "group1": ["group1.a1", "group1.a2"],
+ "group2": ["group2.b1", "group2.b2"],
+ },
+ id="sibling-groups",
+ ),
+ pytest.param(
+ _make_group_bridged_by_outside_task,
+ {None: ["bridge", "group"], "group": ["group.a", "group.b"]},
Review Comment:
Agreed, and confirmed: with `_sort_cyclic_projection` returning
`list(nodes)`, only `sibling-groups` failed. Added both shapes you suggested.
`bridged-group-among-siblings` is your `after` / `bridge` / `x0..x2` shape: it
reaches the fallback through the sweep and expects `[bridge, group, x0, x1, x2,
after]`. `nested-bridged-group` has the cycle between `outer.inner` and
`outer.bridge` with `outer.after` downstream, and goes through pass numbering.
An identity fallback now fails both. The builders also declare children in
label order, the order deserialization gives them, so the same test can assert
that the Task SDK sort agrees (see the other thread).
---
Drafted-by: Claude Code (Opus 5.5); reviewed by @dheerajturaga before posting
--
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]
