-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-22129][SPARK-22138] Release script improvements #19359
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
Conversation
…d coded and just use path version. Remove version swapping on publish-release since we only need it some of the time and it was confusing.
|
Test build #82216 has finished for PR 19359 at commit
|
|
retest this please |
|
Test build #82228 has finished for PR 19359 at commit
|
|
@holdenk, |
| REMOTE_PARENT_DIR=${REMOTE_PARENT_DIR:-/home/$ASF_USERNAME/public_html} | ||
|
|
||
| GPG="gpg --no-tty --batch" | ||
| GPG="gpg -u $GPG_KEY --no-tty --batch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the script work without GPG_KEY , or is it required to run this script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is required the run the script. This parameter was already required to be set (see L60) but was ignored so I'm assuming this was just a bug that happened to work for users with their Apache key as default and it was intended to be used this way previously.
|
|
||
| # Clean-up Zinc nailgun process | ||
| /usr/sbin/lsof -P |grep $ZINC_PORT | grep LISTEN | awk '{ print $2; }' | xargs kill | ||
| lsof -P |grep $ZINC_PORT | grep LISTEN | awk '{ print $2; }' | xargs kill |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, @holdenk, I thought the full path of lsof is required for non-root user in few OSs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh that's weird, not something I expected. I think in that case the user could just add /usr/sbin to their $PATH (whereas on some systems lsof is in /usr/bin not /usr/sbin so fixing that would require root).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a thing people are likely to run into I can add some code to check for lsof in different places under usr, but that seems brittle. Checking on one of the jenkins machines, my local laptop, and a random debian server I run lsof all works as a non-root user just fine. (Although searching on Google does indeed result in some folks who have had to either add /usr/sbin or other dir to their path as a non-root user).
|
Looks fine to me, just few questions. |
|
Thanks for taking a look @HyukjinKwon ! If you have a chance to also 2.1.2 RC I'd appreciate that as well :) Let me know what you think of my responses. |
## What changes were proposed in this pull request? Use the GPG_KEY param, fix lsof to non-hardcoded path, remove version swap since it wasn't really needed. Use EXPORT on JAVA_HOME for downstream scripts as well. ## How was this patch tested? Rolled 2.1.2 RC2 Author: Holden Karau <[email protected]> Closes #19359 from holdenk/SPARK-22129-fix-signing. (cherry picked from commit ecbe416) Signed-off-by: Holden Karau <[email protected]>
## What changes were proposed in this pull request? Use the GPG_KEY param, fix lsof to non-hardcoded path, remove version swap since it wasn't really needed. Use EXPORT on JAVA_HOME for downstream scripts as well. ## How was this patch tested? Rolled 2.1.2 RC2 Author: Holden Karau <[email protected]> Closes #19359 from holdenk/SPARK-22129-fix-signing. (cherry picked from commit ecbe416) Signed-off-by: Holden Karau <[email protected]>
|
Looks like some of the snapshot builds are having lsof issues: spark-build/dev/create-release/release-build.sh: line 344: lsof: command not found |
|
We could switch this to an || so it can be either lsofs? Would you want to do that or should I? |
…lease-build.sh ## What changes were proposed in this pull request? This PR proposes to use `/usr/sbin/lsof` if `lsof` is missing in the path to fix nightly snapshot jenkins jobs. Please refer #19359 (comment): > Looks like some of the snapshot builds are having lsof issues: > > https://amplab.cs.berkeley.edu/jenkins/view/Spark%20Packaging/job/spark-branch-2.1-maven-snapshots/182/console > >https://amplab.cs.berkeley.edu/jenkins/view/Spark%20Packaging/job/spark-branch-2.2-maven-snapshots/134/console > >spark-build/dev/create-release/release-build.sh: line 344: lsof: command not found >usage: kill [ -s signal | -p ] [ -a ] pid ... >kill -l [ signal ] Up to my knowledge, the full path of `lsof` is required for non-root user in few OSs. ## How was this patch tested? Manually tested as below: ```bash #!/usr/bin/env bash LSOF=lsof if ! hash $LSOF 2>/dev/null; then echo "a" LSOF=/usr/sbin/lsof fi $LSOF -P | grep "a" ``` Author: hyukjinkwon <[email protected]> Closes #19695 from HyukjinKwon/SPARK-22377. (cherry picked from commit c8b7f97) Signed-off-by: hyukjinkwon <[email protected]>
…lease-build.sh ## What changes were proposed in this pull request? This PR proposes to use `/usr/sbin/lsof` if `lsof` is missing in the path to fix nightly snapshot jenkins jobs. Please refer #19359 (comment): > Looks like some of the snapshot builds are having lsof issues: > > https://amplab.cs.berkeley.edu/jenkins/view/Spark%20Packaging/job/spark-branch-2.1-maven-snapshots/182/console > >https://amplab.cs.berkeley.edu/jenkins/view/Spark%20Packaging/job/spark-branch-2.2-maven-snapshots/134/console > >spark-build/dev/create-release/release-build.sh: line 344: lsof: command not found >usage: kill [ -s signal | -p ] [ -a ] pid ... >kill -l [ signal ] Up to my knowledge, the full path of `lsof` is required for non-root user in few OSs. ## How was this patch tested? Manually tested as below: ```bash #!/usr/bin/env bash LSOF=lsof if ! hash $LSOF 2>/dev/null; then echo "a" LSOF=/usr/sbin/lsof fi $LSOF -P | grep "a" ``` Author: hyukjinkwon <[email protected]> Closes #19695 from HyukjinKwon/SPARK-22377. (cherry picked from commit c8b7f97) Signed-off-by: hyukjinkwon <[email protected]>
## What changes were proposed in this pull request? Use the GPG_KEY param, fix lsof to non-hardcoded path, remove version swap since it wasn't really needed. Use EXPORT on JAVA_HOME for downstream scripts as well. ## How was this patch tested? Rolled 2.1.2 RC2 Author: Holden Karau <[email protected]> Closes apache#19359 from holdenk/SPARK-22129-fix-signing. (cherry picked from commit ecbe416) Signed-off-by: Holden Karau <[email protected]>
…lease-build.sh ## What changes were proposed in this pull request? This PR proposes to use `/usr/sbin/lsof` if `lsof` is missing in the path to fix nightly snapshot jenkins jobs. Please refer apache#19359 (comment): > Looks like some of the snapshot builds are having lsof issues: > > https://amplab.cs.berkeley.edu/jenkins/view/Spark%20Packaging/job/spark-branch-2.1-maven-snapshots/182/console > >https://amplab.cs.berkeley.edu/jenkins/view/Spark%20Packaging/job/spark-branch-2.2-maven-snapshots/134/console > >spark-build/dev/create-release/release-build.sh: line 344: lsof: command not found >usage: kill [ -s signal | -p ] [ -a ] pid ... >kill -l [ signal ] Up to my knowledge, the full path of `lsof` is required for non-root user in few OSs. ## How was this patch tested? Manually tested as below: ```bash #!/usr/bin/env bash LSOF=lsof if ! hash $LSOF 2>/dev/null; then echo "a" LSOF=/usr/sbin/lsof fi $LSOF -P | grep "a" ``` Author: hyukjinkwon <[email protected]> Closes apache#19695 from HyukjinKwon/SPARK-22377. (cherry picked from commit c8b7f97) Signed-off-by: hyukjinkwon <[email protected]>
What changes were proposed in this pull request?
Use the GPG_KEY param, fix lsof to non-hardcoded path, remove version swap since it wasn't really needed. Use EXPORT on JAVA_HOME for downstream scripts as well.
How was this patch tested?
Rolled 2.1.2 RC2