dheerajturaga opened a new issue, #73678:
URL: https://github.com/apache/airflow/issues/73678
### Summary
Starting with **Airflow 3.3.1**, the Grid and Graph views return **HTTP
500** for Dags whose TaskGroups have dependencies that form a cycle *when each
group is treated as a single unit*, even though the task-level graph is
acyclic. These Dags parse, schedule and run normally, and they rendered in the
UI up to and including 3.3.0.
One proposed change (#73087) would reject these Dags at parse time. That is
also a breaking change for existing Dags, this time with an import error.
Before choosing a direction, I'd like community input on:
- how to handle the regression,
- whether these Dags should be allowed at all,
- how any change should be rolled out and communicated.
I'm affected by this on my own Dags as well.
### How we got here
- #69933 (backported to `v3-3-test` in #70591, released in **3.3.1**) fixed
the group-level topological sort used by Grid/Graph (closes #65291). Before
that change, the sort ignored group-to-group and cross-group edges and silently
rendered an arbitrary order. It now takes those edges into account, so a
pre-existing group-level cycle raises `ValueError("A cyclic dependency occurred
in dag: ...")`, which surfaces as a 500 from the Grid and Graph endpoints.
- #72822 tried to keep these Dags renderable in the UI by falling back to
strongly-connected-component ordering. It was closed after review comments
suggested that cyclic TaskGroup dependencies should be treated as invalid.
Those comments noted that planned TaskGroup features (task loops, dynamic
TaskGroups, "retry/clear this TaskGroup", "wait for TaskGroup completion")
could have undefined behaviour on these Dags.
- #73087 (open) proposes moving the failure to parse time with an import
error that names the offending TaskGroup and the nodes involved in the cycle.
Another user raised concerns there about doing this in a patch release without
following the deprecation policy.
### Affected Dag shapes
A task with no upstream task *inside its own group* counts as a **root** of
that group, even when a task outside the group provides its only upstream.
Group-level edges are built from those roots.
**1. Sibling groups with dependencies in both directions:**
```python
from airflow.providers.standard.operators.empty import EmptyOperator
from airflow.sdk import DAG, TaskGroup
with DAG("tg_cycle", schedule=None):
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")
a1 >> b1 # group1 -> group2
b2 >> a2 # group2 -> group1
```
**2. Two tasks in one group bridged by a task outside it:**
```python
with DAG("tg_bridged", schedule=None):
with TaskGroup("g"):
a = EmptyOperator(task_id="a")
b = EmptyOperator(task_id="b")
ext = EmptyOperator(task_id="ext")
a >> ext >> b # g -> ext -> g
```
Both are acyclic at the task level. This pattern is common when TaskGroups
are used for logical or visual grouping (for example one group per database
schema) rather than as execution units. Users report hitting it when upgrading
from 2.x to 3.3.1.
### Impact by version
| Version | Parsing / scheduling | Grid / Graph view |
|---|---|---|
| ≤ 3.3.0 | Works | Renders, but group order can be wrong |
| 3.3.1, 3.3.2 | Works | **HTTP 500** |
| With #73087 | **Import error**, Dag not loaded | N/A |
### Open questions for the community
1. **Should these Dags be allowed at all?**
- Against allowing them: group-level features need an unambiguous group
ordering (see the discussion in #72822).
- For allowing them: many users rely on TaskGroups purely as a visual or
logical grouping, and the task graph itself is valid.
2. **How should the change be rolled out, given the [deprecation
policy](https://airflow.apache.org/docs/apache-airflow/stable/release-process.html#deprecation-policy)?**
Options include:
- Keep allowing them permanently, and make Grid/Graph tolerate
group-level cycles (e.g. the approach in #72822).
- Hard-reject at parse time in a patch release (#73087 as it stands).
- Emit a deprecation warning / Dag warning at parse time now, keep
Grid/Graph working (e.g. with a fallback ordering), and hard-reject in a later
minor or major release.
- Hard-reject only from 3.4.0, and fix the 500 in 3.3.x separately.
3. **What should 3.3.x users do in the meantime?** Currently the only
workaround is to restructure the Dag: remove the cycle or flatten the affected
TaskGroups. Tasks keep running and stay reachable through the REST API.
4. **Communication:** a `significant` newsfragment exists in #73087. Should
there also be a known-issue note in the 3.3.1/3.3.2 release notes and/or an
announcement on the dev/users mailing list?
### Related
- #65291: original issue (Grid view not in topological order)
- #69933 / #70591: sort fix that exposed the cycle (released in 3.3.1)
- #72822: UI fallback approach (closed)
- #73087: parse-time rejection (open)
### Are you willing to submit PR?
- [X] Yes. I'm willing to submit PRs for whichever direction the community
agrees on.
---
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]
