Uh oh!
There was an error while loading. Please reload this page.
Go-SDK: Dag/Task specs and downstream wiring in the authoring API - #67155
Draft
jason810496 wants to merge 7 commits into
Draft
Go-SDK: Dag/Task specs and downstream wiring in the authoring API#67155jason810496 wants to merge 7 commits into
jason810496 wants to merge 7 commits into
Conversation
1 task
Closed
1 task
jason810496 added a commit
to jason810496/airflow
that referenced
this pull request
May 27, 2026
…dings - Omit map_index from SetXCom for unmapped task instances and carry it as *int from api.TaskInstance through to SetXComMsg. Fixes a compile-time mismatch and matches the supervisor's "absent vs -1" semantics for unmapped tasks. - Honor context cancellation in CoordinatorComm.Communicate by running the request send in a goroutine and selecting on ctx.Done(), so a blocked supervisor socket no longer wedges the caller. The underlying connection is left alone on cancel to avoid poisoning future writes with a stale deadline or leaving a partial length-prefixed frame on the wire. - Decode ConnectionResult.Login and Password as *string via a new mapStringPtr helper so an explicitly empty credential round-trips through the coordinator client instead of being silently treated as absent. sdk.Connection already encodes the distinction. Adds regression tests for each case. go-sdk: drop unused details field from CoordinatorClient CoordinatorClient stored a *StartupDetails it never read, and the companion PRs (apache#67155, apache#67318) that also construct or pass details into NewCoordinatorClient never read c.details either — they work directly off the local *StartupDetails inside RunTask. Keeping the field as API surface implies a contract the type does not honor, so remove it and let callers pass only the comm channel. go-sdk: unify ErrorResponse decoding in CoordinatorComm.Communicate The dispatcher response could carry an error in two places — the third element of a 3-tuple response frame, or as the body of a 2-tuple frame whose "type" is "ErrorResponse" — and Communicate inspected each path independently. The two branches diverged on nil-guarding decodeErrorResponse, which was easy-to-miss latent inconsistency: either path could grow a bug the other did not. Extract the source selection into errMapFromFrame so the decode and *ApiError construction live in one place. go-sdk: widen frame IDs to int64 end-to-end CoordinatorComm's request-id counter was atomic.Int64 (chosen so a long-running runtime cannot wrap), but the value was narrowed to int when stored in the pending map and IncomingFrame.ID. On 32-bit GOARCH that narrowing reintroduces the wraparound the int64 counter was meant to prevent, and the comment promising "wide enough to avoid wraparound" becomes architecture-dependent. Widen IncomingFrame.ID, the pending map key, encodeRequest, and the test fixtures to int64 so the no-wraparound guarantee holds on every supported GOARCH. The change is package-internal; IncomingFrame has no callers outside pkg/execution.
jason810496 added a commit
to jason810496/airflow
that referenced
this pull request
May 29, 2026
…dings - Omit map_index from SetXCom for unmapped task instances and carry it as *int from api.TaskInstance through to SetXComMsg. Fixes a compile-time mismatch and matches the supervisor's "absent vs -1" semantics for unmapped tasks. - Honor context cancellation in CoordinatorComm.Communicate by running the request send in a goroutine and selecting on ctx.Done(), so a blocked supervisor socket no longer wedges the caller. The underlying connection is left alone on cancel to avoid poisoning future writes with a stale deadline or leaving a partial length-prefixed frame on the wire. - Decode ConnectionResult.Login and Password as *string via a new mapStringPtr helper so an explicitly empty credential round-trips through the coordinator client instead of being silently treated as absent. sdk.Connection already encodes the distinction. Adds regression tests for each case. go-sdk: drop unused details field from CoordinatorClient CoordinatorClient stored a *StartupDetails it never read, and the companion PRs (apache#67155, apache#67318) that also construct or pass details into NewCoordinatorClient never read c.details either — they work directly off the local *StartupDetails inside RunTask. Keeping the field as API surface implies a contract the type does not honor, so remove it and let callers pass only the comm channel. go-sdk: unify ErrorResponse decoding in CoordinatorComm.Communicate The dispatcher response could carry an error in two places — the third element of a 3-tuple response frame, or as the body of a 2-tuple frame whose "type" is "ErrorResponse" — and Communicate inspected each path independently. The two branches diverged on nil-guarding decodeErrorResponse, which was easy-to-miss latent inconsistency: either path could grow a bug the other did not. Extract the source selection into errMapFromFrame so the decode and *ApiError construction live in one place. go-sdk: widen frame IDs to int64 end-to-end CoordinatorComm's request-id counter was atomic.Int64 (chosen so a long-running runtime cannot wrap), but the value was narrowed to int when stored in the pending map and IncomingFrame.ID. On 32-bit GOARCH that narrowing reintroduces the wraparound the int64 counter was meant to prevent, and the comment promising "wide enough to avoid wraparound" becomes architecture-dependent. Widen IncomingFrame.ID, the pending map key, encodeRequest, and the test fixtures to int64 so the no-wraparound guarantee holds on every supported GOARCH. The change is package-internal; IncomingFrame has no callers outside pkg/execution.
jason810496 added a commit
that referenced
this pull request
May 30, 2026
#67317) * go-sdk: Add concurrent-safe coordinator comms, log handler, and client Build the comm layer on top of the protocol primitives so subsequent runtime code has a single typed entry point for talking to the supervisor. CoordinatorComm runs a concurrent-safe dispatcher loop that fans inbound frames out to per-request reply channels keyed by a monotonic id, propagates context cancellation, and cleans up pending requests on SendRequest failure. SocketLogHandler streams slog records as structured JSON over the dedicated logs socket so the supervisor can demux task logs without parsing stderr. CoordinatorClient implements the sdk.Client surface (GetVariable honouring AIRFLOW_VAR_* overrides, GetConnection, XCom push/pull, deferral) by routing each method through the dispatcher and translating supervisor not-found responses into the SDK's sentinel errors. No server or task-runner loop is wired yet -- that lands in the next PR in this stack. * self-review: Address coordinator comms, client, and logger review findings - Omit map_index from SetXCom for unmapped task instances and carry it as *int from api.TaskInstance through to SetXComMsg. Fixes a compile-time mismatch and matches the supervisor's "absent vs -1" semantics for unmapped tasks. - Honor context cancellation in CoordinatorComm.Communicate by running the request send in a goroutine and selecting on ctx.Done(), so a blocked supervisor socket no longer wedges the caller. The underlying connection is left alone on cancel to avoid poisoning future writes with a stale deadline or leaving a partial length-prefixed frame on the wire. - Decode ConnectionResult.Login and Password as *string via a new mapStringPtr helper so an explicitly empty credential round-trips through the coordinator client instead of being silently treated as absent. sdk.Connection already encodes the distinction. Adds regression tests for each case. go-sdk: drop unused details field from CoordinatorClient CoordinatorClient stored a *StartupDetails it never read, and the companion PRs (#67155, #67318) that also construct or pass details into NewCoordinatorClient never read c.details either — they work directly off the local *StartupDetails inside RunTask. Keeping the field as API surface implies a contract the type does not honor, so remove it and let callers pass only the comm channel. go-sdk: unify ErrorResponse decoding in CoordinatorComm.Communicate The dispatcher response could carry an error in two places — the third element of a 3-tuple response frame, or as the body of a 2-tuple frame whose "type" is "ErrorResponse" — and Communicate inspected each path independently. The two branches diverged on nil-guarding decodeErrorResponse, which was easy-to-miss latent inconsistency: either path could grow a bug the other did not. Extract the source selection into errMapFromFrame so the decode and *ApiError construction live in one place. go-sdk: widen frame IDs to int64 end-to-end CoordinatorComm's request-id counter was atomic.Int64 (chosen so a long-running runtime cannot wrap), but the value was narrowed to int when stored in the pending map and IncomingFrame.ID. On 32-bit GOARCH that narrowing reintroduces the wraparound the int64 counter was meant to prevent, and the comment promising "wide enough to avoid wraparound" becomes architecture-dependent. Widen IncomingFrame.ID, the pending map key, encodeRequest, and the test fixtures to int64 so the no-wraparound guarantee holds on every supported GOARCH. The change is package-internal; IncomingFrame has no callers outside pkg/execution. * go-sdk: Fix SocketLogHandler WithAttrs/WithGroup ordering Pre-group attrs were silently re-qualified by any later WithGroup call, violating slog.Handler's "subsequently-added attrs only" contract. Snapshot the group prefix when WithAttrs is called and freeze each attr's key, so later WithGroup affects only record-level attrs and attrs added afterwards. Also document the deliberate flat dotted-key wire format on SocketLogHandler so it isn't switched to nested JSON objects without coordinating with the supervisor-side log parser. * fixup: Expand inline slog groups and lower late-reply log level SocketLogHandler dropped inline slog.Group attributes: a KindGroup value resolved to []slog.Attr that marshaled to "{}", so task code logging with slog.Group lost data. Handle now recurses into group values, expanding them into dotted keys (req.method) the same way WithGroup does, and skips empty groups per the slog.Handler contract. Also lower the "Discarding frame with no matching waiter" log from Warn to Debug: Communicate deliberately drops a waiter on cancel/timeout, so a late reply landing here is the expected deadline path, not a protocol bug.
1 task
jason810496force-pushed
the
refactor/go-sdk/dag-authoring-api
branch
from
June 3, 2026 03:14
c3a2584 to
fd01c17Compare4 tasks
jason810496force-pushed
the
refactor/go-sdk/dag-authoring-api
branch
from
July 21, 2026 03:17
1a47ba8 to
8b8f7f3Compare1 task
jason810496force-pushed
the
refactor/go-sdk/dag-authoring-api
branch
2 times, most recently
from
August 20, 2026 09:08
957b6fc to
80374f6CompareNative bundle registration bypasses Python-side validation, so invalid scheduling, identifiers, and task execution flags must be enforced in the Go runtime.
jason810496force-pushed
the
refactor/go-sdk/dag-authoring-api
branch
from
August 21, 2026 03:15
80374f6 to
0245dcbCompareDependency-only task declarations should not need an empty TaskSpec value.
jason810496 added a commit
to jason810496/airflow
that referenced
this pull request
Aug 24, 2026
Review found serde.go's serializeTaskGroup described as shipped present-tense fact; it only exists on the unmerged apache#67155/apache#70158 branches, same class of mistake ADR 7 already had to fix. Also split the ShortCircuit/Branch examples' func declarations out of the registration-statement fences to match ADR 7's presentation, since a package-level func decl mixed with := statements isn't valid Go as written.
jason810496 added a commit
to jason810496/airflow
that referenced
this pull request
Aug 25, 2026
Add ADRs for the Mixed Lang Dag interface already shipped via apache#70209, the proposed Native Dag interface (apache#67155/apache#70158), and the common task constructs a native Go author will need next (TaskGroup, ShortCircuit, Branch, TriggerDagRun). Recording the rationale here gives reviewers and future contributors a single reference for these tradeoffs instead of reconstructing them from scattered PR discussions.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DagImporterinterface)Why
The Go SDK needs a native Dag authoring surface that emits Airflow's serialized Dag format before AIP-85 wires Lang-SDK importers into core. Existing task registrations should remain concise when they do not need dependencies or a task spec.
Example
The existing bundle example remains unchanged, while a separate Dag demonstrates native authoring:
Task registration accepts these forms:
Dependencies come before the optional
TaskSpec, and both are optional. Invalid argument shapes panic during Dag registration.How
DagSpecandTaskSpecfromairflow-core/src/airflow/serialization/schema.json, with explicit generation rules for SDK-only and schema-default behavior.DagFileParseRequeston the coordinator socket and returnDagFileParsingResultwithout changing task execution.native_dagas a separate bundle example and leave Airflow E2E wiring unchanged untilDagImporteris connected.Was generative AI tooling used to co-author this PR?