Skip to content

Go SDK: vendor the schemas it generates from - #74047

Merged
henry3260 merged 2 commits into
apache:mainfrom
henry3260:go-sdk-vendor-schemas
Oct 2, 2026
Merged

henry3260 merged 2 commits into
apache:mainfrom
henry3260:go-sdk-vendor-schemas

Conversation

@henry3260

@henry3260 henry3260 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Why

go-sdk reached across the monorepo for both schemas it generates from — airflow-core's Dag
serialization schema and task-sdk's supervisor wire-schema snapshot. Two things follow from that:

  • A generated file is only explainable from a directory the published Go module does not contain, so
    go generate and two Go tests only work inside the monorepo. messages_test.go carried a skip for
    exactly this.
  • Every change to a Python schema becomes a change that has to carry regenerated Go with it, from
    whoever happened to be editing the Python — Go toolchain, network fetch of go-jsonschema and all.
    That is what check-go-sdk-generated-drift (Go SDK: bring the coordinator-protocol models back to the supervisor schema #73963) asked of them.

ts-sdk and java-sdk already vendor their copies (ts-sdk/schema/dag-schema.json,
java-sdk/sdk/schema/) and split the work into a cheap sync plus a generation check. go-sdk was
the only SDK still reaching out. Raised by @jason810496 in review of #73963.

What

Both schemas are vendored under go-sdk/schema/ and the generators read the copies. Regenerating
from them produces byte-identical output — no .gen.go changes in this PR.

That splits one question into two hooks:

  • sync-go-sdk-schemas (new, scripts/ci/prek/sync_go_sdk_schemas.py) — is the copy equal to
    its original? Copying is mechanical, so it copies for you and fails, the way
    sync-ts-sdk-dag-schema does. Pure file comparison: no Go toolchain, no network. Its failure
    message names what to regenerate, and for the supervisor schema it also names
    SupervisorSchemaVersion, which has to move when api_version does.
  • check-go-sdk-generated-drift — is the committed Go what the copy generates? It now watches
    the copies rather than the Python originals, since what to do about a new schema construct (a
    generator rule, an authoring exclusion) is a decision rather than a copy.

normalize_test.go and messages_test.go read the vendored copies, so the latter's skip is gone:
the version check now runs in a standalone checkout too.

The copies are named dag-schema.json and supervisor-schema.json, not *.schema.json, so the
lint-json-schema hook (files: .*\.schema\.json$) does not rewrite them and break byte-equality
with their source — the same reason ts-sdk named its copy that way.

Left alone deliberately: go-sdk/airflow/spec_test.go still reads trigger_rule.py and
weight_rule.py from airflow-core. That is a tripwire against Python enums, not generated code,
and vendoring a .py into go-sdk is a separate decision.

related: #73963, #73954


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

@FrankYang0529 FrankYang0529 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.

LGTM. Thanks for the PR.

Comment thread scripts/tests/ci/prek/test_sync_go_sdk_schemas.py Outdated

@jason810496 jason810496 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.

Thanks for the follow-up, we need to make sure the sync hook is a manual step only.
Otherwise, we will encounter the same CI failure in the future.

Comment thread .pre-commit-config.yaml
go-sdk reached across the monorepo for both schemas it generates from — airflow-core's
Dag serialization schema and task-sdk's supervisor wire-schema snapshot — so a generated
file was only explainable from a directory the published Go module does not contain, and
every change to a Python schema was a change that had to carry regenerated Go with it,
from whoever happened to be editing the Python.

Both are now vendored under go-sdk/schema/, the way ts-sdk and java-sdk already vendor
theirs, and the generators read the copies. That splits one question into two: the new
sync-go-sdk-schemas hook copies an original over its copy when they differ and fails,
since copying is mechanical and needs neither Go nor the network; and
check-go-sdk-generated-drift regenerates from the copies, since what to do about a new
schema construct is a decision. Regenerating from the copies produces byte-identical
output, and the two Go tests that read a schema now read the copy, so they no longer
skip or break outside the monorepo.
Review feedback: running the sync hook on every commit still fails the PR of
whoever changed a Python schema, which is the coupling vendoring is meant to
remove, so it becomes manual-only the way sync-java-sdk-dag-schema is. The
trade-off is that nothing now announces a stale copy on its own.

The three format_report tests assert the whole report rather than fragments of
it, and a both-refreshed case covers how two refreshes are separated.
@henry3260
henry3260 force-pushed the go-sdk-vendor-schemas branch from df14c7e to 782d109 Compare October 2, 2026 05:45
@henry3260

Copy link
Copy Markdown
Contributor Author

TestSupervisorSchemaVersionMatchesSnapshot now compares the Go constant against the vendored copy, and those move together, so it is no longer a tripwire against the Python original. Nothing announces a stale copy any more. Should we do a follow-up to deal with that? Would a nightly job help?

@henry3260
henry3260 requested a review from jason810496 October 2, 2026 07:58
@jason810496

Copy link
Copy Markdown
Member

Nothing announces a stale copy any more. Should we do a follow-up to deal with that? Would a nightly job help?

I think as long as we record this down in the SDK wise release management process that we should make sure the SDK side catchup with the minor (or major) release. Somehow similar to the i18n translation freeze before the release.

Nightly job might be too often, I think we could have a bi-weekly job as follow-up. Both are valid but not that urgent at this moment if we compare them to the feature gap.

@henry3260
henry3260 merged commit dd27159 into apache:main Oct 2, 2026
98 checks passed
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.

3 participants