[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/17430


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108616336
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -148,6 +148,17 @@ class SparkSubmitSuite
 appArgs.childArgs should be (Seq("--master", "local", "some", 
"--weird", "args"))
   }
 
+  test("print the right queue name") {
+val clArgs = Seq(
+  "--name", "myApp",
+  "--class", "Foo",
+  "--conf", "spark.yarn.queue=thequeue",
+  "userjar.jar")
+val appArgs = new SparkSubmitArguments(clArgs)
+appArgs.queue should be ("thequeue")
+appArgs.toString.contains("thequeue") should be (true)
--- End diff --

OK to go


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108594377
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -148,6 +148,17 @@ class SparkSubmitSuite
 appArgs.childArgs should be (Seq("--master", "local", "some", 
"--weird", "args"))
   }
 
+  test("print the right queue name") {
+val clArgs = Seq(
+  "--name", "myApp",
+  "--class", "Foo",
+  "--conf", "spark.yarn.queue=thequeue",
+  "userjar.jar")
+val appArgs = new SparkSubmitArguments(clArgs)
+appArgs.queue should be ("thequeue")
+appArgs.toString.contains("thequeue") should be (true)
--- End diff --

@yaooqinn while you're here do you want to make this last change?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[felixcheung]

Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108333530
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -148,6 +148,17 @@ class SparkSubmitSuite
 appArgs.childArgs should be (Seq("--master", "local", "some", 
"--weird", "args"))
   }
 
+  test("print the right queue name") {
+val clArgs = Seq(
+  "--name", "myApp",
+  "--class", "Foo",
+  "--conf", "spark.yarn.queue=thequeue",
+  "userjar.jar")
+val appArgs = new SparkSubmitArguments(clArgs)
+appArgs.queue should be ("thequeue")
+appArgs.toString.contains("thequeue") should be (true)
--- End diff --

use `should include ` instead?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108094996
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

OK, orNull seems be more reasonable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[jerryshao]

Github user jerryshao commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108092473
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

I would be in favor of `orNull` instead of "default". 
`SparkSubmitArguments` reflects the arguments we set, since we don't set it, I 
think it would be better to be `null`. You could see other configurations here 
like `spark.driver.memory`, `spark.deploy.mode`, etc. Most of them indeed have 
default values, but here still use `null`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108061061
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

would you plz trigger the test? thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108055529
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

agree. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[felixcheung]

Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108052884
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

ok, I'm keep it that way for consistency, just saying that the default 
queue name is not necessarily "default" either.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108051074
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

The default value of `spark.yarn.queue` is `defualt`, I think we should 
follow that. Here we just print the spark submit arguments from command line 
and the conf file. If the queue of change by user later, they should be aware 
of that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108051067
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -307,7 +308,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
 |  driverExtraLibraryPath  $driverExtraLibraryPath
 |  driverExtraJavaOptions  $driverExtraJavaOptions
 |  supervise   $supervise
-|  queue   $queue
+|  queue   $queue(YARN Only)
--- End diff --

I'm going to  rm it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[felixcheung]

Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108046703
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -307,7 +308,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
 |  driverExtraLibraryPath  $driverExtraLibraryPath
 |  driverExtraJavaOptions  $driverExtraJavaOptions
 |  supervise   $supervise
-|  queue   $queue
+|  queue   $queue(YARN Only)
--- End diff --

the `(YARN Only)` format seems a bit unusual here? is there any other YARN 
only argument and how are they documented?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[felixcheung]

Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/17430#discussion_r108046680
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala ---
@@ -190,6 +190,7 @@ private[deploy] class SparkSubmitArguments(args: 
Seq[String], env: Map[String, S
   .orNull
 numExecutors = Option(numExecutors)
   .getOrElse(sparkProperties.get("spark.executor.instances").orNull)
+queue = 
Option(queue).orElse(sparkProperties.get("spark.yarn.queue")).getOrElse("default")
--- End diff --

instead of `.getOrElse("default")`, i think it should be `orNull` - we 
don't know the default queue name of the system since it could change?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #17430: [SPARK-20096][Spark Submit][Minor]Expose the righ...

[yaooqinn]

GitHub user yaooqinn opened a pull request:

https://github.com/apache/spark/pull/17430

[SPARK-20096][Spark Submit][Minor]Expose the right queue name not null if 
set by --conf or configure file 

## What changes were proposed in this pull request?

while submit apps with -v or --verbose， we can print the right queue 
name, but if we set a queue name with `spark.yarn.queue` by --conf or in the 
spark-default.conf, we just got `null`

## How was this patch tested?

ut manully

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/yaooqinn/spark SPARK-20096

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/17430.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #17430


commit 1facc5ec45e40bf7fb916fe598c2fc0c69d3d7f3
Author: Kent Yao 
Date:   2017-03-25T11:36:29Z

expose queue set by --conf or spark-default.conf




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org