[SPARK-20365][YARN] Remove LocalSchem when add path to ClassPath. #18129
Conversation
liyichao
changed the title
liyichao
changed the title
| if (LOCAL_SCHEME.equals(uri.getScheme())) { | ||
| cp should contain (uri.getPath()) | ||
| if (LOCAL_SCHEME.equals(uri.getScheme)) { | ||
| cp should contain (uri.getPath) |
There was a problem hiding this comment.
Maybe it is not so necessary to change the previous style.
There was a problem hiding this comment.
It is Intellij that advises the above changes. I will revert them.
|
CC @vanzin to take a review. |
vanzin
left a comment
vanzin
left a comment
There was a problem hiding this comment.
Style nit, otherwise looks good pending tests.
| cp should contain (uri.getPath()) | ||
| } else { | ||
| cp should not contain (uri.getPath()) | ||
| cp should not contain uri.getPath() |
There was a problem hiding this comment.
Could you avoid these noisy changes and also keep the existing style of using parentheses in the code you're actually adding?
|
ok to test |
|
Test build #77596 has finished for PR 18129 at commit
|
|
Test build #77623 has finished for PR 18129 at commit
|
| cp should not contain (uri.getPath()) | ||
| } | ||
| }) | ||
| cp should not contain "local" |
There was a problem hiding this comment.
Can you please add parentheses here ("local").
|
Test build #77645 has finished for PR 18129 at commit
|
|
Merging to master / 2.2. |
In Spark on YARN, when configuring "spark.yarn.jars" with local jars (jars started with "local" scheme), we will get inaccurate classpath for AM and containers. This is because we don't remove "local" scheme when concatenating classpath. It is OK to run because classpath is separated with ":" and java treat "local" as a separate jar. But we could improve it to remove the scheme. Updated `ClientSuite` to check "local" is not in the classpath. cc jerryshao Author: Li Yichao <[email protected]> Author: Li Yichao <[email protected]> Closes #18129 from liyichao/SPARK-20365. (cherry picked from commit 640afa4) Signed-off-by: Marcelo Vanzin <[email protected]>
|
@liyichao please update the bug with your JIRA handle if you want it assigned to you. |
4 participants
What changes were proposed in this pull request?
In Spark on YARN, when configuring "spark.yarn.jars" with local jars (jars started with "local" scheme), we will get inaccurate classpath for AM and containers. This is because we don't remove "local" scheme when concatenating classpath. It is OK to run because classpath is separated with ":" and java treat "local" as a separate jar. But we could improve it to remove the scheme.
How was this patch tested?
Updated
ClientSuiteto check "local" is not in the classpath.cc @jerryshao