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

Add interlocks for move_to_xyz scripts. #1319

Merged

Conversation

@fnothaft
Copy link
Member

fnothaft commented Dec 18, 2016

Resolves #1317. Depends on #1318.

CC @shaneknapp this should prevent the problem you saw this week from happening again.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 18, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1682/

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1319/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains da0fd0a # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1319/merge^{commit} # timeout=10Checking out Revision da0fd0a (origin/pr/1319/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f da0fd0aee7456485f314d9dc6630d568335aac8cFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.


set +x

grep -q "spark2" pom.xml

This comment has been minimized.

Copy link
@heuermh

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 19, 2016

Author Member

Not entirely sure I understand your objection here. We run the same command in both move_to_spark_1 and move_to_spark_2 but we check for different exit codes. In the Spark 1 script, we check for exit code 1 which indicates that "spark2" was not found in pom.xml, while in the Spark 2 script, we check for exit code 0, which indicates that "spark2" was found in pom.xml.

This comment has been minimized.

Copy link
@heuermh

heuermh Dec 19, 2016

Member

Ah right, I see that now

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 19, 2016

Author Member

Yeah, I agree that it is non-obvious though. Let me touch it up with some inline docs to draw attention to what's going on here.

@fnothaft fnothaft force-pushed the fnothaft:issues/1317-interlock-move-scripts branch from e7a3ca4 to 4e70a98 Dec 19, 2016
@AmplabJenkins
Copy link

AmplabJenkins commented Dec 19, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1683/

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1319/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains c314a49ddbae9b191a96cc5318190dc4f61e626b # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1319/merge^{commit} # timeout=10Checking out Revision c314a49ddbae9b191a96cc5318190dc4f61e626b (origin/pr/1319/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f c314a49ddbae9b191a96cc5318190dc4f61e626bFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 19, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1684/

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1319/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains df9842722ceff5d115425205b3e4cd7ba76addd7 # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1319/merge^{commit} # timeout=10Checking out Revision df9842722ceff5d115425205b3e4cd7ba76addd7 (origin/pr/1319/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f df9842722ceff5d115425205b3e4cd7ba76addd7First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

Copy link
Contributor

shaneknapp left a comment

some of these look like testing commits... please let me know when the test code is removed so i can give a better review. :)

@@ -2,6 +2,15 @@

set +x

grep -q "spark2" pom.xml
if [[ $? == 1 ]];
then

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 19, 2016

Contributor

i think it'd be better, and more consistent w/the other script(s), if you grep for spark1, check for a 0 exit code, and then exit.

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 19, 2016

Author Member

The Spark 1.x artifact names don't contain "spark1". We append "spark2" to the artifact names when building for Spark 2.

echo "Cowardly refusing to move to Spark 1..."

exit 1
fi

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 19, 2016

Contributor

do you want to fail out of the build if it's already properly set up?

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 19, 2016

Author Member

Yeah, mostly as a heads up.

find . -name "pom.xml" -exec sed -e "s/2.11.8/2.10.6/g" \
-e "s/2.11/2.10/g" -i.2.10.bak \
-e "/no Scala/ s/Scala 2.10/Scala 2.11/g" -i.2.10.bak \
'{}' \;
find . -name "*.2.10.bak" -exec rm {} \;

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 19, 2016

Contributor

generally, it might be good to put a -f flag on these files to ensure they get deleted. completely optional, tho

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 19, 2016

Author Member

+1, will do

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

