kaxil commented on code in PR #74464:
URL: https://github.com/apache/airflow/pull/74464#discussion_r4221205232
##########
dev/README_RELEASE_AIRFLOW.md:
##########
@@ -114,6 +114,25 @@ moves `vX-Y-test` forward to the current `main` - only the
branch-specific commi
`Update default branches for X.Y`) are kept on top of `main`. Each beta is cut
from the branch
in that state, so everything merged to `main` lands in the next beta.
+A beta (`X.Y.0bN`) is cut with the same
[`start-rc-process`](#build-rc-artifacts) command as a
+release candidate - the only difference is that there is no vote. Because no
`vX-Y-stable` branch
Review Comment:
The opening sentence, "The only difference is that there is no vote", no
longer holds: the next two paragraphs add two more differences (constraints
come from the branch tip, and the push must be declined). The link also sends a
beta reader to the note under Build RC artifacts, which says the candidate
resolves its own constraints. After this PR that is only true for an rc. Could
both places say so?
##########
dev/README_RELEASE_AIRFLOW.md:
##########
@@ -114,6 +114,25 @@ moves `vX-Y-test` forward to the current `main` - only the
branch-specific commi
`Update default branches for X.Y`) are kept on top of `main`. Each beta is cut
from the branch
in that state, so everything merged to `main` lands in the next beta.
+A beta (`X.Y.0bN`) is cut with the same
[`start-rc-process`](#build-rc-artifacts) command as a
+release candidate - the only difference is that there is no vote. Because no
`vX-Y-stable` branch
+exists yet during the beta phase, `start-rc-process` validates, merges and
tags against `vX-Y-test`
+instead of `vX-Y-stable` when `--version` is a beta (for an `rc` it uses
`vX-Y-stable` as before). No
+stable branch is required to cut a beta; the stable branch is created at
`X.Y.0rc1`.
+
+Constraints are handled differently for a beta. A beta pins providers at their
released versions
+from sources that match `main`, so its resolution is already what the shared
`constraints-X-Y`
+branch holds - `start-rc-process` therefore tags `constraints-X.Y.0bN` at the
`constraints-X-Y`
+branch tip rather than triggering the `release-constraints` workflow (which
only earns its cost for
+an `rc`, where the provider wave on PyPI must be pinned, and for a final,
which commits onto
+`constraints-X-Y`). Sync `constraints-X-Y` to `constraints-main` before
cutting the beta so the tip
Review Comment:
What's the concrete step for "sync `constraints-X-Y` to `constraints-main`"?
`tag_constraints_from_branch_tip` tags whatever the tip holds without any
freshness check, so this step is the only thing keeping the beta's constraints
current. If it's the `Refresh constraints` workflow run with `ref=vX-Y-test`
(which writes `constraints-X-Y` from that ref's sources), naming it here would
make it repeatable.
##########
dev/breeze/src/airflow_breeze/commands/release_candidate_command.py:
##########
@@ -443,10 +446,42 @@ def sign_the_release(repo_root):
console_print("[success]Release signed")
-def generate_and_push_constraints(version, version_branch):
- # Resolved from the stable branch the candidate was cut from, so the
constraints describe the
- # sources being voted on. The workflow reads "rcN" and allows pre-releases
accordingly.
- publish_constraints(version=version, ref=f"v{version_branch}-stable")
+def tag_constraints_from_branch_tip(version, version_branch, remote_name):
+ """Tag ``constraints-<version>`` at the ``constraints-X-Y`` branch tip.
+
+ A beta pins providers at their released versions, which that tip already
holds, so there is
+ nothing new to resolve. Sync ``constraints-X-Y`` to ``constraints-main``
before cutting so the
+ tip is current.
+ """
+ constraints_branch = f"constraints-{version_branch}"
+ constraints_tag = f"constraints-{version}"
+ if not confirm_action(f"Tag {constraints_tag} at the tip of
{remote_name}/{constraints_branch}?"):
+ return
+ run_command(["git", "fetch", remote_name, constraints_branch], check=True)
Review Comment:
If a beta run fails after this step (SVN, PyPI) and gets re-run, `git tag -a
constraints-<version>` fails because the tag already exists, and it fails after
the build and signing steps. The rc path doesn't hit this because the workflow
deletes and recreates its tag. Could the beta path add
`validate_tag_does_not_exist(f"constraints-{version}", remote_name)` next to
the two tag checks at the start, so a re-run stops before the build and tells
the RM how to delete it?
##########
dev/breeze/tests/test_release_candidate_command.py:
##########
@@ -45,6 +45,124 @@ def rc_cmd():
return module
[email protected](
+ ("version", "version_branch", "expected"),
+ [
+ pytest.param("3.4.0rc1", "3-4", "v3-4-stable", id="rc-uses-stable"),
+ pytest.param("3.4.0rc2", "3-4", "v3-4-stable",
id="later-rc-uses-stable"),
+ pytest.param("3.4.0b1", "3-4", "v3-4-test", id="beta-uses-test"),
+ ],
+)
+def test_get_candidate_base_branch(rc_cmd, version, version_branch, expected):
Review Comment:
These cover the branch choice, but nothing calls
`validate_version_branches_exist`, `merge_pr` or
`validate_on_correct_branch_for_tagging` with a beta, so putting `-stable` back
in any of them keeps the suite green. A parametrized rc/beta test of
`validate_version_branches_exist` where `git branch -r` lists only
`upstream/v3-4-test` (beta passes, rc exits) would pin the behavior this PR is
for.
##########
dev/breeze/src/airflow_breeze/commands/release_candidate_command.py:
##########
@@ -242,7 +245,7 @@ def merge_pr(version_branch, remote_name, sync_branch):
)
if confirm_action("Do you want to push the changes? Pushing the
changes closes the PR"):
Review Comment:
For a beta this still offers to push the merge to `vX-Y-test`, and the
README now says that answer has to be no or the next fast-forward breaks.
`--answer yes` (or `ANSWER=y`) answers it without a prompt, and so does an RM
on rc habit. Since `candidate_base_branch` already says it's a beta, could this
skip the push when it's the test branch and print why, with a test that a beta
`merge_pr` never runs `git push`?
Related: what is the sync PR for a beta? If one is opened against
`vX-Y-test` and the push is declined, it stays open, so the README should
probably say to close it unmerged.
--
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]
