Skip to content

Conversation

@nonsleepr
Copy link

What changes were proposed in this pull request?

Catch IllegalArgumentException which might be thrown if SPARK_YARN_STAGING_DIR is not set or empty.

How was this patch tested?

Manually tested by running ApplicationMaster with SPARK_YARN_STAGING_DIR unset.

@srowen
Copy link
Member

srowen commented May 26, 2017

Why? instead of check the value of the env variable directly.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@nonsleepr
Copy link
Author

@srowen Because Path will perform multiple checks. E.g. check for null or empty string and then URI syntax exception.

@jerryshao
Copy link
Contributor

jerryshao commented May 27, 2017

I'm not sure how this could be happened, "SPARK_YARN_STAGING_DIR" is a Spark internal environment variable which should not be empty, unless you deliberately unset it.

In the JIRA you mentioned you directly running ApplicationMaster, so that you met this issue, I'm not sure why you need to directly call ApplicationMaster?

BTW seems you submitted a PR against branch 2.1, why not against master branch?

@nonsleepr nonsleepr force-pushed the spark-yarn-staging-dir branch from fe9d709 to 3c746ea Compare May 30, 2017 14:47
@nonsleepr nonsleepr changed the base branch from branch-2.1 to master May 30, 2017 14:48
@nonsleepr
Copy link
Author

@jerryshao You're right, it won't be empty if I run the app using spark-submit. The whole reason I encountered this bug is that in my project I'm running Spark job remotely (via YARN's REST API) from the node which doesn't have Spark distro.
My motivation for running it is another story but in short: I don't want to upload my keytab to HDFS to run spark-submit and instead I request delegation tokens for the services my job uses.

@srowen
Copy link
Member

srowen commented May 30, 2017

I don't think we'd change this then if it's kind of complicating the code and for a non-standard usage anyway.

@srowen srowen mentioned this pull request Jun 7, 2017
@asfgit asfgit closed this in b771fed Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants