Skip to content

Share Dag cycle detection between the Task SDK and core - #74004

Merged
jason810496 merged 0 commit into
jason/lang-sdk-e2e/02-importer-registryfrom
jason/lang-sdk-e2e/02b-shared-dag-cycle-detection
Oct 1, 2026
Merged

jason810496 merged 0 commit into
jason/lang-sdk-e2e/02-importer-registryfrom
jason/lang-sdk-e2e/02b-shared-dag-cycle-detection

Conversation

@jason810496

@jason810496 jason810496 commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Stack (bottom to top): #73841, #74004, #73842, #73843, #73844, #73845, #73846, #73847

Why

Cycle detection only runs for Dags that go through the Python SDK's DAG object. A Dag from another language SDK reaches core already serialized, so nothing rejects a cycle in it.

How

Core cannot reuse DAG.check_cycle as it is: it is a method on the authoring class and reads task objects, while core holds serialized data it should not hydrate just to run this check.

  • The traversal moves to shared/dagnode, which task-sdk and airflow-core already depend on.
  • The graph is described by callbacks, not node objects, so a caller answers from whatever data it holds.
  • It returns the offending task id instead of raising. AirflowDagCycleException lives in the Task SDK, and importing it here would invert the dependency, so each caller raises its own error.
from airflow._shared.dagnode.cycle import detect_cycle

downstream = {"extract": ["load"], "load": ["extract"]}
detect_cycle(downstream, downstream.__getitem__)  # "load", or None when there is no cycle

What

  • detect_cycle(node_ids, downstream_of) in shared/dagnode/src/airflow_shared/dagnode/cycle.py, with its unit tests.
  • DAG.check_cycle() delegates to it and keeps its message, Cycle detected in Dag: <dag_id>. Faulty task: <task_id>.

No behavior change: the walk still follows downstream edges only and stays iterative, and the existing TestCycleTester tests pass unchanged. No newsfragment, since nothing user-visible changes.

How to test

uv run --project task-sdk pytest task-sdk/tests/task_sdk/definitions/test_dag.py -k cycle -xvs

shared/dagnode/tests/dagnode/test_cycle.py covers detect_cycle itself.


Was generative AI tooling used to co-author this PR?
  • Yes, with help of Claude Code (Claude Opus 4.5) following the guidelines

@jason810496
jason810496 added this pull request to stack #74005 October 1, 2026 03:04
@jason810496
jason810496 force-pushed the jason/lang-sdk-e2e/02-importer-registry branch from 57c9a34 to a5d284b Compare October 1, 2026 12:41
@jason810496
jason810496 force-pushed the jason/lang-sdk-e2e/02b-shared-dag-cycle-detection branch from e14a584 to 7caeacb Compare October 1, 2026 12:41
@jason810496
jason810496 removed this pull request from stack #74005 October 1, 2026 13:17
@jason810496
jason810496 merged commit 5c20182 into jason/lang-sdk-e2e/02-importer-registry Oct 1, 2026
@jason810496
jason810496 force-pushed the jason/lang-sdk-e2e/02-importer-registry branch from a5d284b to c24e6fa Compare October 1, 2026 13:18
@jason810496
jason810496 force-pushed the jason/lang-sdk-e2e/02b-shared-dag-cycle-detection branch from 7caeacb to 5c20182 Compare October 1, 2026 13:18
@jason810496
jason810496 deleted the jason/lang-sdk-e2e/02b-shared-dag-cycle-detection branch October 1, 2026 13:18
@jason810496
jason810496 restored the jason/lang-sdk-e2e/02b-shared-dag-cycle-detection branch October 1, 2026 13:36
@jason810496

Copy link
Copy Markdown
Member Author

Replaced by #74034. GitHub marked this PR as merged into a stack branch when the stack was reordered; nothing was merged into main.

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.

1 participant