You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgaido91 <gi...@git.apache.org> on 2018/06/10 10:51:45 UTC

[GitHub] spark pull request #21523: [SPARK-24506][UI] Add UI filters also to thriftse...

GitHub user mgaido91 opened a pull request:

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

    [SPARK-24506][UI] Add UI filters also to thriftserver tab

    ## What changes were proposed in this pull request?
    
    Currently, `spark.ui.filters` are not applied to the pages added in the thriftserver tab. This happens because the tab is attached after the UI is started. This can allow unauthorized access to the pages.
    
    The PR adds the filters also to the pages which are added by the thriftserver tab.
    
    ## How was this patch tested?
    
    manual tests (without the patch, starting the thriftserver with `--conf spark.ui.filters=org.apache.hadoop.security.authentication.server.AuthenticationFilter --conf spark.org.apache.hadoop.security.authentication.server.AuthenticationFilter.params="type=simple"` you can access `http://localhost:4040/sqlserver`; with the patch, 401 is the response as for the other pages).


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

    $ git pull https://github.com/mgaido91/spark SPARK-24506

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

    https://github.com/apache/spark/pull/21523.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 #21523
    
----
commit 086a4f62b95b455e9e789ea820b0e1f8b7e8d643
Author: Marco Gaido <ma...@...>
Date:   2018-06-10T10:44:01Z

    [SPARK-24506][UI] Add UI filters also to thriftserver tab

----


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    retest this please


---

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


