From b191cc4767bfcaf23849777a39b7c05b48bfc244 Mon Sep 17 00:00:00 2001 From: Pierre Jeambrun Date: Mon, 5 Oct 2026 11:05:04 +0200 Subject: [PATCH] Fail instead of silently falling behind when a Lang-SDK vendored Dag schema drifts from airflow-core (#74091) A core Dag-serialization schema change shipped with the Go SDK's vendored copy left behind, uncaught by any hook: the sync hooks for Go and Java were manual only, and the one check that runs on every commit watched the vendored copies, not the Python source, so it never saw the schema change at all. The same drift already caused a two-release-old bug in the Go SDK's generated models (#73954). Go's and Java's sync hooks now run on every commit and are triggered by the Python source too, the way the TypeScript SDK's already was, refreshing the vendored copy and failing when they had to. Java's Gradle task keeps the same task name and stays silent for generateDagDsl's own dependency, so an ordinary in-repo build still refreshes a schema someone is mid-edit on without breaking; only the prek hook's invocation passes the flag that makes it fail. --- .pre-commit-config.yaml | 42 ++++++++++++++++---------- java-sdk/sdk/build.gradle.kts | 15 +++++++++ scripts/ci/prek/sync_go_sdk_schemas.py | 21 +++++++------ 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54b29e9cdd6..c04fc9f2aa4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -340,27 +340,33 @@ repos: - id: sync-java-sdk-dag-schema name: Sync Java SDK Dag serialization schema with airflow-core description: "Copy airflow-core's serialization schema when Java SDK's vendored dag-schema.json drifts" - entry: ./java-sdk/gradlew -p ./java-sdk :sdk:syncDagSchema + entry: ./java-sdk/gradlew -p ./java-sdk :sdk:syncDagSchema -PfailOnDagSchemaDrift language: system pass_filenames: false - # Re-vendoring is a java-sdk maintainer's deliberate step after an airflow-core - # schema change, not something every commit should do, so this is manual only: - # prek run sync-java-sdk-dag-schema --hook-stage manual - stages: ['manual'] - files: ^java-sdk/sdk/schema/dag-schema\.json$ + # Runs on every commit: a stale vendored copy is exactly the drift that broke a + # vendored Go copy unnoticed (see sync-go-sdk-schemas below), so this fails + # loudly instead of silently falling behind. -PfailOnDagSchemaDrift is what makes + # it fail here; generateDagDsl's own dependency on this same task never passes it, + # so an ordinary in-repo build keeps refreshing the copy without breaking on a + # schema someone is mid-edit on. The task refreshes the file and fails when it + # had to, so `git add` the refreshed copy and recommit. + files: > + (?x) + ^airflow-core/src/airflow/serialization/schema\.json$| + ^java-sdk/sdk/schema/dag-schema\.json$ - id: sync-go-sdk-schemas name: Sync Go SDK vendored schemas with the distributions that own them description: "Copy airflow-core's Dag schema and task-sdk's supervisor schema when go-sdk's vendored copies drift" entry: ./scripts/ci/prek/sync_go_sdk_schemas.py language: python pass_filenames: false - # Re-vendoring is a go-sdk maintainer's deliberate step after a schema change on - # the Python side, not something every commit should do, so this is manual only, - # the way sync-java-sdk-dag-schema above is: - # prek run sync-go-sdk-schemas --hook-stage manual - # Running it on every commit would make a change to a Python schema fail the PR - # of whoever made it, which is the coupling vendoring is here to remove. - stages: ['manual'] + # Runs on every commit: it used to be manual, "a go-sdk maintainer's deliberate + # step," so a Python-only schema change could ship with the vendored Go copy + # silently left behind — no hook caught it, because this one was never invoked + # and check-go-sdk-generated-drift below only watched the Go side. This refreshes + # the copy and fails when it had to, so `git add` the refreshed file(s) and + # recommit; check-go-sdk-generated-drift then catches the generated code that + # needs regenerating from the refreshed copy. files: > (?x) ^airflow-core/src/airflow/serialization/schema\.json$| @@ -374,11 +380,15 @@ repos: # golang so prek provisions the toolchain the generators need, the way the # other checks that shell out to `go` in go-sdk get theirs. language: golang - # The vendored schemas, not the Python originals: sync-go-sdk-schemas above is - # what ties those to the copies, as a manual step. This one runs on every commit, - # because a refreshed copy that nothing regenerated from is the drift that matters. + # Also triggered by the Python schemas themselves, not just the vendored copies: + # sync-go-sdk-schemas (which runs first, see hook order above) refreshes the + # vendored copy in the same commit a Python schema changes, and this regenerates + # from whatever that copy now holds, so the two together catch both a stale copy + # and generated code that copy was never regenerated from. files: > (?x) + ^airflow-core/src/airflow/serialization/schema\.json$| + ^task-sdk/src/airflow/sdk/execution_time/schema/schema\.json$| ^go-sdk/schema/.*\.json$| ^go-sdk/airflow/spec\.gen\.go$| ^go-sdk/airflow/spec\.go$| diff --git a/java-sdk/sdk/build.gradle.kts b/java-sdk/sdk/build.gradle.kts index 5e14ff2086c..20b491b3721 100644 --- a/java-sdk/sdk/build.gradle.kts +++ b/java-sdk/sdk/build.gradle.kts @@ -225,6 +225,13 @@ abstract class SyncDagSchemaTask : DefaultTask() { @get:Internal abstract val targetFile: RegularFileProperty + // False for an ordinary in-repo build, which must keep refreshing the copy silently + // so it does not break on a schema a developer is actively editing. True for the + // prek hook, which passes -PfailOnDagSchemaDrift so a commit that leaves the + // vendored copy behind fails instead of shipping unnoticed. + @get:Internal + abstract val failOnDagSchemaDrift: Property + @TaskAction fun sync() { val src = sourceFile.get().asFile @@ -239,6 +246,12 @@ abstract class SyncDagSchemaTask : DefaultTask() { } logger.lifecycle("Refreshing vendored dag-schema.json from ${src.path}") src.copyTo(dst, overwrite = true) + if (failOnDagSchemaDrift.getOrElse(false)) { + throw GradleException( + "Vendored dag-schema.json was out of date and has been refreshed from ${src.path}. " + + "Review the diff and commit it.", + ) + } } } @@ -661,6 +674,8 @@ val syncDagSchema by tasks.registering(SyncDagSchemaTask::class) { description = "Refresh the vendored Dag serialization schema from the monorepo copy when present." sourceFile = layout.projectDirectory.file("../../airflow-core/src/airflow/serialization/schema.json") targetFile = dagSchemaInput + // -PfailOnDagSchemaDrift carries no value, so presence (not content) is the signal. + failOnDagSchemaDrift = providers.gradleProperty("failOnDagSchemaDrift").map { true }.orElse(false) } tasks.register("generateDagDsl") { diff --git a/scripts/ci/prek/sync_go_sdk_schemas.py b/scripts/ci/prek/sync_go_sdk_schemas.py index 38171925eb4..5656e07af9e 100755 --- a/scripts/ci/prek/sync_go_sdk_schemas.py +++ b/scripts/ci/prek/sync_go_sdk_schemas.py @@ -26,21 +26,24 @@ contain, and made every change to a Python schema a change that has to carry reg Go with it. It now vendors them under ``go-sdk/schema/``, the way ``ts-sdk`` and ``java-sdk`` already vendor theirs. -Vendoring splits the one question ("is the Go behind Python?") into two: +Vendoring splits the one question ("is the Go behind Python?") into two, and both now +run on every commit that touches either side: * this hook — is the copy equal to the source? Copying is mechanical, so it copies for - you and fails. It is a **manual** hook, the way ``sync-java-sdk-dag-schema`` is: - re-vendoring is a go-sdk maintainer's deliberate step, and running it on every commit - would fail the PR of whoever changed a Python schema, which is the coupling vendoring - is here to remove. Nothing therefore tells you on its own that a copy went stale. + you and fails, triggered by either the Python source or the vendored copy changing. + A Python-only schema PR used to leave this unrun (it was a go-sdk maintainer's manual + step) and the vendored copy going stale was not caught by anything else either — a Go + copy did exactly that undetected once. Failing here, on the PR that caused it, is the + fix. * ``check-go-sdk-generated-drift`` — are the generated files what the copy generates? - That one runs on every commit, because a refreshed copy nothing regenerated from is - the drift that matters, and a new schema construct may need a generator rule or an - authoring exclusion, so what to do about it is a decision, not a copy. + Also triggered by the Python source now, so it runs in the same commit as this hook + and regenerates from whatever this hook leaves the copy holding; a new schema + construct may need a generator rule or an authoring exclusion, so what to do about it + is a decision, not a copy, which is why that part stays a second, separate hook. Run it from the repo root, through prek: - prek run sync-go-sdk-schemas --hook-stage manual + prek run sync-go-sdk-schemas or directly: