[SPARK-2872] Fix conflict between code and doc in YarnClientSchedulerBackend.scala - #1684
[SPARK-2872] Fix conflict between code and doc in YarnClientSchedulerBackend.scala#1684li-zhihui wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch? |
|
@tgravescs can you please to have a look at this patch? |
|
At this point we have released it with env variable overriding configs. I think it would be better to just update the comment (since its just in the code and not user facing) to say what it does vs changing the user behavior. |
|
@tgravescs got it. But, |
|
@tgravescs I rollback previous commit and add a new commit just update comment. |
|
@li-zhihui good point about YarnClusterSchedulerBackend, Taking a deeper look there is YarnClusterSchedulerBackend and then there are a couple other places in which the conf overrides the ENV variable. So lets keep it consistent and go with your original change. Sorry for the extra work. |
|
can you also please file a jira for this and link the PR to it. |
|
@tgravescs done |
|
Jenkins, test this please |
|
QA tests have started for PR 1684. This patch merges cleanly. |
|
@tgravescs Can you ask jenkins test this again? |
|
sorry I haven't had time to get back to this. Originally there were some issues with the order and make sure all the options worked properly from all the various ways to submit - directly using spark-class and then using spark-submit. I believe one of them was the SPARK_YARN_APP_NAME. I think other things have been fixed where this shouldn't be an issue anymore but need to verify. Can you test to make sure they all work, see #539. |
|
Can one of the admins verify this patch? |
|
Note this is going to conflict with #2350 |
Doc say: system properties override environment variables.
https://github.com/apache/spark/blob/master/yarn/common/src/main/scala/org/apache/spark/scheduler/cluster/YarnClientSchedulerBackend.scala#L71
But code is conflict with it.