[SPARK-4623]Add the some error infomation if using spark-sql in yarn-cluster mode #3479
Conversation
| @@ -142,6 +142,8 @@ object SparkSubmit { | |||
| printErrorAndExit("Cluster deploy mode is currently not supported for python applications.") | |||
| case (_, CLUSTER) if isShell(args.primaryResource) => | |||
| printErrorAndExit("Cluster deploy mode is not applicable to Spark shells.") | |||
| case (_, CLUSTER) if isSqlShell(args.mainClass) => | |||
| printErrorAndExit("Cluster deploy mode is not applicable to Spark Sql shells.") | |||
There was a problem hiding this comment.
“shells" should be in singular form.
| * Return whether the given main class represents a sql shell. | ||
| */ | ||
| private[spark] def isSqlShell(mainClass: String): Boolean = { | ||
| mainClass == "org.apache.spark.sql.hive.thriftserver.SparkSQLCLIDriver" |
There was a problem hiding this comment.
Can you add comment int he SparkSQLCLIDriver class that explains there is a dependency here on the exact name? Otherwise that class could be renamed and this would break inadvertently.
|
I had a small comment to make this less prone to being broken. Looks good other than that comment. |
|
add to whitelist. This LGTM |
| @@ -142,6 +142,8 @@ object SparkSubmit { | |||
| printErrorAndExit("Cluster deploy mode is currently not supported for python applications.") | |||
| case (_, CLUSTER) if isShell(args.primaryResource) => | |||
| printErrorAndExit("Cluster deploy mode is not applicable to Spark shells.") | |||
| case (_, CLUSTER) if isSqlShell(args.mainClass) => | |||
| printErrorAndExit("Cluster deploy mode is not applicable to Spark Sql shell.") | |||
There was a problem hiding this comment.
I think elsewhere we use SQL (all caps). Maybe we should say "not applicable to the Spark SQL shell" here
|
Test build #23953 has finished for PR 3479 at commit
|
| @@ -23,6 +23,7 @@ | |||
| # Enter posix mode for bash | |||
| set -o posix | |||
|
|
|||
| # SparkSQLCLIDriver class name was used in SparkSubmit. Be careful about renaming it. | |||
There was a problem hiding this comment.
A minor improvement:
# NOTE: This exact class name is matched downstream by SparkSubmit. Any changes need to be reflected there.
|
Just proposed a minor change to the comment. If you feel the current one is better you can leave it there. Either way LGTM. |
|
Test build #23958 has finished for PR 3479 at commit
|
|
Test build #23959 has finished for PR 3479 at commit
|
|
LGTM - thanks I'll pull this in |
|
Thanks I merged this. I couldn't find you on the ASF jira to give you credit as the author of this. Can you comment here so that I can make you the assignee? |
|
Thanks for mergeing. The jira was also reported by me, and I had rename it as github. |
If using spark-sql in yarn-cluster mode, print an error infomation just as the spark shell in yarn-cluster mode.