Uh oh!
There was an error while loading. Please reload this page.
TS SDK: replace registerTask with Dag and registerDags - #71144
Conversation
dab6b7b to
1676164Compareuranusjr
commented
Aug 6, 2026
What’s the plan to do the external vs pure TS dags after this? |
Along with #71213, the user should set the Additionally, we can introduce a |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
6bfacba to
cedbd25Compare
jason810496
left a comment
There was a problem hiding this comment.
Thanks kaxil for the review, I just resolved all the comments.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
uranusjr
commented
Aug 10, 2026
Maybe we can do something like this instead constdag=newDag(...);// Add tasks...constregistry=newDagRegistry(dag);registry.serve();Or, even more similar to Java SDK, |
8427b72 to
0b864b0CompareThere was a problem hiding this comment.
Thanks TP and Kaxil for the review!
I went with the following interface.
constdag1=newDag(...);// Add tasks...// Add Dags at constructor levelconstregistry=newDagRegistry(dag1,dag2);// Register Dag incrementally registry.register(dag3,dag4);awaitserveDags(registry);// should only be called onceThe reason I used registerDags (I agreed serveDags is a better naming) in the first place is just to remove the "coordinator" term from the user interface and keep the entrypoint as simple as possible for the user.
I prefer to let serveDags(registry) do the final socket works.
For new Server(registry).serve() interface, IMO, we don't need to expose that much details to users. In the case of Python Dag world, we don't even have the user interface to register or serve the Dags at all.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
shivaam
commented
Aug 11, 2026
Just curious about the longer-term direction. Do we expect tasks to become callable, similar to Python TaskFlow? These calls would build the Dag and return typed task outputs rather than execute the handlers. Is something like this planned for future native TypeScript Dags, or is the current TaskRef plus inputs design intended to be the final authoring interface? One option would be for dag.task() to return a callable TaskDefinition<TInputs, TOutput>. Calling that definition would record the input bindings and return a TaskOutput. |
jason810496
commented
Aug 11, 2026
Yes, we will support the TaskFlow-style syntax to define the task dependencies as mentioned in #69288 (comment).
This is what the |
shivaam
left a comment
There was a problem hiding this comment.
Thanks, looks good overall. Two potential nits.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (2)
ts-sdk/src/sdk/dag.ts:205
#validateOptionsclaims unknown option keys are rejected, but it usesObject.keys(...)which ignores symbol keys. Callers can pass symbol-keyed options that will be silently accepted/ignored, contradicting the intended “typo fails at import time” behavior.
#validateOptions(taskId: string, options: TaskOptions): void {
const value: unknown = options;
if (!isPlainRecord(value)) {
throw new Error(`options for Dag "${this.dagId}" task "${taskId}" must be an object`);
}
ts-sdk/src/cli/pack.ts:166
readBundleManifestsays it validates the untrusted metadata line “down to the task-id element”, but it does not validate the Dag ID key at all, and it only checks task IDs are non-empty strings. A bundle that prints the sentinel line itself (malicious or buggy) can slip invalid Dag/task IDs through pack-time validation and generate bad metadata.
// The line is whatever the bundle printed and nothing downstream re-validates
// it, so check each Dag entry down to the task-id element.
for (const [dagId, dag] of Object.entries(manifest.dags)) {
if (dag == null || !isTaskIdList(dag.tasks)) {
throw new Error(
Uh oh!
There was an error while loading. Please reload this page.
registerTask bound one handler at a time to a Dag/task pair declared in a Python stub file, and left no object that could later carry a natively declared TypeScript Dag. Reworking only the authoring interface now means native Dag support and TaskFlow-style data passing can arrive without a second breaking change for users: a Dag instance keeps its spec and every task's handler, spec and declared inputs, which is what a future serialize() needs to emit the serialized Dag JSON. Producing that JSON stays out of scope, so Dag parsing still answers with no serialized Dags. The coordinator wire protocol and the bundle manifest shape are unchanged.
A Dag author has no reason to know Airflow's coordinator exists, and the
three-step shape left two steps that failed quietly when one was missed: a Dag
that was built but never registered was dropped from the packed bundle, and its
task instances showed up as *removed* at runtime with only a warning in the task
log. Folding the runtime handoff into registerDags removes one of those steps
outright; the packer now names the Dags that fell through the other.
Reserved options were the second quiet failure. `inputs` read as though it
declared a dependency, so a user could conclude the value was wired and call
getXCom without a taskId, which reads the running task's own XCom and returns
null without failing the task. Saying plainly that these fields are inert, and
accepting only `{}` until they are real, turns a wrong value into a compile
error. Generated specs will be all-optional types that `{}` still satisfies, so
filling them in later cannot break a call site.
Empty-Dag handling followed airflow-go-pack rather than diverging from it: one
placeholder Dag should not block a bundle build in TypeScript and not in Go.An entrypoint named for registration gave no hint that it also connects sockets and blocks until Airflow's supervisor has its terminal frame, so the name now says what it does. Naming it after serving also matches the Java and Go SDKs, where the socket-owning half is kept separate from the collection of Dags it serves. Making that collection an object the author builds removes the process-wide state the previous shape needed: constructing a Dag no longer registers it as a side effect, and the runtime dispatches through the registry it is handed rather than a module-level singleton. A registry owns no sockets and starts nothing, so a bundle's handlers can be dispatched from a unit test with no runtime in scope. Leaving it in the authoring layer also keeps the dependency between the two pointing one way; a serve() method on the registry would have made it circular. Warning about a Dag that was built but never served was the only thing the process-wide tracking bought, so it goes too. As in the Go and Java SDKs, a Dag left out of the bundle is simply absent from it, and its tasks are marked removed at runtime.
A serve that never got off the ground is not a serve, but the one-shot latch was set before the runtime ran and never released, so a bundle that failed on its socket arguments reported "already called" on the next attempt and hid the real cause. The latch also has to outlive a duplicated dependency: what must not happen twice is one process connecting two pairs of sockets, and a workspace that resolves two copies of the package gives each its own module state, so both would believe they were first. Keying it globally rather than per module makes them agree. Objects from a second copy still cannot be used, because both classes read private state keyed to the class that declared it. They can be recognised though, so a genuine Dag or registry from another copy is now reported as that rather than as the wrong type.
Nothing downstream re-validates the metadata line: the reader in _bundle_metadata.py only checks that the document is a mapping, so a schema version that is merely truthy rather than a non-empty string was rendered into the bundle verbatim and travelled all the way to Airflow. A bundle that printed a bare null crashed the packer with a raw TypeError from reading a field off it, which reads as a bug in airflow-ts-pack rather than as a report about the bundle it was given.
Listing a registry's tasks and Dags is how the runtime dispatches and how the packer builds a manifest; a Dag author declares tasks and serves them, and never needs to ask a registry what is in it. Exporting DagRegistry made both methods public API that has to be kept working forever, for no one's benefit. Serving another copy's registry cannot work either, for the reason registering another copy's Dag cannot: the lookups read private state keyed to the class object of the copy that defined it. The check says that now instead of reporting a genuine registry as the wrong type.
These read as replies to review threads rather than as something useful in an editor hover: the rationale came first, the plain statement of what the option does came last or not at all, and each declaration ordered it differently. "Inert" was doing work that "not used yet" does plainly. Every one now says what it is, what it does not do yet, then what it is for, in that order.
The brand is one mechanism with two users and had no owner between them: the shared error text sat in dag.ts, where the coordinator had to import it to describe a registry, and registry.ts carried a "for the reason given on Dag's" pointer that would rot away from what it points at. The comments introduced alongside it are cut back to what the code does not already say.
validateKey ran inside the Dag constructor and dag.task(), part of the bundle module's top level that also re-runs on every task-execution-runtime startup, so a bundle's keys were re-checked on every task attempt instead of once. Move the check into buildBundleManifest, which only runs for airflow-ts-pack's --airflow-metadata query, so it happens once at pack time.
feff817 to
07f2533CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The throwing check from apache#69400, moved to manifest build time by apache#71144, still fails packing outright when the client-side ID rules drift from the server. Follow the direction settled in apache#69965: warn at build time, let the server validate authoritatively.
Why
registerTask({ dagId, taskId }, handler)binds one handler to a Python stub Dag/task pair at a time and leaves no object that can carry native TypeScript Dag declaration later (#69288). This PR changes only the user-facing authoring interface so it can grow into TaskFlow and native Dag support without another breaking change.How
Please noted that the spec at both Dag and Task level arguments (
DagSpec,TaskSpec) and the taskinputsare no-op, I intentionally placeholder them in this PR to prevent further breaking change.new Dag(dagId, spec?)takes a positional id plus an optional trailing spec object.dag.task(taskId, handler, options?)returns aTaskRefhandle, with placeholderinputsandspecas named options (the further native Dag:{ inputs: { extracted }, spec: { retries: 2 } }) so the future options need no new parameter breaking change. Unknown option keys are rejected, so a typo fails at import time instead of being silently ignored.DagSpecandTaskSpecareRecord<string, never>while they are placeholders, so a field that would be silently dropped does not compile.dag.taskIdslists the attached task IDs, andregistry.listDags()lists the Dags a bundle serves with their tasks, so a user can assert their handlers match the Python@task.stubnames.Daginstance retains its spec and each task's(taskId, handler, spec, inputs), so a futureserialize()can produce the serialized Dag JSON forDagFileParsingResult.serialized_dags.registerTask,listRegisteredTasks, andTaskRegistrationare replaced with a publicDagRegistry— plusserveDags(registry)as the entrypoint.startCoordinatorstays out of the public API.airflow-ts-packwarns instead of failing when a registered Dag has no tasks, matchingairflow-go-pack. A Dag left out of the registry is simply absent from the bundle and its tasks are marked removed at runtime.Was generative AI tooling used to co-author this PR?