You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jason810496 (via GitHub)" <gi...@apache.org> on 2026/10/06 06:12:22 UTC
[PR] TS SDK: Step over an empty nested task group to the group holding it [airflow]
jason810496 opened a new pull request, #74323:
URL: https://github.com/apache/airflow/pull/74323
## Why
Two bugs in how a native TypeScript Dag's task-group edges reach the task graph, both found while
checking the Java SDK's serializer against Python's.
An ordering edge out of a group that holds no tasks expanded onto nothing when that group was
nested inside another. The walk followed the group edges drawn around it and stopped there, while
Python's `find_leaves` takes one more step and stands the group in for its parent. The serialized
`task_group` object still recorded the edge, so the UI drew a constraint the scheduler never
enforced.
## Example
```typescript
const outer = dag.taskGroup("outer");
place(outer, "t");
const inner = outer.taskGroup("inner"); // holds nothing
const load = place(dag, "load");
inner.before(load);
```
| | `outer.t` downstream |
| --- | --- |
| Python | `["load"]` |
| Before | nothing at all |
| After | `["load"]` |
## How
- The upstream side of the step-over walk falls back to the group holding the empty one, matching
Python's `find_leaves`. A group standing downstream gets no such fallback, because Python gives it
none either.
- `scripts/ci/lang_sdk_serialization/test_dags.yaml` gains `conformance_nested_group_edges`, which
covers that shape plus a group edge whose ends sit inside another group. Without the fix the shared
check reports `task outer.t: downstream_task_ids is missing, Python writes ['load']`.
A cron schedule being read in UTC is now written down rather than left implicit. Python takes a Dag's
timezone from `start_date`, but `startDate` here is a `Date`, which is an instant and carries no
zone, so there is nothing to take. No behaviour changes.
---
##### Was generative AI tooling used to co-author this PR?
- [x] Yes, with help of Claude Code Opus 5 following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TS SDK: Step over an empty nested task group to the group holding it [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 merged PR #74323:
URL: https://github.com/apache/airflow/pull/74323
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TS SDK: Step over an empty nested task group to the group holding it [airflow]
Posted by "phanikumv (via GitHub)" <gi...@apache.org>.
phanikumv commented on code in PR #74323:
URL: https://github.com/apache/airflow/pull/74323#discussion_r4192612118
##########
ts-sdk/tests/coordinator/serde.test.ts:
##########
@@ -726,6 +726,37 @@ describe("serializeDag", () => {
expect(tasks.get("before")?.["downstream_task_ids"]).toEqual(["after"]);
});
+ it("steps over an empty nested group to the tasks of the group holding it", () => {
+ // Python's find_leaves falls back to the parent group, so an edge out of
+ // an empty nested group leaves from whatever the parent finishes with.
+ const dag = new Dag("d");
+ const outer = dag.taskGroup("outer");
+ place(outer, "t");
+ const inner = outer.taskGroup("inner");
+ const load = place(dag, "load");
+ inner.before(load);
+
+ const tasks = taskMap(serializeDag(dag, "", ".") as Json);
+ expect(tasks.get("outer.t")?.["downstream_task_ids"]).toEqual(["load"]);
+ });
+
+ it("resolves a group's endpoints against the edges drawn before it", () => {
+ // outer's only root once inner runs before t, so the edge into outer
+ // reaches nothing else.
+ const dag = new Dag("d");
+ const extract = place(dag, "extract");
+ const outer = dag.taskGroup("outer");
+ const t = place(outer, "t");
+ const inner = outer.taskGroup("inner");
+ place(inner, "i");
+ inner.before(t);
+ extract.before(outer);
+
+ const tasks = taskMap(serializeDag(dag, "", ".") as Json);
+ expect(tasks.get("extract")?.["downstream_task_ids"]).toEqual(["outer.inner.i"]);
Review Comment:
The second test, "resolves a group's endpoints against the edges drawn before it", doesn't exercise this new branch. It has no empty group, so the fallback is never reached and it passes without the fix
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TS SDK: Step over an empty nested task group to the group holding it [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #74323:
URL: https://github.com/apache/airflow/pull/74323#discussion_r4192681517
##########
ts-sdk/tests/coordinator/serde.test.ts:
##########
@@ -726,6 +726,37 @@ describe("serializeDag", () => {
expect(tasks.get("before")?.["downstream_task_ids"]).toEqual(["after"]);
});
+ it("steps over an empty nested group to the tasks of the group holding it", () => {
+ // Python's find_leaves falls back to the parent group, so an edge out of
+ // an empty nested group leaves from whatever the parent finishes with.
+ const dag = new Dag("d");
+ const outer = dag.taskGroup("outer");
+ place(outer, "t");
+ const inner = outer.taskGroup("inner");
+ const load = place(dag, "load");
+ inner.before(load);
+
+ const tasks = taskMap(serializeDag(dag, "", ".") as Json);
+ expect(tasks.get("outer.t")?.["downstream_task_ids"]).toEqual(["load"]);
+ });
+
+ it("resolves a group's endpoints against the edges drawn before it", () => {
+ // outer's only root once inner runs before t, so the edge into outer
+ // reaches nothing else.
+ const dag = new Dag("d");
+ const extract = place(dag, "extract");
+ const outer = dag.taskGroup("outer");
+ const t = place(outer, "t");
+ const inner = outer.taskGroup("inner");
+ place(inner, "i");
+ inner.before(t);
+ extract.before(outer);
+
+ const tasks = taskMap(serializeDag(dag, "", ".") as Json);
+ expect(tasks.get("extract")?.["downstream_task_ids"]).toEqual(["outer.inner.i"]);
Review Comment:
Thanks for the review, addressed in the latest commit.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TS SDK: Step over an empty nested task group to the group holding it [airflow]
Posted by "luc-pimentel (via GitHub)" <gi...@apache.org>.
luc-pimentel commented on PR #74323:
URL: https://github.com/apache/airflow/pull/74323#issuecomment-6398840694
I noticed the TypeScript serializer and Python disagree on one shape and I am wondering whether that is the intended behavior or whether an issue or PR should be created for it.
Group `outer` holds task `t` and an empty nested group `outer.spare`; `extract` and `load` are root tasks. With `spare.before(load)` declared first and `extract.before(spare)` second, Python draws `outer.t -> load`, while the TypeScript SDK draws `extract -> load`. The in-order shape and the parent fallback agree on both sides.
| Edges in declaration order | Python 3.4.0b2 and main | TypeScript 1.0.0-beta1 and main |
| --- | --- | --- |
| `extract.before(spare)`, `spare.before(load)` | `extract -> load` | `extract -> load` |
| `spare.before(load)`, `extract.before(spare)` | `outer.t -> load` | `extract -> load` |
| `extract.before(outer)`, `spare.before(load)` | `extract -> t -> load` | `extract -> t -> load` |
Python resolves each `>>` when it is written, so `spare` has no upstream yet at that point, while `buildDagGraph` in `ts-sdk/src/sdk/dag-graph.ts` indexes every order edge before expanding any. The Go SDK gives the same result as TypeScript and documents it as the shared rule; the Java SDK, from reading `DagDef.kt`, follows Python.
Is this the intended behavior for the TypeScript SDK, or should an issue or PR be created for it? Happy to send one if so.
Checked with the conformance harness's `compare_dag` against the Python serializer from main and the released 3.4.0b2.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org