Skip to content

Refactor schema version checks to share target-branch and worktree logic - #72070

Closed
ColtenOuO wants to merge 1 commit into
apache:mainfrom
ColtenOuO:refactor-shared-schema-version-checks
Closed

ColtenOuO wants to merge 1 commit into
apache:mainfrom
ColtenOuO:refactor-shared-schema-version-checks

Conversation

@ColtenOuO

Copy link
Copy Markdown
Contributor

Summary

check_execution_api_versions.py and check_supervisor_schemas_versions.py each carried their own copy of the logic for resolving the target branch, fetching it, and standing up/tearing down a temporary git worktree to compare against it. check_supervisor_schemas_versions.py even had a # TODO: We should consolidte the common logic with check_execution_api_versions.py into common_prek_utils sitting right above its copy, flagging the duplication as something that should eventually move to the shared common_prek_utils.py module. This PR does that consolidation.

Keeping two independent copies of this logic around is a maintenance hazard: any fix to one script's target-branch resolution or worktree cleanup (for example, handling a missing GITHUB_BASE_REF, or making sure the worktree is always removed even when schema generation raises) has to be manually ported to the other script, and it is easy to fix one and forget the other. Consolidating into a single, shared implementation means both checks get the same behavior automatically and future fixes only need to happen in one place.

Changes

  • Added shared helpers to scripts/ci/prek/common_prek_utils.py:
    • get_target_branch() and get_changed_files() — moved verbatim from the two check scripts, which had identical copies of both.
    • fetch_target_branch() — new helper that wraps get_remote_for_main() + get_target_branch() + the git fetch call and returns the resolved remote/branch ref, replacing the repeated three-line sequence in both scripts.
    • create_temporary_worktree() — new context manager that creates a temporary worktree for a given ref and guarantees its removal on exit (including when the code inside the with block raises), replacing the duplicated tempfile.mkdtemp() + git worktree add + try/finally + git worktree remove blocks in both scripts.
  • Updated check_execution_api_versions.py and check_supervisor_schemas_versions.py to import and use these shared helpers instead of their own local copies, removing the duplicated code (including the now-resolved TODO comment).
  • Added unit test coverage:
    • scripts/tests/ci/prek/test_check_execution_api_versions.py and test_check_supervisor_schemas_versions.py are new — neither script had tests before this change.
    • scripts/tests/ci/prek/test_common_prek_utils.py gained tests for the four helpers listed above (get_target_branch, get_changed_files, fetch_target_branch, create_temporary_worktree), including a case verifying create_temporary_worktree cleans up the worktree even when the caller raises inside the with block.

This is an internal CI/dev-tooling refactor with no change in behavior.


Was generative AI tooling used to co-author this PR?
  • Yes — Codex (5.6 sol)

Execution API and Supervisor checks duplicated target-branch and temporary-worktree management. Keeping that infrastructure synchronized prevents divergent validation behavior and ensures temporary resources are cleaned up.
@potiuk

potiuk commented Sep 25, 2026

Copy link
Copy Markdown
Member

Hello @ColtenOuO - thank you for your contributions to Apache Airflow!

The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 19 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:

These pull requests stay open because maintainers are already engaged in them - they count towards your limit:

This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are.

What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or gh pr reopen <PR_NUMBER> --repo apache/airflow. Reopen the ones you are ready to follow through - keep them rebased, respond to review comments and fix failing checks.

While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack.

Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

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

Labels

area:dev-tools closed because of open PR limit Closed as a one-time step of introducing the open pull request limit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants