Skip to content

Conversation

@shaneknapp
Copy link
Contributor

What changes were proposed in this pull request?

i broke run-tests.py for non-PRB builds in this PR:
#25423

Why are the changes needed?

to fix what i broke

Does this PR introduce any user-facing change?

no

How was this patch tested?

the build system will test this

@shaneknapp
Copy link
Contributor Author

@HyukjinKwon @srowen

it behooves us, when checking for env vars in non-prb builds, to always check for the key before referencing it in an if statement. :)

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, right.

@shaneknapp
Copy link
Contributor Author

once we get to the Running Spark unit tests block i will feel comfortable in merging this...

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-28701] fix the key error when looking in os.environ [SPARK-28701][INFRA] Fix the key error when looking in os.environ Aug 26, 2019
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-28701][INFRA] Fix the key error when looking in os.environ [SPARK-28701][INFRA][FOLLOWUP] Fix the key error when looking in os.environ Aug 26, 2019
@shaneknapp
Copy link
Contributor Author

thanks @dongjoon-hyun

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@SparkQA
Copy link

SparkQA commented Aug 26, 2019

Test build #109747 has finished for PR 25585 at commit 9b6fc4f.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shaneknapp
Copy link
Contributor Author

  • This patch fails Spark unit tests.

this has to be unrelated... should i test again or just go ahead and merge?

@srowen
Copy link
Member

srowen commented Aug 26, 2019

It clearly fixes the issue that was failing before, right? you can see the desired output in the logs that even starts the tests? then I'd merge it. That can't be related.

@shaneknapp
Copy link
Contributor Author

alrighty! things looking good. the builds i broke (spark-master-test-sbt-*) are good, and i spot-checked one (https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-sbt-hadoop-3.2/324/) and the spark unit tests just started successfully:

========================================================================
Running Spark unit tests
========================================================================
[info] Running Spark tests using SBT with these arguments:  -Phadoop-3.2 -Pkubernetes -Phadoop-cloud -Phive -Phive-thriftserver -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Pmesos test
Using /usr/java/jdk1.8.0_191 as default JAVA_HOME.
Note, this will be overridden by -java-home if it is set.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants