[BEAM-8337] Hard-code Flink versions.#10549
Conversation
Also publish the RC images instead of rebuilding.
mxm
left a comment
There was a problem hiding this comment.
What were the problems? I'd prefer not hardcoding the versions. If there are problems with this, I'd reckon they are just a symptom of other non-deterministic patterns in the release scripts.
The issues were with creating FLINK_VER as a bash array from subshell output. I recommend using strings instead. I don't mind hard-coded versions for now. |
mxm
left a comment
There was a problem hiding this comment.
Seems fair to change this if the bash version approach is too fragile.
| docker pull "${FLINK_IMAGE_NAME}:${RELEASE}_${RC_VERSION}" | ||
|
|
||
| # Tag with ${RELEASE} and push to dockerhub. | ||
| docker tag "${FLINK_IMAGE_NAME}:${RELEASE}_${RC_VERSION}" "${FLINK_IMAGE_NAME}:${RELEASE}" |
There was a problem hiding this comment.
Looks like the indention is off here (3 instead of 2 spaces).
|
We could've fixed the bash instead. I prefer this way because the publish
script no longer has to either make a clone or be run from within an up to
date clone, instead it can be run from anywhere.
…On Fri, Jan 10, 2020 at 10:20 AM Maximilian Michels < ***@***.***> wrote:
***@***.**** approved this pull request.
Seems fair to change this if the bash version approach is too fragile.
------------------------------
In release/src/main/scripts/publish_docker_images.sh
<#10549 (comment)>:
> FLINK_IMAGE_NAME=apachebeam/flink${ver}_job_server
+
+ # Pull verified RC from dockerhub.
+ docker pull "${FLINK_IMAGE_NAME}:${RELEASE}_${RC_VERSION}"
+
+ # Tag with ${RELEASE} and push to dockerhub.
+ docker tag "${FLINK_IMAGE_NAME}:${RELEASE}_${RC_VERSION}" "${FLINK_IMAGE_NAME}:${RELEASE}"
Looks like the indention is off here (3 instead of 2 spaces).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10549?email_source=notifications&email_token=AD2L6NW5MTEVYQ7UWB5WFBDQ5C37BA5CNFSM4KFBDZ2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRMGJQQ#pullrequestreview-341337282>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD2L6NS4R45EHTHKNBYV2ZLQ5C37BANCNFSM4KFBDZ2A>
.
|
|
Looks good to me. |
I see that the scripts are also written in markdown within the release guide "run all steps manually" section. I'm not sure why that section's there, however. Is there a difference between that and the contents of the scripts? |
This script is used when release it manually, for example, the script failed during running and release manager release left part manually. So we need to keep it consistent. However, I think this part can be improved. |
I'd prefer to keep only the full scripts as the single source-of-truth for what needs to be run. But that's out of scope for this PR. |
|
For now, I've updated the release guide. For future discussion/action on simplifying the release guide, I filed https://issues.apache.org/jira/browse/BEAM-9087. |
Also publish the RC images instead of rebuilding.
There were problems with the bash code for munging Flink versions, so I got rid of it and replaced with a hard-coded list. The release manager will have to manually validate that they're correct, but I think that's a good idea anyway.
@Hannah-Jiang @udim @mxm
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.