-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-19464][BUILD][HOTFIX] run-tests should use hadoop2.6 #16858
Conversation
@srowen Could you review this? |
@@ -165,12 +165,6 @@ def main(): | |||
if "test-maven" in ghprb_pull_title: | |||
os.environ["AMPLAB_JENKINS_BUILD_TOOL"] = "maven" | |||
# Switch the Hadoop profile based on the PR title: |
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.
This removes hadoop 2.5 and earlier, too.
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.
Ah... right missed this. This part doesn't actually matter, but, do the Jenkins jobs not all set a Hadoop version? then yeah could default to hadoop2.3 which doesn't exist
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.
Yep. This is not related to the SparkPullRequestBuilder failures.
SparkPullRequestBuilder failure is due to the other file in this PR.
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.
BTW while you're at it, I spotted one more thing that we could remove although it probably won't matter at all. Look for expect_equal(basenameSansExtFromUrl(y), "spark-2.1.0-bin-hadoop2.4-without-hive")
in test_utils.R
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.
Sure!
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.
I fixed that too.
@@ -505,14 +505,14 @@ def main(): | |||
# if we're on the Amplab Jenkins build servers setup variables | |||
# to reflect the environment settings | |||
build_tool = os.environ.get("AMPLAB_JENKINS_BUILD_TOOL", "sbt") | |||
hadoop_version = os.environ.get("AMPLAB_JENKINS_BUILD_PROFILE", "hadoop2.3") |
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.
The default value should be 2.6.
test_env = "amplab_jenkins" | ||
# add path for Python3 in Jenkins if we're calling from a Jenkins machine | ||
os.environ["PATH"] = "/home/anaconda/envs/py3k/bin:" + os.environ.get("PATH") | ||
else: | ||
# else we're running locally and can use local settings | ||
build_tool = "sbt" | ||
hadoop_version = os.environ.get("HADOOP_PROFILE", "hadoop2.3") |
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.
Here, too.
@@ -232,9 +232,9 @@ test_that("basenameSansExtFromUrl", { | |||
x <- paste0("http://people.apache.org/~pwendell/spark-nightly/spark-branch-2.1-bin/spark-2.1.1-", | |||
"SNAPSHOT-2016_12_09_11_08-eb2d9bf-bin/spark-2.1.1-SNAPSHOT-bin-hadoop2.7.tgz") | |||
y <- paste0("http://people.apache.org/~pwendell/spark-releases/spark-2.1.0-rc2-bin/spark-2.1.0-", | |||
"bin-hadoop2.4-without-hive.tgz") | |||
"bin-hadoop2.6-without-hive.tgz") |
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.
Actually there is no such artifact anymore. Just remove this line and the one below that also referenced it
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.
Oh, I see.
I removed those lines, too. |
Thank you for review, @srowen . Also, since it touches R now, cc @felixcheung . |
Test build #72596 has finished for PR 16858 at commit
|
The only failure is irrelevant to this PR, ad also the very next Jenkins pass the test.
|
Test build #72595 has finished for PR 16858 at commit
|
OK merging this as it's a fix for the build |
Oh, wait a moment please. The Jenkins on the last commit will succeed soon. |
Test build #72597 has finished for PR 16858 at commit
|
Finally, it passed. |
Merged to master |
Thank you for review and merging, @srowen ! |
Test build #3567 has finished for PR 16858 at commit
|
Thanks but the R test removal is unnecessary but probably ideally should be added back at some point. it was just parsing a sample URL - we should have one to make sure we don't break with the release share URL in which the sub directory structure is different from nightly snapshot builds.
It's not urgent.
|
This particular URL pattern won't exist anymore, so seems fine to not test it at this point. |
## What changes were proposed in this pull request? After SPARK-19464, **SparkPullRequestBuilder** fails because it still tries to use hadoop2.3. **BEFORE** https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72595/console ``` ======================================================================== Building Spark ======================================================================== [error] Could not find hadoop2.3 in the list. Valid options are ['hadoop2.6', 'hadoop2.7'] Attempting to post to Github... > Post successful. ``` **AFTER** https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72595/console ``` ======================================================================== Building Spark ======================================================================== [info] Building Spark (w/Hive 1.2.1) using SBT with these arguments: -Phadoop-2.6 -Pmesos -Pkinesis-asl -Pyarn -Phive-thriftserver -Phive test:package streaming-kafka-0-8-assembly/assembly streaming-flume-assembly/assembly streaming-kinesis-asl-assembly/assembly Using /usr/java/jdk1.8.0_60 as default JAVA_HOME. Note, this will be overridden by -java-home if it is set. ``` ## How was this patch tested? Pass the existing test. Author: Dongjoon Hyun <dongjoon@apache.org> Closes apache#16858 from dongjoon-hyun/hotfix_run-tests.
What changes were proposed in this pull request?
After SPARK-19464, SparkPullRequestBuilder fails because it still tries to use hadoop2.3.
BEFORE
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72595/console
AFTER
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72595/console
How was this patch tested?
Pass the existing test.