Skip to content

SPARK-2058: Overriding SPARK_HOME/conf with SPARK_CONF_DIR #2481

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.

Sign up for GitHub

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

Jump to bottom
Closed
EugenCepoi wants to merge 2 commits into from
Closed

SPARK-2058: Overriding SPARK_HOME/conf with SPARK_CONF_DIR #2481

EugenCepoi wants to merge 2 commits into from

Conversation

@EugenCepoi
Copy link
Contributor

@EugenCepoi EugenCepoi commented Sep 21, 2014

Update of PR #997.

With this PR, setting SPARK_CONF_DIR overrides SPARK_HOME/conf (not only spark-defaults.conf and spark-env).

@SparkQA
Copy link

SparkQA commented Sep 21, 2014

QA tests have started for PR 2481 at commit 77f35d7.

  • This patch merges cleanly.

@EugenCepoi EugenCepoi mentioned this pull request Sep 21, 2014
Closed
@SparkQA
Copy link

SparkQA commented Sep 21, 2014

QA tests have finished for PR 2481 at commit 77f35d7.

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

andrewor14
andrewor14 reviewed Sep 29, 2014
View reviewed changes
bin/compute-classpath.cmd Outdated
Copy link
Contributor

@andrewor14 andrewor14 Sep 29, 2014

Choose a reason for hiding this comment

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

Why do you have an extra % here before CLASSPATH? What does that do?

@andrewor14
Copy link
Contributor

andrewor14 commented Sep 29, 2014

Minor comments, but LGTM otherwise.

@EugenCepoi
Copy link
Contributor Author

EugenCepoi commented Oct 3, 2014

@andrewor14 The trailing % came from a bad copy paste, nice catch :) Double percent exists but was wrong in this case, http://stackoverflow.com/questions/14509652/what-is-the-difference-between-and-in-a-cmd-file.

@SparkQA
Copy link

SparkQA commented Oct 3, 2014

QA tests have started for PR 2481 at commit 0bb32c2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Oct 3, 2014

QA tests have finished for PR 2481 at commit 0bb32c2.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Oct 3, 2014

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

andrewor14
andrewor14 reviewed Oct 3, 2014
View reviewed changes
bin/compute-classpath.sh
Copy link
Contributor

@andrewor14 andrewor14 Oct 3, 2014

Choose a reason for hiding this comment

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

I would move this back down to where the comment is, but this is trivial I can fix it myself when I merge it.

@andrewor14
Copy link
Contributor

andrewor14 commented Oct 3, 2014

LGTM. I'm merging this into master and 1.1. Thanks @EugenCepoi!

@asfgit asfgit closed this in f0811f9 Oct 3, 2014
asfgit pushed a commit that referenced this pull request Oct 3, 2014
@EugenCepoi @andrewor14
SPARK-2058: Overriding SPARK_HOME/conf with SPARK_CONF_DIR
5d991db 
Update of PR #997.

With this PR, setting SPARK_CONF_DIR overrides SPARK_HOME/conf (not only spark-defaults.conf and spark-env).

Author: EugenCepoi <[email protected]>

Closes #2481 from EugenCepoi/SPARK-2058 and squashes the following commits:

0bb32c2 [EugenCepoi] use orElse orNull and fixing trailing percent in compute-classpath.cmd
77f35d7 [EugenCepoi] SPARK-2058: Overriding SPARK_HOME/conf with SPARK_CONF_DIR

(cherry picked from commit f0811f9)
Signed-off-by: Andrew Or <[email protected]>
@EugenCepoi
Copy link
Contributor Author

EugenCepoi commented Oct 3, 2014

@andrewor14 great! Glad to see it merged into master :)

@JoshRosen
Copy link
Contributor

JoshRosen commented Oct 3, 2014

@andrewor14 #2541 looks like it fixes one more place where we need to handle the override.

@andrewor14
Copy link
Contributor

andrewor14 commented Oct 3, 2014

Ah yes I believe that's for the Spark daemons. We can merge that separately.

@sarutak sarutak mentioned this pull request Oct 6, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@EugenCepoi @SparkQA @andrewor14 @AmplabJenkins @JoshRosen