vatsrahul1001 commented on code in PR #75329:
URL: https://github.com/apache/airflow/pull/75329#discussion_r4340011275
##########
.github/workflows/release-constraints.yml:
##########
@@ -118,15 +118,14 @@ jobs:
constraints_branch="constraints-${major}-${minor}"
echo "constraints-branch=${constraints_branch}" >> "${GITHUB_OUTPUT}"
# A pre-release - beta or rc - pins the providers at the versions
PyPI holds (rc provider
- # waves included) and lands on a branch and tag of its own, so it
never moves the shared
- # constraints-X-Y branch. A final pins released providers and
commits onto the shared branch.
+ # waves included); a final pins released providers. Both commit onto
the shared
+ # constraints-X-Y branch and are tagged constraints-<VERSION>.
Review Comment:
Note / ack (not a change request): this reverses the old "pre-releases never
move the shared branch" invariant. `constraints-X-Y` now transiently carries
the pre-release's rc-wave provider pins between this run and the next rolling
`update-constraints-on-push`. Fine for a minor-in-prep; but for a *patch* line
(e.g. `constraints-3-3`, an already-released line) it briefly exposes `rcN`
provider pins to anyone tracking the rolling branch. Intentional tradeoff —
just flagging it's a conscious behavior change.
##########
.github/workflows/release-constraints.yml:
##########
@@ -231,39 +230,37 @@ jobs:
with:
pattern: constraints-*
path: ./files
- # A candidate's branch and tag belong to that candidate alone, so
re-running for the same
- # rc replaces them rather than adding to them: the branch would
otherwise already hold the
- # previous run's constraints (making the push a non-fast-forward) and
the tag already exist.
- # A final never gets this - it commits onto the shared constraints-X-Y
branch, whose history
- # every other build reads.
- - name: "Delete the previous ${{ needs.build-info.outputs.target-branch
}} branch and tag"
+ # Re-running for the same pre-release adds a commit on constraints-X-Y
and moves its tag, so
+ # the previous run's tag is deleted first. A leftover
constraints-<VERSION> branch (from when
+ # pre-releases had a branch of their own) would make that tag name
ambiguous, so it fails fast.
+ - name: "Check for a stale constraints-${{ inputs.version }} branch and
replace the tag"
if: needs.build-info.outputs.allow-pre-releases == 'true'
working-directory: "constraints"
shell: bash
run: |
- if git ls-remote --exit-code origin "refs/heads/${TARGET_BRANCH}" >
/dev/null; then
- echo "Deleting the existing '${TARGET_BRANCH}' branch."
- git push origin --delete "refs/heads/${TARGET_BRANCH}"
+ if git ls-remote --exit-code origin
"refs/heads/constraints-${VERSION}" > /dev/null; then
+ echo "ERROR! Branch 'constraints-${VERSION}' must not exist -" \
+ "it would make the 'constraints-${VERSION}' tag ambiguous."
+ echo "Delete it first, e.g.: git push upstream --delete
'refs/heads/constraints-${VERSION}'"
Review Comment:
Nit: remote-name mismatch. The workflow uses `origin` everywhere (`git
ls-remote origin`, `git push origin …`), but this hint prints `git push
upstream --delete …`. Pasted as-is it targets the wrong remote in the
workflow/RM-checkout context — suggest `origin` for consistency.
##########
.github/workflows/release-constraints.yml:
##########
@@ -231,39 +230,37 @@ jobs:
with:
pattern: constraints-*
path: ./files
- # A candidate's branch and tag belong to that candidate alone, so
re-running for the same
- # rc replaces them rather than adding to them: the branch would
otherwise already hold the
- # previous run's constraints (making the push a non-fast-forward) and
the tag already exist.
- # A final never gets this - it commits onto the shared constraints-X-Y
branch, whose history
- # every other build reads.
- - name: "Delete the previous ${{ needs.build-info.outputs.target-branch
}} branch and tag"
+ # Re-running for the same pre-release adds a commit on constraints-X-Y
and moves its tag, so
+ # the previous run's tag is deleted first. A leftover
constraints-<VERSION> branch (from when
+ # pre-releases had a branch of their own) would make that tag name
ambiguous, so it fails fast.
+ - name: "Check for a stale constraints-${{ inputs.version }} branch and
replace the tag"
if: needs.build-info.outputs.allow-pre-releases == 'true'
working-directory: "constraints"
shell: bash
run: |
- if git ls-remote --exit-code origin "refs/heads/${TARGET_BRANCH}" >
/dev/null; then
- echo "Deleting the existing '${TARGET_BRANCH}' branch."
- git push origin --delete "refs/heads/${TARGET_BRANCH}"
+ if git ls-remote --exit-code origin
"refs/heads/constraints-${VERSION}" > /dev/null; then
+ echo "ERROR! Branch 'constraints-${VERSION}' must not exist -" \
+ "it would make the 'constraints-${VERSION}' tag ambiguous."
+ echo "Delete it first, e.g.: git push upstream --delete
'refs/heads/constraints-${VERSION}'"
+ exit 1
fi
if git ls-remote --exit-code origin
"refs/tags/constraints-${VERSION}" > /dev/null; then
echo "Deleting the existing 'constraints-${VERSION}' tag."
git push origin --delete "refs/tags/constraints-${VERSION}"
fi
git tag --delete "constraints-${VERSION}" > /dev/null 2>&1 || true
- git branch --delete --force "${TARGET_BRANCH}" > /dev/null 2>&1 ||
true
- name: "Switch to ${{ needs.build-info.outputs.target-branch }}"
working-directory: "constraints"
shell: bash
- run: git switch -c "${TARGET_BRANCH}" 2>/dev/null || git switch
"${TARGET_BRANCH}"
+ run: git switch "${TARGET_BRANCH}"
- name: "Diff in constraints for Python: ${{
needs.build-info.outputs.python-versions-list-as-string }}"
run: ./scripts/ci/constraints/ci_diff_constraints.sh
- name: "Commit changed constraint files"
run: ./scripts/ci/constraints/ci_commit_constraints.sh
- name: "Push ${{ needs.build-info.outputs.target-branch }}"
working-directory: "constraints"
shell: bash
- # Qualify as refs/heads/ -- for a release candidate the branch and the
tag share a name
- # (both constraints-<VERSION>), so a bare refspec is ambiguous
("matches more than one").
+ # Qualify as refs/heads/ so the push never resolves to a same-named
tag.
run: git push origin "refs/heads/${TARGET_BRANCH}"
Review Comment:
Pre-releases now push to the shared `constraints-X-Y` branch, but this is a
plain `git push` — no `--rebase`/retry/force (`ci_commit_constraints.sh` just
commits). `constraints-X-Y` is *also* written by
`update-constraints-on-push.yml` on every `vX-Y-test` uv.lock change, which
happens routinely during the exact beta/rc prep window this workflow runs in. A
concurrent write → non-fast-forward → this job fails and the RM has to re-run.
The old layout avoided this (pre-releases had their own branch); finals hit it
too but run once, whereas betas/rcs run several times per cycle.
Suggest `git pull --rebase origin ${TARGET_BRANCH}` + a small retry loop
around the push — constraints commits don't conflict in content, so
rebase-and-repush is safe. Worth noting this is exactly the race previously
cited as the reason candidates got their own branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
