You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by azagrebin <gi...@git.apache.org> on 2015/02/15 16:14:56 UTC

[GitHub] spark pull request: [SPARK-3340] Deprecate ADD_JARS and ADD_FILES

GitHub user azagrebin opened a pull request:

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

    [SPARK-3340] Deprecate ADD_JARS and ADD_FILES

    I created a patch that disables the environment variables.
    Thereby scala or python shell log a warning message to notify user about the deprecation 
    with the following message:
    scala: "ADD_JARS environment variable is deprecated, use --jar spark submit argument instead"
    python: "Warning: ADD_FILES environment variable is deprecated, use --py-files argument instead"
    
    Is it what is expected or the code associated with the variables should be just completely removed?
    Should it be somewhere documented?

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

    $ git pull https://github.com/azagrebin/spark master

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

    https://github.com/apache/spark/pull/4616.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 #4616
    
----
commit 42f010767b9f17fc41ca47eece84de5359871ae8
Author: azagrebin <az...@gmail.com>
Date:   2015-02-15T13:57:10Z

    [SPARK-3340] Deprecate ADD_JARS and ADD_FILES

----


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74421518
  
      [Test build #27523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27523/consoleFull) for   PR 4616 at commit [`42f0107`](https://github.com/apache/spark/commit/42f010767b9f17fc41ca47eece84de5359871ae8).
     * 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74557764
  
    Hi @andrewor14, thanks for the comment. So the variables need to be supported until some next release and a warning should be logged that they are deprecated (may be not supported in some next release), right? I have done another commit.


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74606484
  
    LGTM I'm merging this into master and 1.3 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74557556
  
      [Test build #27563 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27563/consoleFull) for   PR 4616 at commit [`0643895`](https://github.com/apache/spark/commit/0643895b38d6bbcde312dcc81abdacb0b86a4d27).
     * 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74573715
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27567/
    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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74421286
  
    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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74606768
  
    Hey @azagrebin what is your JIRA name? I would like to give you credit for 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#discussion_r24771625
  
    --- Diff: repl/scala-2.11/src/main/scala/org/apache/spark/repl/Main.scala ---
    @@ -51,6 +51,11 @@ object Main extends Logging {
     
       def getAddedJars: Array[String] = {
         val envJars = sys.env.get("ADD_JARS")
    +    envJars match {
    +      case Some(_) =>
    +        logWarning("ADD_JARS environment variable is deprecated, use --jar spark submit argument instead")
    +      case _ =>
    +    }
    --- End diff --
    
    same here


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74563971
  
      [Test build #27567 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27567/consoleFull) for   PR 4616 at commit [`bab1aa9`](https://github.com/apache/spark/commit/bab1aa935b5255ef6fb0eefbb86d62b3e3f6887e).
     * 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74424545
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27523/
    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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#discussion_r24771610
  
    --- Diff: repl/scala-2.10/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
    @@ -1064,15 +1064,18 @@ class SparkILoop(
       private def main(settings: Settings): Unit = process(settings)
     }
     
    -object SparkILoop {
    +object SparkILoop extends Logging {
       implicit def loopToInterpreter(repl: SparkILoop): SparkIMain = repl.intp
       private def echo(msg: String) = Console println msg
     
       def getAddedJars: Array[String] = {
         val envJars = sys.env.get("ADD_JARS")
    -    val propJars = sys.props.get("spark.jars").flatMap { p =>
    -      if (p == "") None else Some(p)
    +    envJars match {
    +      case Some(_) =>
    +        logWarning("ADD_JARS environment variable is deprecated, use --jar spark submit argument instead")
    +      case _ =>
         }
    --- End diff --
    
    minor point but I think it's more readable here if you just do
    ```
    if (envJars.isDefined) {
      logWarning(...)
    }
    ```


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

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


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74564112
  
    yeah, really better, thanks for the hint, committed again


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74421348
  
    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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#discussion_r24788093
  
    --- Diff: repl/scala-2.10/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
    @@ -1064,15 +1064,15 @@ class SparkILoop(
       private def main(settings: Settings): Unit = process(settings)
     }
     
    -object SparkILoop {
    +object SparkILoop extends Logging {
       implicit def loopToInterpreter(repl: SparkILoop): SparkIMain = repl.intp
       private def echo(msg: String) = Console println msg
     
       def getAddedJars: Array[String] = {
         val envJars = sys.env.get("ADD_JARS")
    -    val propJars = sys.props.get("spark.jars").flatMap { p =>
    -      if (p == "") None else Some(p)
    -    }
    +    if (envJars.isDefined)
    +      logWarning("ADD_JARS environment variable is deprecated, use --jar spark submit argument instead")
    --- End diff --
    
    our style guide actually requires us to put `{ } around the content of an `if`. I'll do this myself when I merge don't worry.


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74567551
  
      [Test build #27563 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27563/consoleFull) for   PR 4616 at commit [`0643895`](https://github.com/apache/spark/commit/0643895b38d6bbcde312dcc81abdacb0b86a4d27).
     * 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74549382
  
    @azagrebin thanks for working on this. However, deprecated just means it will be released in a future released, but not in the current release. If we log a warning saying that they are deprecated we still need to support them, at least for another release (until 1.4 or something).


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74567563
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27563/
    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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74573712
  
      [Test build #27567 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27567/consoleFull) for   PR 4616 at commit [`bab1aa9`](https://github.com/apache/spark/commit/bab1aa935b5255ef6fb0eefbb86d62b3e3f6887e).
     * 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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74631484
  
    @andrewor14, thanks for the brackets and the credit, I have actually read the style guide but forgot them, sorry for that. I have also created the JIRA account, my username is the same: azagrebin.


---
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-3340] Deprecate ADD_JARS and ADD_FILES

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

    https://github.com/apache/spark/pull/4616#issuecomment-74424541
  
      [Test build #27523 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27523/consoleFull) for   PR 4616 at commit [`42f0107`](https://github.com/apache/spark/commit/42f010767b9f17fc41ca47eece84de5359871ae8).
     * 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