You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by deanchen <gi...@git.apache.org> on 2015/04/12 09:13:30 UTC

[GitHub] spark pull request: [SPARK-6868][YARN] Fix broken container log li...

GitHub user deanchen opened a pull request:

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

    [SPARK-6868][YARN] Fix broken container log link on executor page when HTTPS_ONLY.

    Correct http schema in YARN container log link in Spark UI when container logs when YARN is configured to be HTTPS_ONLY.

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

    $ git pull https://github.com/deanchen/spark container-url

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

    https://github.com/apache/spark/pull/5477.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 #5477
    
----
commit 13d40727e95fd9511bcefbbdd2f4e0050e99567b
Author: Dean Chen <de...@gmail.com>
Date:   2015-04-12T05:57:10Z

    Correct http schema in YARN container log link in Spark UI when container logs when YARN is configured to be HTTPS_ONLY.

----


---
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: [SPARK-6868][YARN] Fix broken container log li...

Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:

    https://github.com/apache/spark/pull/5477#issuecomment-92235067
  
    This LGTM as well


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#discussion_r28204626
  
    --- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnable.scala ---
    @@ -290,10 +290,19 @@ class ExecutorRunnable(
           YarnSparkHadoopUtil.setEnvFromInputString(env, userEnvs)
         }
     
    +    // lookup appropriate http scheme for container log urls
    +    val yarnHttpPolicy = yarnConf.get(
    +      YarnConfiguration.YARN_HTTP_POLICY_KEY,
    --- End diff --
    
    Yes, seems OK. We only support YARN 2.2+ now. LGTM.


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#issuecomment-92032463
  
      [Test build #30117 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30117/consoleFull) for   PR 5477 at commit [`91d3090`](https://github.com/apache/spark/commit/91d30901e833de808ba05e7dfa775aa0380aad5f).


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#discussion_r28204600
  
    --- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnable.scala ---
    @@ -290,10 +290,19 @@ class ExecutorRunnable(
           YarnSparkHadoopUtil.setEnvFromInputString(env, userEnvs)
         }
     
    +    // lookup appropriate http scheme for container log urls
    +    val yarnHttpPolicy = yarnConf.get(
    +      YarnConfiguration.YARN_HTTP_POLICY_KEY,
    --- End diff --
    
    YARN_HTTP_POLICY_KEY and YARN_HTTP_POLICY_DEFAULT were part of the initial 2.2 release: [YARN-1277](https://issues.apache.org/jira/browse/YARN-1277)([commit](https://github.com/apache/hadoop/commit/21181b65531449e5fda321c11f0672c3067641aa#diff-6b291e39bccffdc169e769d0b09310c8R49)). 


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#issuecomment-92002852
  
    Can one of the admins verify this patch?


---
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: [SPARK-6868][YARN] Fix broken container log li...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:

    https://github.com/apache/spark/pull/5477#issuecomment-92032014
  
    ok to test


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#discussion_r28202475
  
    --- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnable.scala ---
    @@ -290,10 +290,19 @@ class ExecutorRunnable(
           YarnSparkHadoopUtil.setEnvFromInputString(env, userEnvs)
         }
     
    +    // lookup appropriate http scheme for container log urls
    +    val yarnHttpPolicy = yarnConf.get(
    +      YarnConfiguration.YARN_HTTP_POLICY_KEY,
    --- End diff --
    
    Do you happen to know when these two fields were added to `YarnConfiguration`? Just want to make sure this isn't introducing an incompatibility with old versions. That said I see this in at least 2.2.0 so probably fine. LGTM.


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#issuecomment-92056486
  
      [Test build #30117 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30117/consoleFull) for   PR 5477 at commit [`91d3090`](https://github.com/apache/spark/commit/91d30901e833de808ba05e7dfa775aa0380aad5f).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-6868][YARN] Fix broken container log li...

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

    https://github.com/apache/spark/pull/5477#issuecomment-92056488
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30117/
    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: [SPARK-6868][YARN] Fix broken container log li...

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

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


---
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