Skip the ts-sdk supervisor schema check on PRs not touching ts-sdk - #69583
Merged
jason810496 merged 4 commits intoJul 8, 2026
Merged
Conversation
CI's static-checks job runs `prek --all-files`, which runs every default-stage hook against the whole repo tree regardless of what a given PR actually touched. Because check-ts-sdk-supervisor-schema lists task-sdk's schema.json as a trigger file, any task-sdk-only PR that changes the wire schema fails static checks over an unrelated, not-yet-regenerated ts-sdk file it never intended to touch. Move the hook to the manual stage, matching how other on-demand checks in this file (update-notice-year, upgrade-important-versions, update-docker-gpg-keys) are handled: it stops gating unrelated PRs and ts-sdk owners run it themselves (`prek run --hook-stage manual check-ts-sdk-supervisor-schema`) when they pick up a schema change.
boring-cyborg
Bot
added
area:dev-tools
backport-to-v3-3-test
The first attempt made check-ts-sdk-supervisor-schema a manual-stage hook, but review found that removes it from CI entirely (nothing invokes it at the manual stage), so schema drift would merge undetected. Reuse the existing selective-skip mechanism already used for ktlint and the mypy-* hooks instead: the hook keeps running in CI's default static-checks sweep, but is now skipped when neither ts-sdk/ nor the supervisor wire schema changed, so unrelated task-sdk-only PRs no longer fail on it while real drift is still caught.
jason810496
requested review from
amoghrajesh,
ashb,
bugraoz93,
choo121600,
ephraimbuddy,
gopidesupavan,
jedcunningham and
vatsrahul1001
as code owners
July 8, 2026 03:21
Lee-W
approved these changes
Jul 8, 2026
Member
|
hmmm the CI still fails |
Member
Author
Found the root case, let me fix it shortly. |
A task-sdk PR that bumps the supervisor wire schema should not be responsible for regenerating the ts-sdk types - that is the ts-sdk follow-up PR's job. Trigger the hook only on ts-sdk/ changes, both in CI (selective skip) and locally (the hook's files pattern), so schema authors are never blocked on a file they do not own. Drift merged via a schema-only PR is still caught by full-tests runs (canary and structural-change PRs run every hook) and by the next ts-sdk PR.
The wire schema gained an extra field on GetAssetEventByAsset and GetAssetEventByAssetAlias in apache#69453 without the ts-sdk types being regenerated. This PR's own changes to dev/breeze and the prek config force a full static-checks run, where no hook is skipped, so the check correctly surfaced that pre-existing drift and blocked CI.
jason810496
changed the title
amoghrajesh
approved these changes
Jul 8, 2026
Member
Author
|
Merging this first as |
Contributor
Backport failed to create: v3-3-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker c96da9b v3-3-testThis should apply the commit to the v3-3-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
Member
Author
|
No need to backport, |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Why
CI's static-checks job runs
prek --all-files, so a task-sdk-only PR that bumps the supervisor wire schema failscheck-ts-sdk-supervisor-schemaover a not-yet-regeneratedts-sdk/src/generated/supervisor.tsit never touched. Regenerating the ts-sdk types is the ts-sdk follow-up PR's job, not the schema author's.What
check-ts-sdk-supervisor-schemavia the existing selective-skip mechanism (likektlint) when nots-sdk/file changed; dropschema.jsonfrom the hook'sfiles:trigger so local commits behave the same. Drift from a schema-only merge is still caught by full-tests runs (canary) and the next ts-sdk PR.supervisor.ts: Reject invalid partition keys in the create asset events API #69453 changed the wire schema without regenerating it; this PR's full static-checks run surfaced that pre-existing drift.Verification
dev/breezeselective-checks tests: 182 passed. Hook is skipped for schema-only / unrelated changes, runs for anyts-sdk/change.prek run check-ts-sdk-supervisor-schema --all-filespasses after the regeneration.Was generative AI tooling used to co-author this PR?