You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dongjoon-hyun <gi...@git.apache.org> on 2017/08/26 20:31:12 UTC

[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

GitHub user dongjoon-hyun opened a pull request:

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

    [SPARK-21568][CORE] ConsoleProgressBar should only be enabled in shells

    ## What changes were proposed in this pull request?
    
    This PR disables console progress bar feature in non-shell environment by overriding the configuratino.
    
    ## How was this patch tested?
    
    Pass the Jenkins with a newly added test case.

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

    $ git pull https://github.com/dongjoon-hyun/spark SPARK-21568

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

    https://github.com/apache/spark/pull/19061.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 #19061
    
----
commit 62645d2a585fe41c6d74fec37f66af3a2a0dd0dd
Author: Dongjoon Hyun <do...@apache.org>
Date:   2017-08-26T20:15:42Z

    [SPARK-21568][CORE] ConsoleProgressBar should only be enabled in shells

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143073559
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
    @@ -399,6 +399,18 @@ class SparkSubmitSuite
         mainClass should be ("org.apache.spark.deploy.yarn.Client")
       }
     
    +  test("SPARK-21568 ConsoleProgressBar should be enabled only in shells") {
    +    val clArgs1 = Seq("--class", "org.apache.spark.repl.Main", "spark-shell")
    +    val appArgs1 = new SparkSubmitArguments(clArgs1)
    +    val (_, _, sysProps1, _) = prepareSubmitEnvironment(appArgs1)
    +    sysProps1("spark.ui.showConsoleProgress") should be ("true")
    --- End diff --
    
    `UI_SHOW_CONSOLE_PROGRESS.key`


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Thank you for review and merging, @vanzin !


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    It's done. Thank you for review, @jiangxb1987 .


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82488 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82488/testReport)** for PR 19061 at commit [`6e59cf1`](https://github.com/apache/spark/commit/6e59cf1057a1f9d2584256a3423421c9f6924da1).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Hi, @vanzin .
    Could you review this when you have sometime? I'm wondering if this is implemented correctly in a way you expected. Please let me know if there is something to do more. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #81156 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81156/testReport)** for PR 19061 at commit [`62645d2`](https://github.com/apache/spark/commit/62645d2a585fe41c6d74fec37f66af3a2a0dd0dd).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82441 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82441/testReport)** for PR 19061 at commit [`e2158ad`](https://github.com/apache/spark/commit/e2158ad13a5ff185bbe9dd26e4c00bc75db9dcfb).


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82415/
    Test PASSed.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142569400
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    --- End diff --
    
    Ur, @vanzin . It seems there is no pre-defined config for `spark.ui.XXX`.
    Is it a convention for Apache Spark? Which class do you prefer to add a new config?


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142570436
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    +        sysProps("spark.ui.showConsoleProgress") = "true"
    +      }
    +    } else {
    +      sysProps("spark.ui.showConsoleProgress") = "false"
    --- End diff --
    
    I don't see a need to prevent users from forcing this configuration if they want to.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143076481
  
    --- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
    @@ -434,7 +434,7 @@ class SparkContext(config: SparkConf) extends Logging {
         _statusTracker = new SparkStatusTracker(this)
     
         _progressBar =
    -      if (_conf.getBoolean("spark.ui.showConsoleProgress", true) && !log.isInfoEnabled) {
    +      if (_conf.getBoolean(UI_SHOW_CONSOLE_PROGRESS.key, false) && !log.isInfoEnabled) {
    --- End diff --
    
    Thanks. Do you mean this?
    ```
    if (_conf.get(UI_SHOW_CONSOLE_PROGRESS)) {
    ...
    }
    ```


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82441/
    Test PASSed.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142570065
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    +        sysProps("spark.ui.showConsoleProgress") = "true"
    +      }
    +    } else {
    +      sysProps("spark.ui.showConsoleProgress") = "false"
    --- End diff --
    
    Or, yes. It can be removed because `SparkContext` also accepts user's configuration. In that case, you're right. This is not needed.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142570193
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    +        sysProps("spark.ui.showConsoleProgress") = "true"
    +      }
    +    } else {
    +      sysProps("spark.ui.showConsoleProgress") = "false"
    --- End diff --
    
    Originally, I prefer blocking it from here. Please comment me back if this should be removed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82488 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82488/testReport)** for PR 19061 at commit [`6e59cf1`](https://github.com/apache/spark/commit/6e59cf1057a1f9d2584256a3423421c9f6924da1).


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merging to master.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82545 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82545/testReport)** for PR 19061 at commit [`ff4b8e4`](https://github.com/apache/spark/commit/ff4b8e49355e1866a9f0f337cb0c7673e13fdcaf).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143073586
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
    @@ -399,6 +399,18 @@ class SparkSubmitSuite
         mainClass should be ("org.apache.spark.deploy.yarn.Client")
       }
     
    +  test("SPARK-21568 ConsoleProgressBar should be enabled only in shells") {
    +    val clArgs1 = Seq("--class", "org.apache.spark.repl.Main", "spark-shell")
    +    val appArgs1 = new SparkSubmitArguments(clArgs1)
    +    val (_, _, sysProps1, _) = prepareSubmitEnvironment(appArgs1)
    +    sysProps1("spark.ui.showConsoleProgress") should be ("true")
    +
    +    val clArgs2 = Seq("--class", "org.SomeClass", "thejar.jar")
    +    val appArgs2 = new SparkSubmitArguments(clArgs2)
    +    val (_, _, sysProps2, _) = prepareSubmitEnvironment(appArgs2)
    +    sysProps2.keys should not contain "spark.ui.showConsoleProgress"
    --- End diff --
    
    `UI_SHOW_CONSOLE_PROGRESS.key`


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142556519
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    --- End diff --
    
    Add a config constant now you're using this in more places?


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143085886
  
    --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
    @@ -203,6 +203,10 @@ package object config {
       private[spark] val HISTORY_UI_MAX_APPS =
         ConfigBuilder("spark.history.ui.maxApplications").intConf.createWithDefault(Integer.MAX_VALUE)
     
    +  private[spark] val UI_SHOW_CONSOLE_PROGRESS = ConfigBuilder("spark.ui.showConsoleProgress")
    --- End diff --
    
    Please add document to this config using `.doc(...)`


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82415 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82415/testReport)** for PR 19061 at commit [`6f6141e`](https://github.com/apache/spark/commit/6f6141ea6e4cad61384338e56053cef9423a77b6).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82491 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82491/testReport)** for PR 19061 at commit [`ff4b8e4`](https://github.com/apache/spark/commit/ff4b8e49355e1866a9f0f337cb0c7673e13fdcaf).


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142569029
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    +        sysProps("spark.ui.showConsoleProgress") = "true"
    +      }
    +    } else {
    +      sysProps("spark.ui.showConsoleProgress") = "false"
    --- End diff --
    
    This'll prevent user's mistakes by overriding user's `spark.ui.showConsoleProgress=true` with `SparkSubmit` CLI. In case of `SparkSubmit`, only shells will accept user's `spark.ui.showConsoleProgress`.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    > Is it okay we change this behavior like this?
    
    It's not optimal, but at the same time, the existing behavior wasn't really correctly advertised anyway. If such a thing as a non-Spark repl-like application exists, it wouldn't be getting the progress bar by default, for example, because its default log level is "INFO" in Spark, something that disables the progress bar.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82488/
    Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82491/
    Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Retest this please.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Thank you for reiview, @vanzin . Yep. @jerryshao also advised me the missing point. I'll try to make it more general.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Hi, @vanzin .
    Could you review this PR?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    The PR is updated. Thank you for review, @vanzin !


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81156/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142570378
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    --- End diff --
    
    It should be added to the config package object like all other constants in core.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    >If such a thing as a non-Spark repl-like application exists, it wouldn't be getting the progress bar by default, for example, because its default log level is "INFO" in Spark, something that disables the progress bar.
    
    That makes sense!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Hi, @vanzin and @jerryshao .
    Could you review this `ConsoleProgressBar` issue when you have some time?


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142576602
  
    --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
    @@ -203,6 +203,10 @@ package object config {
       private[spark] val HISTORY_UI_MAX_APPS =
         ConfigBuilder("spark.history.ui.maxApplications").intConf.createWithDefault(Integer.MAX_VALUE)
     
    +  private[spark] val UI_SHOW_CONSOLE_PROGRESS = ConfigBuilder("spark.ui.showConsoleProgress")
    --- End diff --
    
    While updating the PR, I noticed that `internal/config/package.scala` looks more natural for this.
    
    Like [spark.ui.retainedTasks](https://github.com/apache/spark/pull/19061/files#diff-6bdad48cfc34314e89599655442ff210R198), I added here.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    I see. Thank you for the guide!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r135663178
  
    --- Diff: docs/configuration.md ---
    @@ -804,7 +804,7 @@ Apart from these, the following properties are also available, and may be useful
       <td>
         Show the progress bar in the console. The progress bar shows the progress of stages
         that run for longer than 500ms. If multiple stages run at the same time, multiple
    -    progress bars will be displayed on the same line.
    +    progress bars will be displayed on the same line. This is applied only in shells.
    --- End diff --
    
    It will still be enabled for normal apps if explicitly set in the configuration, so your update is not really correct.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82401 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82401/testReport)** for PR 19061 at commit [`a465619`](https://github.com/apache/spark/commit/a465619c6393156c65ec808000f6ab35753c27a5).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    BTW, @vanzin . I was waiting your comment because @jerryshao is worried about 3rd party relations. 
    > other repl-like projects may actually require this
    
    Is it okay we change this behavior like this?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143073506
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,11 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource) && !sparkConf.contains(UI_SHOW_CONSOLE_PROGRESS.key)) {
    --- End diff --
    
    `!sparkConf.contains(UI_SHOW_CONSOLE_PROGRESS)`


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143073447
  
    --- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
    @@ -434,7 +434,7 @@ class SparkContext(config: SparkConf) extends Logging {
         _statusTracker = new SparkStatusTracker(this)
     
         _progressBar =
    -      if (_conf.getBoolean("spark.ui.showConsoleProgress", true) && !log.isInfoEnabled) {
    +      if (_conf.getBoolean(UI_SHOW_CONSOLE_PROGRESS.key, false) && !log.isInfoEnabled) {
    --- End diff --
    
    `_conf.get(UI_SHOW_CONSOLE_PROGRESS)`


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Thank you for review, @jerryshao .
    @vanzin reported the issue in non-shell apps in the JIRA. Please refer JIRA.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143073410
  
    --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
    @@ -203,6 +203,10 @@ package object config {
       private[spark] val HISTORY_UI_MAX_APPS =
         ConfigBuilder("spark.history.ui.maxApplications").intConf.createWithDefault(Integer.MAX_VALUE)
     
    +  private[spark] val UI_SHOW_CONSOLE_PROGRESS = ConfigBuilder("spark.ui.showConsoleProgress")
    +    .booleanConf
    +    .createOptional
    --- End diff --
    
    `createWithDefault(false)`


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82401/testReport)** for PR 19061 at commit [`a465619`](https://github.com/apache/spark/commit/a465619c6393156c65ec808000f6ab35753c27a5).


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #81159 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81159/testReport)** for PR 19061 at commit [`1db4bb1`](https://github.com/apache/spark/commit/1db4bb18ff90d4dde2f3eb81cf3fe8e080b50250).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #81159 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81159/testReport)** for PR 19061 at commit [`1db4bb1`](https://github.com/apache/spark/commit/1db4bb18ff90d4dde2f3eb81cf3fe8e080b50250).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82545/
    Test PASSed.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142556541
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    +        sysProps("spark.ui.showConsoleProgress") = "true"
    +      }
    +    } else {
    +      sysProps("spark.ui.showConsoleProgress") = "false"
    --- End diff --
    
    This is not needed right?


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142571432
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    --- End diff --
    
    Thanks! Then, I'll create a `deploy/config.scala` and put it there. The followings are the existing `config.scala` files, but it seems to be irrelevant to `deploy/SparkSubmit.scala`.
    ```
    $ find . -name config.scala
    ./core/src/main/scala/org/apache/spark/deploy/history/config.scala
    ./resource-managers/mesos/src/main/scala/org/apache/spark/deploy/mesos/config.scala
    ./resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/config.scala
    ```



---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Hi, @vanzin and @jerryshao .
    Could you review this again?


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82545 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82545/testReport)** for PR 19061 at commit [`ff4b8e4`](https://github.com/apache/spark/commit/ff4b8e49355e1866a9f0f337cb0c7673e13fdcaf).


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143076954
  
    --- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
    @@ -434,7 +434,7 @@ class SparkContext(config: SparkConf) extends Logging {
         _statusTracker = new SparkStatusTracker(this)
     
         _progressBar =
    -      if (_conf.getBoolean("spark.ui.showConsoleProgress", true) && !log.isInfoEnabled) {
    +      if (_conf.getBoolean(UI_SHOW_CONSOLE_PROGRESS.key, false) && !log.isInfoEnabled) {
    --- End diff --
    
    Oh, never mind.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Hi, @vanzin and @jerryshao .
    Could you review this again when you have a chance? Thank you!


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143092704
  
    --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
    @@ -203,6 +203,10 @@ package object config {
       private[spark] val HISTORY_UI_MAX_APPS =
         ConfigBuilder("spark.history.ui.maxApplications").intConf.createWithDefault(Integer.MAX_VALUE)
     
    +  private[spark] val UI_SHOW_CONSOLE_PROGRESS = ConfigBuilder("spark.ui.showConsoleProgress")
    --- End diff --
    
    Sure!


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    @dongjoon-hyun I'm just thinking if other repl-like projects may actually require this, you changes here make them fail to leverage this feature. Did you see any issue with this feature on in non-shell apps?
    
    Also I think you should do the check in SparkConf because user can still set this with SparkConf in run-time.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82491 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82491/testReport)** for PR 19061 at commit [`ff4b8e4`](https://github.com/apache/spark/commit/ff4b8e49355e1866a9f0f337cb0c7673e13fdcaf).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142420371
  
    --- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
    @@ -434,7 +434,7 @@ class SparkContext(config: SparkConf) extends Logging {
         _statusTracker = new SparkStatusTracker(this)
     
         _progressBar =
    -      if (_conf.getBoolean("spark.ui.showConsoleProgress", true) && !log.isInfoEnabled) {
    +      if (_conf.getBoolean("spark.ui.showConsoleProgress", false) && !log.isInfoEnabled) {
    --- End diff --
    
    Now, the default value is false. However, `SparkSubmit` will inject 'true' if `spark.ui.showConsoleProgress` is not defined in our Shell environments.
    
    The original issue was the default behavior of this option. This PR solves that. If you want to stop users from using this option in order to enable `ConsoleProgressBar`, we also are able to change this config completely into some internal one, `spark.internal.ui.showConsoleProgress` in this `SparkContext` class.  In that case, `spark.ui.showConsoleProgress` will converted into `spark.internal.ui.showConsoleProgress` in `SparkSubmit`.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #81156 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81156/testReport)** for PR 19061 at commit [`62645d2`](https://github.com/apache/spark/commit/62645d2a585fe41c6d74fec37f66af3a2a0dd0dd).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82415 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82415/testReport)** for PR 19061 at commit [`6f6141e`](https://github.com/apache/spark/commit/6f6141ea6e4cad61384338e56053cef9423a77b6).


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81159/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82401/
    Test PASSed.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r143076726
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,11 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource) && !sparkConf.contains(UI_SHOW_CONSOLE_PROGRESS.key)) {
    --- End diff --
    
    And, this?
    ```
    if (isShell(args.primaryResource) && !sparkConf.contains(UI_SHOW_CONSOLE_PROGRESS)) {
    ...
    }
    ```



---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142570731
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    +        sysProps("spark.ui.showConsoleProgress") = "true"
    +      }
    +    } else {
    +      sysProps("spark.ui.showConsoleProgress") = "false"
    --- End diff --
    
    Yep. I see. Thanks!


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE][WIP] ConsoleProgressBar should only ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    **[Test build #82441 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82441/testReport)** for PR 19061 at commit [`e2158ad`](https://github.com/apache/spark/commit/e2158ad13a5ff185bbe9dd26e4c00bc75db9dcfb).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r142568710
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -598,6 +598,15 @@ object SparkSubmit extends CommandLineUtils with Logging {
           }
         }
     
    +    // In case of shells, spark.ui.showConsoleProgress can be true by default or by user.
    +    if (isShell(args.primaryResource)) {
    +      if (!sparkConf.contains("spark.ui.showConsoleProgress")) {
    --- End diff --
    
    Thank you, @vanzin . I'll create a new config for this.


---

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


[GitHub] spark pull request #19061: [SPARK-21568][CORE] ConsoleProgressBar should onl...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19061#discussion_r135663076
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -537,6 +537,11 @@ object SparkSubmit extends CommandLineUtils {
           }
         }
     
    +    // SPARK-21568 ConsoleProgressBar should be enabled only in shells
    --- End diff --
    
    I'm not a fan of comments like this. It's just repeating what the code is doing.
    
    Instead, can you change the command to mention that it's ok to do this because the user configuration is loaded later in this method and will override this value if set?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #19061: [SPARK-21568][CORE] ConsoleProgressBar should only be en...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19061
  
    Could you review this `ConsoleProgressBar` PR again, @vanzin ?


---

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