[SPARK-1516]Throw exception in yarn client instead of run system.exit directly. #490
Conversation
|
Can one of the admins verify this patch? |
|
Jenkins, add to whitelist. |
|
Jenkins, test this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
This change makes sense to me. |
…sdefined Replace the check for None Option with isDefined and isEmpty in Scala code Propose to replace the Scala check for Option "!= None" with Option.isDefined and "=== None" with Option.isEmpty. I think this, using method call if possible then operator function plus argument, will make the Scala code easier to read and understand. Pass compile and tests.
|
I think this change would be good too. I think we should also look into what our exit code is and how it would fix into workflow managers like oozie. |
|
This looks good to me. However, we still have more of System.exit in different deployment code; we probably want to review and fix them. This can be a good step! |
|
@codeboyyong It is not mergable now. Do you mind merging the master branch and also create a separate PR for branch-0.9? |
|
I merged it to the master now. Will do 0.9 soon |
|
@mengxr Do you think it's in good shape now? This is the only issue blocking us using vanilla spark. Thanks. |
… directly. All the changes is in the package of "org.apache.spark.deploy.yarn": 1) Add a ClientException with an exitCode 2) Throws exception in ClinetArguments and ClientBase instead of exit directly 3) in Client's main method, catch exception and exit with the exitCode. After the fix, if user integrate the spark yarn cline into their applications, when the argument is wrong or the running is finished, the application will not exit. And the exit only happens in command line running.
|
@mengxr, I made the change based on your comments. |
|
Jenkins, add to whitelist. |
|
Merged build triggered. |
|
Jenkins, test this please. |
|
Merged build started. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
1 similar comment
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
All automated tests passed. |
|
@codeboyyong I've merged this. Could you please make a patch for branch-0.9? Thanks! |
|
Sure, will do this at weekend. On Jun 12, 2014, at 10:13 PM, Xiangrui Meng [email protected] wrote:
|
… directly.
All the changes is in the package of "org.apache.spark.deploy.yarn":
1) Throw exception in ClinetArguments and ClientBase instead of exit directly.
2) in Client's main method, if exception is caught, it will exit with code 1, otherwise exit with code 0.
After the fix, if user integrate the spark yarn client into their applications, when the argument is wrong or the running is finished, the application won't be terminated.
Author: John Zhao <[email protected]>
Closes apache#490 from codeboyyong/jira_1516_systemexit_inyarnclient and squashes the following commits:
138cb48 [John Zhao] [SPARK-1516]Throw exception in yarn clinet instead of run system.exit directly. All the changes is in the package of "org.apache.spark.deploy.yarn": 1) Add a ClientException with an exitCode 2) Throws exception in ClinetArguments and ClientBase instead of exit directly 3) in Client's main method, catch exception and exit with the exitCode.
… directly.
All the changes is in the package of "org.apache.spark.deploy.yarn":
1) Throw exception in ClinetArguments and ClientBase instead of exit directly.
2) in Client's main method, if exception is caught, it will exit with code 1, otherwise exit with code 0.
After the fix, if user integrate the spark yarn client into their applications, when the argument is wrong or the running is finished, the application won't be terminated.
Author: John Zhao <[email protected]>
Closes apache#490 from codeboyyong/jira_1516_systemexit_inyarnclient and squashes the following commits:
138cb48 [John Zhao] [SPARK-1516]Throw exception in yarn clinet instead of run system.exit directly. All the changes is in the package of "org.apache.spark.deploy.yarn": 1) Add a ClientException with an exitCode 2) Throws exception in ClinetArguments and ClientBase instead of exit directly 3) in Client's main method, catch exception and exit with the exitCode.
…sdefined Replace the check for None Option with isDefined and isEmpty in Scala code Propose to replace the Scala check for Option "!= None" with Option.isDefined and "=== None" with Option.isEmpty. I think this, using method call if possible then operator function plus argument, will make the Scala code easier to read and understand. Pass compile and tests. (cherry picked from commit f16c21e) Signed-off-by: Patrick Wendell <[email protected]>
Merge from upstream
Comment out bazel related roles, we use cloud image for KinD test, previously the job fails because of bazel installation, the cloud image uploaded included a successfuly built bazel. lets skip the bazel installation and make the job running first. We can refactor the bazel role latter if required. Related: theopenlab/openlab#230
… the lock by default for CTAS to reduce HDFS operations (apache#490)
7 participants
All the changes is in the package of "org.apache.spark.deploy.yarn":
1) Throw exception in ClinetArguments and ClientBase instead of exit directly.
2) in Client's main method, if exception is caught, it will exit with code 1, otherwise exit with code 0.
After the fix, if user integrate the spark yarn client into their applications, when the argument is wrong or the running is finished, the application won't be terminated.