Skip to content

Generate data models from supervisor schema for Go-SDK#68629

Open
jason810496 wants to merge 8 commits into
apache:mainfrom
jason810496:refactor/go-sdk/generate-data-models-from-supervisor-schema
Open

Generate data models from supervisor schema for Go-SDK#68629
jason810496 wants to merge 8 commits into
apache:mainfrom
jason810496:refactor/go-sdk/generate-data-models-from-supervisor-schema

Conversation

@jason810496

@jason810496 jason810496 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Why

Same as Java-SDK implementation, we should auto generate the data models that will directly be sent to the Coordinator IPC channel from task-sdk/src/airflow/sdk/execution_time/schema/schema.json. Actually, all the Lang-SDK should follow the generating data models from supervisor schema pattern to avoid the schema drift.


Was generative AI tooling used to co-author this PR?

@jason810496 jason810496 force-pushed the refactor/go-sdk/generate-data-models-from-supervisor-schema branch from f0a609a to 5939e8a Compare June 18, 2026 06:25
@jason810496 jason810496 self-assigned this Jun 22, 2026
@jason810496 jason810496 force-pushed the refactor/go-sdk/generate-data-models-from-supervisor-schema branch from 5939e8a to b5001f5 Compare June 22, 2026 05:51
…inator protocol

Outbound messages rely on a single chokepoint (EnsureType) to stamp the
wire "type" discriminator. A pointer body slipped through that switch
unstamped, producing a frame the supervisor could not dispatch; it now
stamps the pointee so the binding holds regardless of how a caller passes
the body. Supervisor error replies also lost their typed error code when
the detail payload was off-contract, degrading specific errors (e.g.
variable-not-found) into a generic one; the code is now recovered
independently so callers still see the right typed error.
@jason810496 jason810496 marked this pull request as ready for review June 22, 2026 10:54
@jason810496 jason810496 requested review from kaxil and uranusjr June 22, 2026 10:54
@jason810496 jason810496 added this to the Go SDK 1.0 milestone Jun 22, 2026

@uranusjr uranusjr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclosure: I don’t really understand Go tooling; the reviews are highly assisted by Claude.

Comment thread go-sdk/pkg/execution/genmodels/models.gen.go
Comment thread go-sdk/Justfile
The HasType flag and its wrapper struct recorded which parsed structs
declared a Type discriminator field, but discriminator emission now keys
off the schema's "type" const rather than the parsed struct, so the flag
was set and never read. Collapsing the per-struct metadata to a plain
field slice removes the unread state with no change to generated output.
@uranusjr

Copy link
Copy Markdown
Member

According to Claude:

The defaults.gen.go approach addresses inbound decoding only. The outbound encoding problem we discussed is still present: GetPreviousTI.MapIndex remains int with omitempty, so sending GetPreviousTI{MapIndex: 0} to the supervisor will silently drop the 0, and the supervisor will apply its default (-1), incorrectly treating a map_index=0 request as unmapped.

The minimal fix is to add a pass in gen/main.go that rewrites any struct field in models.gen.go where:

  • the Go type is a concrete integer (int), and
  • the schema property has a non-zero default (i.e., defaultLiteral would produce a seed for it)

...from int to *int, and strips omitempty from the tag (since with a pointer, nil already means "absent" and omitempty would be redundant and confusing).

With this type, the call site passes &mapIndex to get it encoded, or nil to omit it. The supervisor receives the explicit 0 and treats it as index 0, not unmapped.

defaults.gen.go would then also need to change: GetPreviousTI.DecodeMsgpack would use a *int pre-seed, which means &-1 as the default pointer. The alias constructor becomes alias{MapIndex: intPtr(-1)} (with a small helper, or inline with new(int) and assignment). Or, since *int + absent key decodes to nil, the decode-side default for GetPreviousTI.MapIndex is just nil — and the call site treats nil the same as -1. That's actually the cleaner outcome: the Go SDK sends nil to mean "unmapped" (which the supervisor interprets as its -1 default), and sends &0 to mean index 0. No decode-side seeding needed at all for that field.

The change to gen/main.go would sit inside writeDefaults (or a new pre-pass before it): scan structs[structName] for int-typed fields whose schema property has a non-zero default, emit an AST rewrite of models.gen.go that changes the field type to *int and strips ,omitempty from the tag. That rewrite would be analogous to the dead-typedef stripping already done in parseModels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Use Supervisor Schema to generate Go-SDK Data Models

2 participants