SPARK-2058: Overriding config from SPARK_HOME with SPARK_CONF_DIR #997
Conversation
|
Can one of the admins verify this patch? |
bin/compute-classpath.cmd
Outdated
There was a problem hiding this comment.
Isn't this condition inverted?
|
Modification to compute-classpath.sh similar to SPARK-1559, pull request #471 |
There was a problem hiding this comment.
Use Files.createTempDir(), and delete it when the test is done (using scalatest's BeforeAndAfter); there are several examples of this in other tests.
There was a problem hiding this comment.
Just a clarification: I see you're deleting the directory below, so BeforeAndAfter does not apply here. But still I'd prefer to use Files.createTempDir() instead of a hardcoded name. Just for paranoia.
|
LGTM aside from the minor test issues. |
There was a problem hiding this comment.
While we're here, this can be one call to Utils.deleteRecursively(tmpDir)
…rdcoded conf dir name with generated one
|
Thx for the comments. Updated the PR based on them. |
|
LGTM. Thanks! |
|
We should document SPARK_CONF_DIR in the configuration guide. |
|
Updated the doc. BTW, actually the behaviour is not symmetric, for example spark-env if only searched in SPARK_CONF_DIR if defined but for the configs loaded from classpath, I am enriching the CP, so if some file is missing in SPARK_CONF_DIR it will still be taken from SPARK_HOME/conf. What do you think about that? Should we update the scripts so instead of adding SPARK_CONF_DIR it replaces the default conf dir? |
|
add to whitelist |
There was a problem hiding this comment.
No need to pass sys.env to SparkSubmitArguments. You can just get it from there directly since it's the same JVM
|
@EugenCepoi Thanks for fixing this. It seems this PR is only for branch-1.0, but it would be good to fix this for branch-1.1 and master as well. The code has changed quite a bit between now and then, so could you open up a new PR against the master branch? In response to your question, I think the asymmetry can be a source of confusion. If the user sets |
|
test this please |
|
QA tests have started for PR 997 at commit
|
|
QA tests have finished for PR 997 at commit
|
|
@andrewor14 Agreed it's too verbose. I will make those changes and reopen it against branch-1.1 but I wont have the time to do it right now. |
|
Let's close this issue then pending follow up from @EugenCepoi. |
|
@andrewor14 @pwendell I updated it and opened a new PR #2481 against master. |
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
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]>
10 participants
Fixes https://issues.apache.org/jira/browse/SPARK-2058
The general idea is to use SPARK_CONF_DIR content instead of SPARK_HOME/conf when defined.