rm -f? (if you don't want to bother adding -f to all of your rm calls this time around, then please feel free to punt)

@shaneknapp
Copy link
Contributor

shaneknapp commented Dec 19, 2016

another thought: since the sed/etc scripts are quite small, it might be more readable if everything is located in one script w/function calls.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 21, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1686/

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1319/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 1b7df1d72133ed6b6768344d679be4d135626c20 # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1319/merge^{commit} # timeout=10Checking out Revision 1b7df1d72133ed6b6768344d679be4d135626c20 (origin/pr/1319/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 1b7df1d72133ed6b6768344d679be4d135626c20First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft
Copy link
Member Author

fnothaft commented Dec 22, 2016

Jenkins, retest this please.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 22, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1689/

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1319/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 1b7df1d72133ed6b6768344d679be4d135626c20 # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1319/merge^{commit} # timeout=10Checking out Revision 1b7df1d72133ed6b6768344d679be4d135626c20 (origin/pr/1319/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 1b7df1d72133ed6b6768344d679be4d135626c20First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft
Copy link
Member Author

fnothaft commented Dec 23, 2016

Jenkins, test this please.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 23, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1693/

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1319/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 1b7df1d72133ed6b6768344d679be4d135626c20 # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1319/merge^{commit} # timeout=10Checking out Revision 1b7df1d72133ed6b6768344d679be4d135626c20 (origin/pr/1319/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 1b7df1d72133ed6b6768344d679be4d135626c20First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 23, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1694/
Test PASSed.

@fnothaft fnothaft force-pushed the fnothaft:issues/1317-interlock-move-scripts branch from ffb547c to 5c236b4 Dec 23, 2016
@fnothaft
Copy link
Member Author

fnothaft commented Dec 23, 2016

Squashed down, this should be good to go.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 23, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1695/
Test PASSed.

@fnothaft
Copy link
Member Author

fnothaft commented Dec 23, 2016

@heuermh do you want this in 0.21.0, or shall we bump it? I have it tagged in the release and I think it is good to go, if you can merge.

@fnothaft fnothaft added this to the 0.21.0 milestone Dec 23, 2016
@heuermh
Copy link
Member

heuermh commented Dec 23, 2016

If you feel good about it (I haven't been looking closely at any of the jenkins failures), then I'm fine with it going in 0.21.0. @shaneknapp can we get a +1 from you?

@fnothaft
Copy link
Member Author

fnothaft commented Dec 23, 2016

If you feel good about it (I haven't been looking closely at any of the jenkins failures), then I'm fine with it going in 0.21.0.

Yeah, I think it is good to go. The jenkins failures were due to not cleaning up the POMs after modification and failing the git status --porcelain test, not test failures. Those are resolved now.

@shaneknapp
Copy link
Contributor

shaneknapp commented Dec 23, 2016

taking a quick look now...

Copy link
Contributor

shaneknapp left a comment

the profusion of these shell scripts to hack the build environment is fragile... as a longer-term goal i think it'd be wise to refactor (or even replace them w/something like python+jinja templates).

./scripts/move_to_spark_1.sh
if [[ $? == 0 ]];
then
echo "Moving to Spark 1 did not cowardly fail!"

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

i really don't like these messages. please clean them up.

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 23, 2016

Author Member

OOC, what is it that you don't like about them? I'm willing to clean them up, but I don't have enough info here to know what direction to clean them in.

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

"did not cowardly fail" is not as clear as, for example, "completed updating pom.xml for spark1"

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 23, 2016

Author Member

Oh, sure! Let me get those updated.

./scripts/move_to_scala_2.10.sh
if [[ $? == 0 ]];
then
echo "Moving to Scala 2.10 did not cowardly fail!"

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

same, re: message

./scripts/move_to_spark_2.sh
if [[ $? == 0 ]];
then
echo "Moving to Spark 2 twice did not cowardly fail!"

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

same, re: message

./scripts/move_to_scala_2.11.sh
if [[ $? == 0 ]];
then
echo "Moving to Scala 2.11 twice did not cowardly fail!"

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

same, re: message

if [[ $? == 0 ]];
then
echo "POM is already set up for Spark 2."
echo "Cowardly refusing to move to Spark 2..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

same, re: message

find . -name "pom.xml" -exec sed -e "s/2.11.8/2.10.6/g" \
-e "s/2.11/2.10/g" -i.2.10.bak \
-e "/no Scala/ s/Scala 2.10/Scala 2.11/g" -i.2.10.bak \
'{}' \;
find . -name "*.2.10.bak" -exec rm {} \;

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

rm -f? (if you don't want to bother adding -f to all of your rm calls this time around, then please feel free to punt)

if [[ $? == 0 ]];
then
echo "Scala version is already set to 2.11."
echo "Cowardly refusing to move to Scala 2.11..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

same, re: message

if [[ $? == 0 ]];
then
echo "Scala version is already set to 2.10."
echo "Cowardly refusing to move to Scala 2.10..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

same, re: message

@fnothaft
Copy link
Member Author

fnothaft commented Dec 23, 2016

the profusion of these shell scripts to hack the build environment seems to me to be fragile... as a longer-term goal i think it'd be wise to refactor (or even replace them w/something like python+jinja).

Yeah, @ryan-williams has been looking at ways to do this with sbt. I don't like sbt at all, but I would consider moving if it eliminates this problem.

@fnothaft fnothaft force-pushed the fnothaft:issues/1317-interlock-move-scripts branch from 3555e4a to 70fe5da Dec 23, 2016
@shaneknapp
Copy link
Contributor

shaneknapp commented Dec 23, 2016

i have... mixed feelings about SBT as well.

@shaneknapp
Copy link
Contributor

shaneknapp commented Dec 23, 2016

btw, i'm about to leave for the afternoon (bowling w/the inlaws+kids), so don't hesitate on merging if everything passes. :)

@fnothaft
Copy link
Member Author

fnothaft commented Dec 23, 2016

Yeah, would be good to discuss more @shaneknapp. I think @heuermh and I are pretty much of the mindset that while Maven is not a beautiful tool, it is a solid tool. I agree with @ryan-williams that the build hacking is crufty, but I've found SBT to be wanting in the past and have migrated several projects off of SBT in the past 4 years. From what I hear, SBT has been improving a lot though.

Also, I've just updated the error messages. Let me know if they look better, and have a great time bowling!

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 23, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1703/
Test PASSed.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 23, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1704/
Test PASSed.

if [[ $? == 0 ]];
then
echo "Scala version is already set to 2.10 (Scala artifacts have _2.10 version suffix in artifact name)."
echo "Cowardly refusing to move to Scala 2.10 a second time..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

missed one....

This comment has been minimized.

Copy link
@fnothaft

fnothaft Dec 24, 2016

Author Member

I think I misunderstood you the first time around. I thought that what you were saying was that the previous messages weren't sufficiently descriptive, so I added to them in bdc90a8. E.g., here we went from:

    echo "Scala version is already set to 2.10."
     echo "Cowardly refusing to move to Scala 2.10."

to:

    echo "Scala version is already set to 2.10 (Scala artifacts have _2.10 version suffix in artifact name)."
    echo "Cowardly refusing to move to Scala 2.10 a second time..."

Since these messages are still not looking good to you, I think I must've misunderstood your critique. If you've got a minute, perhaps it'd be useful if you gave an example of what you think a good message would be?

if [[ $? == 0 ]];
then
echo "Scala version is already set to 2.11 (Scala artifacts have _2.11 version suffix in artifact name)."
echo "Cowardly refusing to move to Scala 2.11 a second time..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

and another...

if [[ $? == 1 ]];
then
echo "POM is already set up for Spark 1 (Spark 1/2 artifacts are missing -spark2 suffix in artifact names)."
echo "Cowardly refusing to move to Spark 1 a second time..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

...and another.

if [[ $? == 0 ]];
then
echo "POM is already set up for Spark 2 (Spark 1/2 artifacts have -spark2 suffix in artifact names)."
echo "Cowardly refusing to move to Spark 2 a second time..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

omg. more.

# this next line is supposed to fail
set +e

echo "Rewriting POM.xml files to Scala 2.10 and Spark 1 should error..."

This comment has been minimized.

Copy link
@shaneknapp

shaneknapp Dec 23, 2016

Contributor

remove this comment (or add more comments when running the subsequent scripts)

@fnothaft
Copy link
Member Author

fnothaft commented Dec 29, 2016

Heads up to @shaneknapp: @heuermh and I spoke today, and this commit is blocking a release that we're trying to cut ASAP. We're going to move forward on merging this PR (once I do a bit of cleanup) and I will circle back with you after new years to sort out the error message problems.

@fnothaft fnothaft force-pushed the fnothaft:issues/1317-interlock-move-scripts branch from bdc90a8 to 32749d9 Dec 29, 2016
@fnothaft fnothaft force-pushed the fnothaft:issues/1317-interlock-move-scripts branch from 32749d9 to d774073 Dec 29, 2016
@fnothaft
Copy link
Member Author

fnothaft commented Dec 29, 2016

@heuermh this is good to go from my side.

@AmplabJenkins
Copy link

AmplabJenkins commented Dec 29, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1710/
Test PASSed.

@shaneknapp
Copy link
Contributor

shaneknapp commented Dec 29, 2016

@heuermh heuermh merged commit 0ad63f1 into bigdatagenomics:master Dec 29, 2016
1 check passed
1 check passed
default Merged build finished.
Details
@heuermh
Copy link
Member

heuermh commented Dec 29, 2016

Thank you, @fnothaft @shaneknapp!

@fnothaft
Copy link
Member Author

fnothaft commented Dec 29, 2016

Thanks @heuermh and @shaneknapp. I've created #1331 to follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.