-
Notifications
You must be signed in to change notification settings - Fork 29k
SPARK-10481 SPARK_PREPEND_CLASSES make spark-yarn related jar could n… #8649
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
|
Paste the new exception after this patch. |
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.
Nit: this line is too long (100 chars).
|
Wouldn't it make more sense to check for this in SparkSubmitArguments or maybe the yarn specific validation code? |
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 want to be more helpful, you can also mention that the user can set spark.yarn.jar to force Spark to use a specific Spark archive instead of trying to figure it out from the classpath.
I'm not sure there's a common enough place where you could call this code, at least without reflection. You could try to copy it, but duplicated code is bad. In any case, this should only really affect Spark developers, so while it's nice to have a better message, we shouldn't need to be too fancy here. |
|
ok to test |
|
@vanzin makes sense if its only going to impact spark developers (thought it might also be a user facing exception). |
|
Test build #42155 has finished for PR 8649 at commit
|
|
@zjffdu could you re-word the message? I'd suggest something like: |
|
Thanks @vanzin |
|
Test build #42194 has finished for PR 8649 at commit
|
|
LGTM, merging. |
Throw a more readable exception. Please help review. Thanks