Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-41102: [Packaging][Release] Create unique git tags for release candidates (e.g. apache-arrow-{MAJOR}.{MINOR}.{PATCH}-rc{RC_NUM}) #41131

Merged
merged 120 commits into from
Jun 13, 2024

Conversation

sgilmore10
Copy link
Member

@sgilmore10 sgilmore10 commented Apr 10, 2024

Rationale for this change

As per @kou's suggestion in #40956, we should create unique git tags (e.g. apache-arrow-{MAJOR}.{MINOR}.{VERSION}-rc{RC_NUM}) instead re-using the same git tag (apache-arrow-{MAJOR}.{MINOR}.{VERSION}) for each release candidate. The official release candidate tag (apache-arrow-{MAJOR}.{MINOR}.{VERSION}) should be created only after a release candidate is voted on and accepted. This "official" release tag should point to the same object in the git database as the accepted release candidate tag.

The new release workflow could look like the following:

  1. Create a apache-arrow-X.Y.Z-rc0 tag for X.Y.Z RC0
  2. (Found a problem for X.Y.Z RC0)
  3. Create a apache-arrow-X.Y.Z-rc1 tag for X.Y.Z RC1
  4. Vote
  5. Passed
  6. Create a apache-arrow-X.Y.Z tag from apache-arrow-X.Y.Z-rc1 ike apache/arrow-adbc and apache/arrow-flight-sql-postgresql do

See @kou's comment for more details.

What changes are included in this PR?

  1. Updated dev/release/01-prepare.sh to create release-candidate-specific git tags (e.g. apache-arrow-{MAJOR}.{MINOR}.{PATCH}-rc{RC_NUM}).
  2. Updated scripts in dev/release to use the new git tag name.
  3. Added GitHub Workflow file publish_release_candidate.yml. This workflow is triggered when a release candidate git tag is pushed and creates a Prerelease GitHub Release.
  4. Added logic to dev/release/02-post-binary.sh to create and push the release git tag (i.e. apache-arrow-{MAJOR}.{MINOR}.{PATCH}).
  5. Added GitHub Workflow publish_release.yml. This workflow is triggered when the release tag is pushed and creates a GitHub Release for the approved release (i.e. the voted upon release).
  6. Added dev/release/post-16-delete-release-candidates.sh to delete the release candidate git tags and their associated GitHub Releases.
  7. Updated docs/developers/release.rst with the new steps.

Are these changes tested?

  1. We were not able to verify the changes made to the scripts in dev/release. Any suggestions on how we can verify these scripts would be much appreciated :)
  2. We did test the new GitHub Workflows (publish_release_candidate.yml and publish_release.yml) work as intended by pushing git tags to mathworks/arrow.

Are there any user-facing changes?

No.

Open Questions

  1. We noticed that apache/arrow-flight-sql-postgresql does not delete the release candidate Prereleases from their GitHub Releases area. Should we be doing the same? Or would it be preferable to just delete the the release candidates without deleting the release candidate tags.
  2. We're not that familiar with ruby, so we're not sure if the changes we made to dev/release/02-source-test.rb make sense.

Future Directions

  1. Continue working on GH-40924: [MATLAB][Packaging] Add script for uploading Release Candidate (RC) MLTBX packages for the MATLAB bindings to the Apache Arrow GitHub Releases area #40956
  2. Add logic to auto-sign release artifacts in GitHub Actions Workflows.

Copy link
Member

@assignUser assignUser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a first look at the workflows only. I think we can just merge them into one and run|skip some steps with if: when the triggering tag has the -rc suffix.

.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot added awaiting review Awaiting review awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Apr 10, 2024
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
dev/release/02-source.sh Show resolved Hide resolved
dev/release/post-02-binary.sh Outdated Show resolved Hide resolved
dev/release/post-16-delete-release-candidates.sh Outdated Show resolved Hide resolved
docs/source/developers/release.rst Outdated Show resolved Hide resolved
docs/source/developers/release.rst Show resolved Hide resolved
@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting committer review Awaiting committer review awaiting changes Awaiting changes labels Apr 11, 2024
@sgilmore10
Copy link
Member Author

I had a first look at the workflows only. I think we can just merge them into one and run|skip some steps with if: when the triggering tag has the -rc suffix.

That's a good point. I'll go ahead and merge the yml files together.

@sgilmore10
Copy link
Member Author

Going to test out the new release.yml workflow on mathworks/arrow. I'll post a comment once I'm done with that.

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
dev/release/post-01-tag.sh Outdated Show resolved Hide resolved
dev/release/post-01-tag.sh Outdated Show resolved Hide resolved
@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Apr 12, 2024
@assignUser
Copy link
Member

assignUser commented Apr 12, 2024

I just noticed we might need INFRA approval before merginging this:

Automated services MUST NOT push data to a repository or branch that is subject to official release as a software package by the project, unless the project secures specific prior authorization of the workflow from Infrastructure.

https://infra.apache.org/github-actions-policy.html

Kind of depends on if creating a release counts as 'pushing data to the repo'. I actually don't think it does because its Github only and doesn't affect the 'arrow.git' contents. But I will ask.

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Apr 12, 2024
@assignUser
Copy link
Member

I was about to open an issue for INFRA to look over the workflow when I noticed that this PR also contains the changes from #40956 (+ some new changes on the workflows) is this intentional or do we want to move those changes back out of this PR? I just need clarity which PR I send to INFRA ^^

@sgilmore10
Copy link
Member Author

sgilmore10 commented Apr 12, 2024

Just a quick update. I tested the new release_candidate.yml and release.yml workflows on the mathworks/arrow repository by pushing the tags test-apache-arrow-16.0.0-rc0 and test-apache-arrow-16.0.0 to mathworks/arrow. It's worth mentioning I modified the workflow files to expect the tag names to start with the prefix test (I didn't want to push tags whose names matched the patterns apache-arrow-X.Y.Z and apache-arrow-X.Y.Z-rcN).

Both the release_candidate and the release workflows passed. You can view the generated releases in mathworks/arrow's GitHub Releases area.

I'll delete those releases once this PR is done. For now, I'll leave them up to serve as a proof of concept - unless you prefer I delete them asap.

@sgilmore10
Copy link
Member Author

I was about to open an issue for INFRA to look over the workflow when I noticed that this PR also contains the changes from #40956 (+ some new changes on the workflows) is this intentional or do we want to move those changes back out of this PR? I just need clarity which PR I send to INFRA ^^

I think the plan is to first submit this PR and then rebase #40956 once this PR is merged. While working on #40956, I realized it made more sense to first add "generic" release candidate and release workflows before adding the MATLAB specific logic. Once this PR is merged, I'll go back to working on #40956. Sorry for the confusion!

@sgilmore10
Copy link
Member Author

The INFRA ticket is now closed and I believe this PR is ready to merge. @assignUser, do you have any more flags? I think you just requested changes so we knew not to merge this PR until the Jira ticket was closed.

Otherwise, unless anyone has any other flags, I think I can merge this.

Also a big thank you to everyone who helped me!

Copy link
Member

@assignUser assignUser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the long running efforts! Feel free to merge!

@sgilmore10 sgilmore10 merged commit 6ec2f22 into apache:main Jun 13, 2024
12 checks passed
@sgilmore10 sgilmore10 deleted the GH-41102 branch June 13, 2024 13:06
@sgilmore10 sgilmore10 removed the awaiting change review Awaiting change review label Jun 13, 2024
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 6ec2f22.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

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

Successfully merging this pull request may close these issues.

4 participants