Repository navigation
Conversation
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.
|
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 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 |
Summary
check_execution_api_versions.pyandcheck_supervisor_schemas_versions.pyeach 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.pyeven had a# TODO: We should consolidte the common logic with check_execution_api_versions.py into common_prek_utilssitting right above its copy, flagging the duplication as something that should eventually move to the sharedcommon_prek_utils.pymodule. 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
scripts/ci/prek/common_prek_utils.py:get_target_branch()andget_changed_files()— moved verbatim from the two check scripts, which had identical copies of both.fetch_target_branch()— new helper that wrapsget_remote_for_main()+get_target_branch()+ thegit fetchcall and returns the resolvedremote/branchref, 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 thewithblock raises), replacing the duplicatedtempfile.mkdtemp()+git worktree add+try/finally+git worktree removeblocks in both scripts.check_execution_api_versions.pyandcheck_supervisor_schemas_versions.pyto import and use these shared helpers instead of their own local copies, removing the duplicated code (including the now-resolved TODO comment).scripts/tests/ci/prek/test_check_execution_api_versions.pyandtest_check_supervisor_schemas_versions.pyare new — neither script had tests before this change.scripts/tests/ci/prek/test_common_prek_utils.pygained tests for the four helpers listed above (get_target_branch,get_changed_files,fetch_target_branch,create_temporary_worktree), including a case verifyingcreate_temporary_worktreecleans up the worktree even when the caller raises inside thewithblock.This is an internal CI/dev-tooling refactor with no change in behavior.
Was generative AI tooling used to co-author this PR?