Skip to content

Deprecate cyclic TaskGroup dependencies - #73746

Open
dheerajturaga wants to merge 12 commits into
apache:mainfrom
dheerajturaga:deprecate-cyclic-taskgroup-dependencies
Open

dheerajturaga wants to merge 12 commits into
apache:mainfrom
dheerajturaga:deprecate-cyclic-taskgroup-dependencies

Conversation

@dheerajturaga

@dheerajturaga dheerajturaga commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Dags whose TaskGroups depend on each other in a cycle, when each group is treated as a single unit, parse and run today even though planned TaskGroup features need an unambiguous order between groups. Following the dev list discussion, this deprecates them: they keep working, users are warned now, and Airflow 3.5 is planned to reject them at parse time. This replaces the immediate parse-time rejection proposed in #73087.

Treating a group as a single unit means a dependency into or out of any of its tasks counts as a dependency of the whole group. This covers sibling groups that depend on each other in both directions, and a path that leaves a TaskGroup and comes back into it, even when the tasks inside the group are also ordered directly:

with TaskGroup("group"):
    a = EmptyOperator(task_id="a")
    b = EmptyOperator(task_id="b")
    a >> b

a >> EmptyOperator(task_id="bridge") >> b  # group -> bridge -> group

To remove the cycle, move bridge into the group, or b out of it.

Parsing a Dag with such a cycle now:

  • issues a TaskGroupCycleDeprecationWarning from DAG.check_cycle(), so CI can catch it, for example with pytest -W error::airflow.sdk.exceptions.TaskGroupCycleDeprecationWarning;
  • records a new task group cycle Dag warning, shown in the UI's Dag warnings and returned by GET /api/v2/dagWarnings.

The message names only the tasks and TaskGroups on the cycle:

Dag 'etl': group1 and group2 depend on each other in a cycle. Cyclic TaskGroup dependencies are deprecated and will fail Dag parsing in Airflow 3.5. See "Cyclic TaskGroup dependencies" in the docs.

The Grid/Graph HTTP 500 for these Dags is fixed separately in #73724. The parse-time rejection in 3.5 is tracked in #73678.

related: #73678
Discussion: https://lists.apache.org/thread/sossl7b2w2ftyk4028qrhps2tcdxj2px
Lazy consensus: https://lists.apache.org/thread/b6h120jw7tv1l1zf1ovwrok2zk4hyvwc

image
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5.5)

Generated-by: Claude Code (Opus 5.5) following the guidelines

@dheerajturaga

Copy link
Copy Markdown
Member Author

@bbovenzi would like your thoughts on the UI, would like to get this in 3.4 as agreed in Lazy consensus https://lists.apache.org/thread/5fpwlxg4y6jhyrw7n2m3jz3o7t6xvhlk

cc: @ashb

@bbovenzi

bbovenzi commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Makes sense to me UI-wise. I am working on improving our warning banner UX but having another category doesn't get in the way of that

@dheerajturaga
dheerajturaga force-pushed the deprecate-cyclic-taskgroup-dependencies branch from e08d1f1 to a8f63d7 Compare October 6, 2026 22:30
Comment thread task-sdk/src/airflow/sdk/definitions/taskgroup.py Outdated
Comment thread airflow-core/src/airflow/dag_processing/dagbag.py Outdated
Comment thread airflow-core/newsfragments/73746.significant.rst Outdated
Comment thread airflow-core/docs/core-concepts/dags.rst
Comment thread task-sdk/src/airflow/sdk/definitions/dag.py
@dheerajturaga
dheerajturaga force-pushed the deprecate-cyclic-taskgroup-dependencies branch from 29b252b to 36958b3 Compare October 8, 2026 01:47
Comment thread task-sdk/src/airflow/sdk/definitions/dag.py Outdated
Comment thread airflow-core/src/airflow/dag_processing/dagbag.py Outdated
@dheerajturaga
dheerajturaga force-pushed the deprecate-cyclic-taskgroup-dependencies branch from 36958b3 to 7ecffd6 Compare October 8, 2026 22:01
@dheerajturaga
dheerajturaga requested a review from kaxil October 9, 2026 14:48
Comment thread airflow-core/src/airflow/dag_processing/dagbag.py Outdated

@kaxil kaxil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. Feel free to ignore these:

  • test_dag_topological_sort_task_group_cycle builds the a >> bridge >> b shape and calls dag.check_cycle() bare, so it now emits TaskGroupCycleDeprecationWarning without asserting it, and the docstring's "although check_cycle passes" is out of date. Wrapping the call in pytest.warns(TaskGroupCycleDeprecationWarning) would make that explicit.
  • The function-body from airflow.sdk import TriggerRule in test_dag_add_task_checks_trigger_rule is redundant now that TriggerRule is in the module-level imports.

Dags whose TaskGroups depend on each other in a cycle, when each group
is treated as a single unit, parse and run today, but planned TaskGroup
features need an unambiguous order between groups. Following the dev
list discussion, users are warned about these Dags now so they have a
release to restructure them before Airflow 3.5 rejects them at parse
time.
Following dev list feedback on the lazy consensus, a Dag in which a path
leaves a TaskGroup and comes back into it is cyclic when the group is
treated as a single unit, even if the tasks inside the group are also
ordered directly. The warning only looked at edges into a group's roots,
so these Dags would have parsed silently in 3.4 and then failed in 3.5
without a deprecation period.
The warning category is defined only in the Task SDK, and bag_dag already
works on SDK Dag objects in the Dag processor, so the import is deliberate
rather than a new scheduler or API server dependency on the SDK.
The TaskGroup cycle check walked each group through its iterator, and a
mapped group's iterator rejects a task with trigger_rule="always" even
when the group is expanded over a literal list. A Dag with no cycle at
all that parsed before the deprecation became an import error.
The warning was recorded before the Dag was added to the bag, so a
duplicate Dag id that the bag rejected could give the bagged Dag a
cycle warning it does not have, or remove the one it does. Folder-wide
bags such as `airflow dags reserialize` write these warnings to the
database.
A setup and teardown pair in a TaskGroup around work outside it is
flagged on purpose, and users with that layout need to know before
Airflow 3.5 rejects it. Lang SDK Dags are not checked yet, and turning
the warning into an error only catches Dags loaded through a DagBag.
A Dag warning is stored in a Text column, which MySQL limits to 64 KB.
Naming every member of every cycle could exceed that for a large Dag, and
the failed insert shares a transaction with the serialized Dag writes, so
none of the file's parse results would be saved.
Importing airflow.sdk.exceptions does not load the DagBag module, so there
is no import cycle to defer.
The same strongly connected component search now lives in the shared
TaskGroupMixin, which TaskGroup inherits, so a second copy would only drift.
The noqa hid a runtime airflow.sdk import from the check that keeps core
from gaining Task SDK dependencies. DagBag already depends on
airflow.sdk.exceptions for AirflowDagCycleException, so sharing that import
adds no new dependency and leaves the file's recorded count unchanged.
The bridged TaskGroup in that test is now a deprecated cycle, so check_cycle
warns there instead of passing silently, and its docstring no longer held.
TriggerRule is already imported at module level for the mapped group test.
@dheerajturaga
dheerajturaga force-pushed the deprecate-cyclic-taskgroup-dependencies branch from 9330730 to 264b03d Compare October 11, 2026 03:02

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants