You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/11/10 04:50:04 UTC
[GitHub] [beam] youngoli opened a new pull request #13290: [BEAM-11074] build_release_candidate usability improvements.
youngoli opened a new pull request #13290:
URL: https://github.com/apache/beam/pull/13290
Specifically, this change checks whether a tag exists before calling the gradle release plugin, allowing the user to either delete the existing tag or skip creating the tag. Also avoids going into interactive commit messages with SVN, instead using commit messages built-in to the script.
Note that the original intention behind this change is to allow finer-grained skipping in the release candidate script to avoid problems if steps need to be repeated if a release candidate fails. For example, in the current implementation if a release candidate fails after creating a release candidate tag and that step gets run again, it'll cause an error due to trying to create an existing tag. I tried to read through the script to find other instances where repeating a step might be problematic, but I couldn't really find anything else that isn't already skippable. Reviewers, let me know if I missed any.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] youngoli commented on a change in pull request #13290: [BEAM-11074] build_release_candidate usability improvements.
Posted by GitBox <gi...@apache.org>.
youngoli commented on a change in pull request #13290:
URL: https://github.com/apache/beam/pull/13290#discussion_r520976431
##########
File path: release/src/main/scripts/build_release_candidate.sh
##########
@@ -116,13 +116,21 @@ if [[ $confirmation = "y" ]]; then
echo "-------------Building Java Artifacts with Gradle-------------"
git config credential.helper store
- ./gradlew release -Prelease.newVersion=${RELEASE}-SNAPSHOT \
- -Prelease.releaseVersion=${RELEASE}-RC${RC_NUM} \
- -Prelease.useAutomaticVersion=true --info --no-daemon
-
- git push origin "${RELEASE_BRANCH}"
- git push origin "v${RELEASE}-RC${RC_NUM}"
-
+ if git rev-parse "v${RELEASE}-RC${RC_NUM}" >/dev/null 2>&1; then
+ echo "Tag v${RELEASE}-RC${RC_NUM} already exists."
+ echo "Delete the tag and create a new tag commit (y) or skip this step (n)? [y/N]"
+ read confirmation
+ if [[ $confirmation = "y" ]]; then
+ git push origin :refs/tags/v${RELEASE}-RC${RC_NUM}
+ fi
+ fi
+ if [[ $confirmation = "y" ]]; then # Expected to only be "n" if user chose to skip creating tag.
Review comment:
Done.
##########
File path: release/src/main/scripts/build_release_candidate.sh
##########
@@ -116,13 +116,21 @@ if [[ $confirmation = "y" ]]; then
echo "-------------Building Java Artifacts with Gradle-------------"
git config credential.helper store
- ./gradlew release -Prelease.newVersion=${RELEASE}-SNAPSHOT \
- -Prelease.releaseVersion=${RELEASE}-RC${RC_NUM} \
- -Prelease.useAutomaticVersion=true --info --no-daemon
-
- git push origin "${RELEASE_BRANCH}"
- git push origin "v${RELEASE}-RC${RC_NUM}"
-
+ if git rev-parse "v${RELEASE}-RC${RC_NUM}" >/dev/null 2>&1; then
+ echo "Tag v${RELEASE}-RC${RC_NUM} already exists."
+ echo "Delete the tag and create a new tag commit (y) or skip this step (n)? [y/N]"
+ read confirmation
+ if [[ $confirmation = "y" ]]; then
+ git push origin :refs/tags/v${RELEASE}-RC${RC_NUM}
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] youngoli commented on pull request #13290: [BEAM-11074] build_release_candidate usability improvements.
Posted by GitBox <gi...@apache.org>.
youngoli commented on pull request #13290:
URL: https://github.com/apache/beam/pull/13290#issuecomment-724449856
R: @ibzib @kennknowles
CC: @lostluck
Kenn, adding you as a reviewer because one of my changes basically reverts a change you made [in an old commit](https://github.com/apache/beam/commit/f68779fc99796e2622236889270a7eeaef56812b). I found it slightly user unfriendly because one moment the script is just running and then suddenly you're thrown into an interactive commit prompt without much context about what to do. Going back to having the script prompting seems more user-friendly, and also means that the script can automatically set a commit message.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] youngoli merged pull request #13290: [BEAM-11074] build_release_candidate usability improvements.
Posted by GitBox <gi...@apache.org>.
youngoli merged pull request #13290:
URL: https://github.com/apache/beam/pull/13290
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #13290: [BEAM-11074] build_release_candidate usability improvements.
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #13290:
URL: https://github.com/apache/beam/pull/13290#issuecomment-725486322
Run Java PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ibzib commented on a change in pull request #13290: [BEAM-11074] build_release_candidate usability improvements.
Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #13290:
URL: https://github.com/apache/beam/pull/13290#discussion_r520760990
##########
File path: release/src/main/scripts/build_release_candidate.sh
##########
@@ -116,13 +116,21 @@ if [[ $confirmation = "y" ]]; then
echo "-------------Building Java Artifacts with Gradle-------------"
git config credential.helper store
- ./gradlew release -Prelease.newVersion=${RELEASE}-SNAPSHOT \
- -Prelease.releaseVersion=${RELEASE}-RC${RC_NUM} \
- -Prelease.useAutomaticVersion=true --info --no-daemon
-
- git push origin "${RELEASE_BRANCH}"
- git push origin "v${RELEASE}-RC${RC_NUM}"
-
+ if git rev-parse "v${RELEASE}-RC${RC_NUM}" >/dev/null 2>&1; then
+ echo "Tag v${RELEASE}-RC${RC_NUM} already exists."
+ echo "Delete the tag and create a new tag commit (y) or skip this step (n)? [y/N]"
+ read confirmation
+ if [[ $confirmation = "y" ]]; then
+ git push origin :refs/tags/v${RELEASE}-RC${RC_NUM}
+ fi
+ fi
+ if [[ $confirmation = "y" ]]; then # Expected to only be "n" if user chose to skip creating tag.
Review comment:
```suggestion
if [[ $confirmation = "y" ]]; then # Expected to be "y" unless user chose to skip creating tag.
```
##########
File path: release/src/main/scripts/build_release_candidate.sh
##########
@@ -116,13 +116,21 @@ if [[ $confirmation = "y" ]]; then
echo "-------------Building Java Artifacts with Gradle-------------"
git config credential.helper store
- ./gradlew release -Prelease.newVersion=${RELEASE}-SNAPSHOT \
- -Prelease.releaseVersion=${RELEASE}-RC${RC_NUM} \
- -Prelease.useAutomaticVersion=true --info --no-daemon
-
- git push origin "${RELEASE_BRANCH}"
- git push origin "v${RELEASE}-RC${RC_NUM}"
-
+ if git rev-parse "v${RELEASE}-RC${RC_NUM}" >/dev/null 2>&1; then
+ echo "Tag v${RELEASE}-RC${RC_NUM} already exists."
+ echo "Delete the tag and create a new tag commit (y) or skip this step (n)? [y/N]"
+ read confirmation
+ if [[ $confirmation = "y" ]]; then
+ git push origin :refs/tags/v${RELEASE}-RC${RC_NUM}
Review comment:
Can you add a comment explaining how this deletes the tag?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org