You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by WangTaoTheTonic <gi...@git.apache.org> on 2014/12/29 09:20:34 UTC

[GitHub] spark pull request: [SPARK-4990]to find default properties file, s...

GitHub user WangTaoTheTonic opened a pull request:

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

    [SPARK-4990]to find default properties file, search SPARK_CONF_DIR first

    https://issues.apache.org/jira/browse/SPARK-4990

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

    $ git pull https://github.com/WangTaoTheTonic/spark SPARK-4990

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

    https://github.com/apache/spark/pull/3823.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 #3823
    
----
commit c5a85eb37389f3c849129267fcef0dfa608d09c6
Author: WangTaoTheTonic <ba...@aliyun.com>
Date:   2014-12-29T08:17:32Z

    to find default properties file, search SPARK_CONF_DIR first

----


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68322885
  
      [Test build #24875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24875/consoleFull) for   PR 3823 at commit [`07b9ebf`](https://github.com/apache/spark/commit/07b9ebf45cb9cf06cc3347aff829ae6d77562f87).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22306164
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    --- End diff --
    
    `SparkSubmitArguments` already ultimately handles this case, right? What does this fix?


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22307986
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    +if [ ! -f "DEFAULT_PROPERTIES_FILE" ]; then
    +  DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +fi
     export SPARK_SUBMIT_DEPLOY_MODE=${SPARK_SUBMIT_DEPLOY_MODE:-"client"}
     export SPARK_SUBMIT_PROPERTIES_FILE=${SPARK_SUBMIT_PROPERTIES_FILE:-"$DEFAULT_PROPERTIES_FILE"}
    --- End diff --
    
    If user didn't pass `--properties-file` and define SPARK_CONF_DIR while not SPARK_HOME, then spark-submit will see if `spark.driver.extra*` in directory specified by `DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"`. Obviously it will make the wrong judgement when SPARK_CONF_DIR does not equal `$SPARK_HOME/conf`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69431301
  
      [Test build #25347 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25347/consoleFull) for   PR 3823 at commit [`b1ab402`](https://github.com/apache/spark/commit/b1ab402a0a835a642c99064fc0fa3d4a320b8b94).
     * 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22308280
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    +if [ ! -f "DEFAULT_PROPERTIES_FILE" ]; then
    +  DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +fi
     export SPARK_SUBMIT_DEPLOY_MODE=${SPARK_SUBMIT_DEPLOY_MODE:-"client"}
     export SPARK_SUBMIT_PROPERTIES_FILE=${SPARK_SUBMIT_PROPERTIES_FILE:-"$DEFAULT_PROPERTIES_FILE"}
    --- End diff --
    
    OK I see the scenario now where this matters. So the default config for an installation might in fact set these special properties, which the script needs to handle before `SparkSubmit` starts. Maybe someone else can double-check, but that makes sense to me.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22308521
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    +if [ ! -f "DEFAULT_PROPERTIES_FILE" ]; then
    +  DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +fi
     export SPARK_SUBMIT_DEPLOY_MODE=${SPARK_SUBMIT_DEPLOY_MODE:-"client"}
     export SPARK_SUBMIT_PROPERTIES_FILE=${SPARK_SUBMIT_PROPERTIES_FILE:-"$DEFAULT_PROPERTIES_FILE"}
    --- End diff --
    
    I mean there is a possibility that user don't use the conf sub-directory default installation location but specified.
    For instance, I touch a spark-defaults.conf under `/etc/my-spark/` and use it for submitting applications, so I set SPARK_CONF_DIR to `/etc/my-spark/` to make the properties file work.
    The properties file under `$SPARK_HOME/conf` could be unused or used for submitting other applications.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69431707
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25348/
    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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69429218
  
    Ok I'm merging this into master since tests are irrelevant here 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68335760
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24883/
    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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69282780
  
      [Test build #25286 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25286/consoleFull) for   PR 3823 at commit [`4cc7f34`](https://github.com/apache/spark/commit/4cc7f3467ed78bb4b3a1a404c0b1daf1bd009c35).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68325790
  
      [Test build #24875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24875/consoleFull) for   PR 3823 at commit [`07b9ebf`](https://github.com/apache/spark/commit/07b9ebf45cb9cf06cc3347aff829ae6d77562f87).
     * 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

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


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22730425
  
    --- Diff: bin/spark-submit ---
    @@ -44,7 +44,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +if [ ! -d "$SPARK_CONF_DIR" ]; then
    --- End diff --
    
    i.e.
    ```
    if [ -z "$SPARK_CONF_DIR" ]; then
      export SPARK_CONF_DIR="$SPARK_HOME/conf"
    fi
    ```


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68244186
  
      [Test build #24857 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24857/consoleFull) for   PR 3823 at commit [`c5a85eb`](https://github.com/apache/spark/commit/c5a85eb37389f3c849129267fcef0dfa608d09c6).
     * 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68257882
  
    @OopsOutOfMemory Thanks for your comment and I understand your concern.
    Actually it doesn't matter that if SPARK_CONF_DIR does not exist here because we can use `$SPARK_HOME/conf` instead. And checking logic of properties file contains that of SPARK_CONF_DIR.
    In other places of spark codes, we usually use a `getOrElse` logic to handle configuration.
    It is easy to analyse when use got some specific config wrong and we'd better not to broke this tradition. :)


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68324325
  
      [Test build #24877 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24877/consoleFull) for   PR 3823 at commit [`d8d3cb7`](https://github.com/apache/spark/commit/d8d3cb7972422361c538c3f2cc2c4126e1826fcd).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68327498
  
      [Test build #24877 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24877/consoleFull) for   PR 3823 at commit [`d8d3cb7`](https://github.com/apache/spark/commit/d8d3cb7972422361c538c3f2cc2c4126e1826fcd).
     * 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68332440
  
      [Test build #24883 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24883/consoleFull) for   PR 3823 at commit [`55300bc`](https://github.com/apache/spark/commit/55300bcd566b53d0b9a4a579aa3074c7c3ac66b9).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22337458
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +if [ ! -d "$SPARK_CONF_DIR" ]; then
    +  SPARK_CONF_DIR="$SPARK_HOME/conf"
    --- End diff --
    
    I recommend to add `export` keyword to make the SPARK_CONF_DIR global :)
    ```
     export SPARK_CONF_DIR="$SPARK_HOME/conf"
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22730354
  
    --- Diff: bin/spark-submit ---
    @@ -44,7 +44,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +if [ ! -d "$SPARK_CONF_DIR" ]; then
    --- End diff --
    
    actually, maybe this should be "if not defined" instead of "if not exists". Otherwise if the user sets `SPARK_CONF_DIR` to a directory that doesn't exist then this will silently use a different config.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69244069
  
    LGTM, but this seems to have a conflict from your other patch now. Could you rebase to master?


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69431314
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25347/
    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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69287260
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25286/
    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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22306481
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    --- End diff --
    
    Used for 
    
    >if [[ "$SPARK_SUBMIT_DEPLOY_MODE" == "client" && -f "$SPARK_SUBMIT_PROPERTIES_FILE" ]]; then
      # Parse the properties file only if the special configs exist
      contains_special_configs=$(
        grep -e "spark.driver.extra*\|spark.driver.memory" "$SPARK_SUBMIT_PROPERTIES_FILE" | \
        grep -v "^[[:space:]]*#"
      )
      if [ -n "$contains_special_configs" ]; then
        export SPARK_SUBMIT_BOOTSTRAP_DRIVER=1
      fi
    fi
    



---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68261526
  
    Sorry, maybe here is a misunderstanding.
    What I mean is to __`change` the `checking logic of properties file`__ instead of __`checking whether the SPARK_CONF_DIR   is `user-specifc` or `default` . But not to add an extra checking directory here .
    Let me raise an example:
    ```
    if [ ! -d "$SPARK_CONF_DIR" ]; then
       export SPARK_CONF_DIR = $SPARK_HOME/conf
    fi
    DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    XXX_PROPERTIES_FILE = "$SPARK_CONF_DIR/xxx.conf
    ```
    To check conf directory is more reasonable, because the key point is the  `SPARK_CONF_DIR `.  The original concern here is to change the `path` of `SPARK_CONF_DIR` but not only `spark-default.conf`.
    
    
    > analyse when use got some specific config wrong
    
    We may add an extra warning for the `key configuration file` here. i.e If  `spark-deault.conf` is missing or changed to a user-specific dir under conf_dir, we may raise an `warning log` to let user aware of this before submitting.
    
    Currently, I think both of the two solutions is ok! : ) 


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68239625
  
      [Test build #24857 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24857/consoleFull) for   PR 3823 at commit [`c5a85eb`](https://github.com/apache/spark/commit/c5a85eb37389f3c849129267fcef0dfa608d09c6).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69287254
  
      [Test build #25286 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25286/consoleFull) for   PR 3823 at commit [`4cc7f34`](https://github.com/apache/spark/commit/4cc7f3467ed78bb4b3a1a404c0b1daf1bd009c35).
     * 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69425347
  
      [Test build #25347 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25347/consoleFull) for   PR 3823 at commit [`b1ab402`](https://github.com/apache/spark/commit/b1ab402a0a835a642c99064fc0fa3d4a320b8b94).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68244188
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24857/
    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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68322557
  
    @OopsOutOfMemory  Ok I got what you mean. After checking the logic in `SparkSubmitArguments.scala` I do think that your solution is more reasonable. Thanks.
    
    >      val sparkHomeConfig = env.get("SPARK_HOME").map(sparkHome => s"${sparkHome}${sep}conf")
          val confDir = env.get("SPARK_CONF_DIR").orElse(sparkHomeConfig)


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69282525
  
    @andrewor14 Ok, rebase done.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68327501
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24877/
    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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69425743
  
      [Test build #25348 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25348/consoleFull) for   PR 3823 at commit [`133c43e`](https://github.com/apache/spark/commit/133c43e79482d2f88392dc287aa185564c2ed557).
     * This patch merges cleanly.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#discussion_r22306764
  
    --- Diff: bin/spark-submit ---
    @@ -42,7 +42,10 @@ while (($#)); do
       shift
     done
     
    -DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +DEFAULT_PROPERTIES_FILE="$SPARK_CONF_DIR/spark-defaults.conf"
    +if [ ! -f "DEFAULT_PROPERTIES_FILE" ]; then
    +  DEFAULT_PROPERTIES_FILE="$SPARK_HOME/conf/spark-defaults.conf"
    +fi
     export SPARK_SUBMIT_DEPLOY_MODE=${SPARK_SUBMIT_DEPLOY_MODE:-"client"}
     export SPARK_SUBMIT_PROPERTIES_FILE=${SPARK_SUBMIT_PROPERTIES_FILE:-"$DEFAULT_PROPERTIES_FILE"}
    --- End diff --
    
    It's used here actually, but the purpose below seems to be detecting whether the user has set particular properties. Finding the default config doesn't matter here since it is a case where the user hasn't set these properties.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69425610
  
    @andrewor14  Ok, than makes sense. I changed `not exist` to `not defined` in windows cmd too.


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-69431702
  
      [Test build #25348 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25348/consoleFull) for   PR 3823 at commit [`133c43e`](https://github.com/apache/spark/commit/133c43e79482d2f88392dc287aa185564c2ed557).
     * 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 pull request: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68256422
  
    Hi, @WangTaoTheTonic : )
    This make sense to me, like hadoop also have HADOOP_CONF_DIR. 
    
    But I prefer to `check` if the `SPARK_CONF_DIR` `directory` is exists first, not only check the `configuration file`.
    If there are many files under SPARK_CONF_DIR need to be added in spark-submit in the future, you will need to check each file is exists or not. 
    
    you can do it like :
    ```shell
    if [ ! -d "$SPARK_CONF_DIR" ]; then
    ```


---
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-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68325792
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24875/
    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: [SPARK-4990][Deploy]to find default properties...

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

    https://github.com/apache/spark/pull/3823#issuecomment-68335758
  
      [Test build #24883 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24883/consoleFull) for   PR 3823 at commit [`55300bc`](https://github.com/apache/spark/commit/55300bcd566b53d0b9a4a579aa3074c7c3ac66b9).
     * 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