[GitHub] spark pull request #21523: [SPARK-24506][UI] Add UI filters also to thriftse...

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

    https://github.com/apache/spark/pull/21523#discussion_r194488465
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/WebUI.scala ---
    @@ -91,7 +91,12 @@ private[spark] abstract class WebUI(
       /** Attach a handler to this UI. */
       def attachHandler(handler: ServletContextHandler) {
         handlers += handler
    -    serverInfo.foreach(_.addHandler(handler))
    +    serverInfo.foreach { sInfo =>
    +      sInfo.addHandler(handler)
    +      // If the UI has already been bound, we need to add the filters to the newly attached
    +      // handlers. Otherwise, they will be attached when binding.
    +      addFilters(Seq(handler), conf)
    --- End diff --
    
    Nevermind you already removed it.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91723 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91723/testReport)** for PR 21523 at commit [`02a9f4e`](https://github.com/apache/spark/commit/02a9f4e444b3c587733dde08560ed078ff4f3c50).


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91632 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91632/testReport)** for PR 21523 at commit [`086a4f6`](https://github.com/apache/spark/commit/086a4f62b95b455e9e789ea820b0e1f8b7e8d643).
     * 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 #21523: [SPARK-24506][UI] Add UI filters also to thriftse...

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

    https://github.com/apache/spark/pull/21523#discussion_r194722157
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/WebUI.scala ---
    @@ -91,7 +91,12 @@ private[spark] abstract class WebUI(
       /** Attach a handler to this UI. */
       def attachHandler(handler: ServletContextHandler) {
         handlers += handler
    -    serverInfo.foreach(_.addHandler(handler))
    +    serverInfo.foreach { sInfo =>
    --- End diff --
    
    I see, @vanzin , thanks for checking this. Do you have this WIP as a PR? If so, should I close this or updating according to your suggestion?


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    Merging to master / 2.3 (will try back to 2.1).


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    you're right @jerryshao, thanks. The reason why I didn't do a general fix on first instance was that this would apply also to the metrics and I was not it was right. But thinking more carefully now I see that it is indeed the right thing to do, as we have to make sure that unauthorized users have no access to any data. Thanks.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91723 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91723/testReport)** for PR 21523 at commit [`02a9f4e`](https://github.com/apache/spark/commit/02a9f4e444b3c587733dde08560ed078ff4f3c50).
     * 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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91659 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91659/testReport)** for PR 21523 at commit [`08cd576`](https://github.com/apache/spark/commit/08cd576f71b9ed4e7f6af62c2d90b3a650f6d82c).


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3891/
    Test FAILed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    I think here the problem is that `filters` are only added for those handler who added before server starts.
    
    This may not be the only problem for `ThriftserverTab`, I guess `SQLTable` or `StreamingTab` may also have the similar problem, we should have a general fix for it.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3892/
    Test PASSed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    @mgaido91 Please fix the PR title and description to reflect the new changes you made.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91658 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91658/testReport)** for PR 21523 at commit [`0ac1848`](https://github.com/apache/spark/commit/0ac1848efcb5d95ea3b3ab29aca8ff14dcc17dd2).
     * 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 #21523: [SPARK-24506][UI] Add UI filters also to thriftse...

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

    https://github.com/apache/spark/pull/21523#discussion_r194488116
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/WebUI.scala ---
    @@ -91,7 +91,12 @@ private[spark] abstract class WebUI(
       /** Attach a handler to this UI. */
       def attachHandler(handler: ServletContextHandler) {
         handlers += handler
    -    serverInfo.foreach(_.addHandler(handler))
    +    serverInfo.foreach { sInfo =>
    +      sInfo.addHandler(handler)
    +      // If the UI has already been bound, we need to add the filters to the newly attached
    +      // handlers. Otherwise, they will be attached when binding.
    +      addFilters(Seq(handler), conf)
    --- End diff --
    
    There's a call to `addFilters` in `HistoryServer.scala` that becomes redundant with this code, I think.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91711 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91711/testReport)** for PR 21523 at commit [`02a9f4e`](https://github.com/apache/spark/commit/02a9f4e444b3c587733dde08560ed078ff4f3c50).
     * This patch **fails Spark unit 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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    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 #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3885/
    Test PASSed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/59/
    Test PASSed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/47/
    Test PASSed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91658 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91658/testReport)** for PR 21523 at commit [`0ac1848`](https://github.com/apache/spark/commit/0ac1848efcb5d95ea3b3ab29aca8ff14dcc17dd2).


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91711 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91711/testReport)** for PR 21523 at commit [`02a9f4e`](https://github.com/apache/spark/commit/02a9f4e444b3c587733dde08560ed078ff4f3c50).


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    LGTM pending tests.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    cc @jerryshao @vanzin 


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91658/
    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 #21523: [SPARK-24506][UI] Add UI filters to tabs added af...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    2.1 had non-trivial conflicts. Probably not worth it in any case.


---

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


[GitHub] spark pull request #21523: [SPARK-24506][UI] Add UI filters also to thriftse...

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

    https://github.com/apache/spark/pull/21523#discussion_r194490802
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/WebUI.scala ---
    @@ -91,7 +91,12 @@ private[spark] abstract class WebUI(
       /** Attach a handler to this UI. */
       def attachHandler(handler: ServletContextHandler) {
         handlers += handler
    -    serverInfo.foreach(_.addHandler(handler))
    +    serverInfo.foreach { sInfo =>
    --- End diff --
    
    I have a slight preference for doing this in the `ServerInfo` class, so that we're sure we're covering all paths.
    
    I have a WIP patch for another thing where I did that, something like this:
    
    ```
    @@ -507,17 +517,19 @@ private[spark] case class ServerInfo(
         server: Server,
         boundPort: Int,
         securePort: Option[Int],
    +    private val conf: SparkConf,
         private val rootHandler: ContextHandlerCollection) {
     
    -  def addHandler(handler: ContextHandler): Unit = {
    +  def addHandler(handler: ServletContextHandler): Unit = {
         handler.setVirtualHosts(JettyUtils.toVirtualHosts(JettyUtils.SPARK_CONNECTOR_NAME))
    +    JettyUtils.addFilters(Seq(handler), conf)
         rootHandler.addHandler(handler)
         if (!handler.isStarted()) {
           handler.start()
         }
       }
    ```
    
    It also avoids a race where an already started handler would be added before filters are installed. Extremely unlikely that would be a problem, but well.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3948/
    Test PASSed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters to tabs added after bin...

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

    https://github.com/apache/spark/pull/21523
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3937/
    Test PASSed.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91659 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91659/testReport)** for PR 21523 at commit [`08cd576`](https://github.com/apache/spark/commit/08cd576f71b9ed4e7f6af62c2d90b3a650f6d82c).
     * 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 #21523: [SPARK-24506][UI] Add UI filters to tabs added af...

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

    https://github.com/apache/spark/pull/21523#discussion_r194814773
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/WebUI.scala ---
    @@ -91,7 +91,12 @@ private[spark] abstract class WebUI(
       /** Attach a handler to this UI. */
       def attachHandler(handler: ServletContextHandler) {
         handlers += handler
    -    serverInfo.foreach(_.addHandler(handler))
    +    serverInfo.foreach { sInfo =>
    --- End diff --
    
    I'm still working on it since it touches a bunch of other things, not just this. This is fine as is, it's better to have a more targeted fix here and leave my PR just as refactoring.


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

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


---

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


[GitHub] spark issue #21523: [SPARK-24506][UI] Add UI filters also to thriftserver ta...

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

    https://github.com/apache/spark/pull/21523
  
    **[Test build #91632 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91632/testReport)** for PR 21523 at commit [`086a4f6`](https://github.com/apache/spark/commit/086a4f62b95b455e9e789ea820b0e1f8b7e8d643).


---

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