-
Notifications
You must be signed in to change notification settings - Fork 28.8k
[SPARK-2098] All Spark processes should support spark-defaults.conf, config file #1256
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
Conversation
Merged build triggered. |
Merged build started. |
Merged build triggered. |
Merged build started. |
Merged build finished. All automated tests passed. |
All automated tests passed. |
Merged build finished. All automated tests passed. |
All automated tests passed. |
Merged build triggered. |
Merged build started. |
Merged build finished. All automated tests passed. |
All automated tests passed. |
} | ||
|
||
/** Load any spark.* system properties */ | ||
private[spark] def loadSystemProperties(isOverride: Boolean = true) { |
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 is only called in one place, why the argument? Seems like we always want system properties to override other settings anyway.
I'd like to see better documentation about the semantics of how the different sources for configuration override each other. Also, some tests would be nice. |
Merged build triggered. |
Merged build started. |
Merged build finished. |
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16465/ |
Jenkins, retest this please. |
Merged build triggered. |
Merged build started. |
val message = s"Failed when loading Spark properties" | ||
throw new SparkException(message, e) | ||
} finally { | ||
inReader.close() |
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.
If you're keeping this, you should document that this method will close the given input stream.
test("loading from file") { | ||
val outFile = File.createTempFile("sparkConf-loading-from-file", "") | ||
outFile.deleteOnExit() | ||
val outStream: FileWriter = new FileWriter(outFile) |
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.
You could use this method instead.
Still a few nits left, but looks ok to me. |
QA tests have started for PR 1256. This patch merges cleanly. |
QA results for PR 1256: |
QA tests have started for PR 1256. This patch merges cleanly. |
QA results for PR 1256: |
Jenkins, retest this please. |
QA tests have started for PR 1256. This patch merges cleanly. |
QA results for PR 1256: |
QA tests have started for PR 1256. This patch merges cleanly. |
QA results for PR 1256: |
@pwendell @andrewor14 could you guys take a look at this PR? Thanks! |
<td>spark.history.fs.logDirectory</td> | ||
<td>(none)</td> | ||
<td> | ||
Directory where app logs are stored. |
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.
How about "Directory that contains application event logs to be loaded by the history server"
@witgo I like the idea of having all processes (HistoryServer, Master, Worker etc.) read from the properties file in addition just |
OK, I generally understand what you mean,I will re-implement the feature at the weekend. |
@witgo would you mind closing this since you opened another one? |
OK |
…config file This is another implementation about #1256 cc andrewor14 vanzin Author: GuoQiang Li <[email protected]> Closes #2379 from witgo/SPARK-2098-new and squashes the following commits: 4ef1cbd [GuoQiang Li] review commit 49ef70e [GuoQiang Li] Refactor getDefaultPropertiesFile c45d20c [GuoQiang Li] All Spark processes should support spark-defaults.conf, config file
* [CARMEL-6541] Support Query Level SQL Conf leveraging Hint * fix code style * fix ut * Add one more test Co-authored-by: Chao Sun <[email protected]>
No description provided.