dheerajturaga commented on code in PR #73746:
URL: https://github.com/apache/airflow/pull/73746#discussion_r4224577380
##########
task-sdk/src/airflow/sdk/definitions/dag.py:
##########
@@ -1150,6 +1154,30 @@ def check_cycle(self) -> None:
f"Cycle detected in Dag: {self.dag_id}. Faulty task:
{faulty_task_id}"
)
+ self._warn_task_group_cycles()
+
+ def _warn_task_group_cycles(self) -> None:
+ task_group_dict = self.task_group.get_task_group_dict()
+ # With only the root group, the projection is the task graph, which is
acyclic here.
+ if len(task_group_dict) == 1:
+ return
+ cycles = [
+ f"{', '.join(cycle[:-1])} and {cycle[-1]}"
+ for task_group in task_group_dict.values()
+ for cycle in
task_group._find_dependency_cycles(group_dict=task_group_dict)
+ ]
+ if not cycles:
+ return
+ # Airflow 3.5 raises AirflowDagCycleException here instead; tracked at
+ # https://github.com/apache/airflow/issues/73678
+ warnings.warn(
+ f"Dag '{self.dag_id}': {'; '.join(cycles)} depend on each other in
a cycle. Cyclic TaskGroup "
Review Comment:
Capped. Each cycle now lists its first five members and then "and K more",
and the message lists at most ten cycles, then "(K more cycles not listed)". I
capped the cycle count too because a Dag with many TaskGroups, each in its own
small cycle, runs into the same limit. With both caps, the message length stays
bounded whatever the Dag's size.
`test_task_group_cycle_warning_caps_listed_ids` covers the setup/teardown shape
from the docs with 100 tasks between `create` and `delete`, and Dags with 11
and 12 bridged groups. dags.rst now mentions the caps.
---
Drafted-by: Claude Code (Opus 5.5); reviewed by @dheerajturaga before posting
##########
airflow-core/src/airflow/dag_processing/dagbag.py:
##########
@@ -491,7 +496,23 @@ def bag_dag(self, dag: DAG):
:raises: AirflowDagCycleException if a cycle is detected.
:raises: AirflowDagDuplicatedIdException if this dag already exists in
the bag.
"""
- dag.check_cycle()
+ from airflow.sdk.exceptions import TaskGroupCycleDeprecationWarning #
noqa: SDK001
Review Comment:
Done, it's with the module-level imports now and keeps `# noqa: SDK001` on
that line.
---
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]
