You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dongjoon-hyun <gi...@git.apache.org> on 2018/09/04 21:03:20 UTC

[GitHub] spark pull request #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it'...

GitHub user dongjoon-hyun opened a pull request:

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

    [SPARK-25335][BUILD] Skip Zinc downloading if it's installed in the system

    ## What changes were proposed in this pull request?
    
    Zinc is 23.5MB.
    ```
    $ curl -LO https://downloads.lightbend.com/zinc/0.3.15/zinc-0.3.15.tgz
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 23.5M  100 23.5M    0     0  35.4M      0 --:--:-- --:--:-- --:--:-- 35.3M
    ```
    
    Currently, Spark downloads Zinc once. However, this PR aims to skip Zinc downloading when the system already has it.
    ```
    $ build/mvn clean
    exec: curl --progress-bar -L https://downloads.lightbend.com/zinc/0.3.15/zinc-0.3.15.tgz
    ######################################################################## 100.0%
    ########################################################################
    ```
    
    This will reduce many resources(CPU/Networks/DISK) at least in Mac and Docker-based build system.
    
    ## How was this patch tested?
    
    Pass the Jenkins.

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

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

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

    https://github.com/apache/spark/pull/22333.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 #22333
    
----
commit ca9963477cb7cc1b9ff012222bc8adcd4a482a88
Author: Dongjoon Hyun <do...@...>
Date:   2018-09-04T20:59:31Z

    [SPARK-25335][BUILD] Skip Zinc downloading if it's installed in the system

----


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    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 #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    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/2882/
    Test PASSed.


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    **[Test build #95728 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95728/testReport)** for PR 22333 at commit [`14d1017`](https://github.com/apache/spark/commit/14d10172f9d62ba9a8eabec5aaa11759757278bf).
     * 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 #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it'...

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

    https://github.com/apache/spark/pull/22333#discussion_r215265080
  
    --- Diff: build/mvn ---
    @@ -91,15 +92,23 @@ install_mvn() {
     
     # Install zinc under the build/ folder
     install_zinc() {
    -  local zinc_path="zinc-0.3.15/bin/zinc"
    -  [ ! -f "${_DIR}/${zinc_path}" ] && ZINC_INSTALL_FLAG=1
    -  local TYPESAFE_MIRROR=${TYPESAFE_MIRROR:-https://downloads.lightbend.com}
    +  ZINC_VERSION=0.3.15
    --- End diff --
    
    Last tiny nit, should this be `local` to be completely symmetric with the Maven-handling function above?


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    **[Test build #95683 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95683/testReport)** for PR 22333 at commit [`ca99634`](https://github.com/apache/spark/commit/ca9963477cb7cc1b9ff012222bc8adcd4a482a88).
     * 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 #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it'...

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

    https://github.com/apache/spark/pull/22333#discussion_r215124159
  
    --- Diff: build/mvn ---
    @@ -91,15 +92,23 @@ install_mvn() {
     
     # Install zinc under the build/ folder
     install_zinc() {
    -  local zinc_path="zinc-0.3.15/bin/zinc"
    -  [ ! -f "${_DIR}/${zinc_path}" ] && ZINC_INSTALL_FLAG=1
    -  local TYPESAFE_MIRROR=${TYPESAFE_MIRROR:-https://downloads.lightbend.com}
    +  ZINC_VERSION=0.3.15
    +  ZINC_BIN="$(command -v zinc)"
    +  if [ "$ZINC_BIN" ]; then
    +    local ZINC_DETECTED_VERSION="$(zinc -version | head -n1 | awk '{print $5}')"
    +  fi
     
    -  install_app \
    -    "${TYPESAFE_MIRROR}/zinc/0.3.15" \
    -    "zinc-0.3.15.tgz" \
    -    "${zinc_path}"
    -  ZINC_BIN="${_DIR}/${zinc_path}"
    +  if [ $(version $ZINC_DETECTED_VERSION) -lt $(version $ZINC_VERSION) ]; then
    --- End diff --
    
    Thank you for review, @srowen . Yes. We use the same logic for Maven.
    If no zinc was detected, it is evaluated as `000000000 -lt 000003015`, and eventually `false`.


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    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 #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

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


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

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


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    **[Test build #95728 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95728/testReport)** for PR 22333 at commit [`14d1017`](https://github.com/apache/spark/commit/14d10172f9d62ba9a8eabec5aaa11759757278bf).


---

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


[GitHub] spark pull request #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it'...

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

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


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    Oh, I assumed that it's already dockerized. Sorry, never mind about that @shaneknapp . And, thanks!


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    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/2845/
    Test PASSed.


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    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 #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

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


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    Hi, @shaneknapp and @srowen .
    Can we build and use the zinc-installed docker images in our build system?
    - https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-2.7/5276/consoleFull


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    Merged to master


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    moving any parts of the spark build infrastructure to use docker is a big project and not happening in the next few months.  


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

    https://github.com/apache/spark/pull/22333
  
    Thank you, @srowen .


---

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


[GitHub] spark issue #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it's insta...

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

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


---

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


[GitHub] spark pull request #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it'...

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

    https://github.com/apache/spark/pull/22333#discussion_r215115678
  
    --- Diff: build/mvn ---
    @@ -91,15 +92,23 @@ install_mvn() {
     
     # Install zinc under the build/ folder
     install_zinc() {
    -  local zinc_path="zinc-0.3.15/bin/zinc"
    -  [ ! -f "${_DIR}/${zinc_path}" ] && ZINC_INSTALL_FLAG=1
    -  local TYPESAFE_MIRROR=${TYPESAFE_MIRROR:-https://downloads.lightbend.com}
    +  ZINC_VERSION=0.3.15
    +  ZINC_BIN="$(command -v zinc)"
    +  if [ "$ZINC_BIN" ]; then
    +    local ZINC_DETECTED_VERSION="$(zinc -version | head -n1 | awk '{print $5}')"
    +  fi
     
    -  install_app \
    -    "${TYPESAFE_MIRROR}/zinc/0.3.15" \
    -    "zinc-0.3.15.tgz" \
    -    "${zinc_path}"
    -  ZINC_BIN="${_DIR}/${zinc_path}"
    +  if [ $(version $ZINC_DETECTED_VERSION) -lt $(version $ZINC_VERSION) ]; then
    --- End diff --
    
    I assume this evaluates to false if no zinc was detected; that seems to be how the maven logic above works. OK. I also would like to use my local zinc if available.


---

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


[GitHub] spark pull request #22333: [SPARK-25335][BUILD] Skip Zinc downloading if it'...

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

    https://github.com/apache/spark/pull/22333#discussion_r215365678
  
    --- Diff: build/mvn ---
    @@ -91,15 +92,23 @@ install_mvn() {
     
     # Install zinc under the build/ folder
     install_zinc() {
    -  local zinc_path="zinc-0.3.15/bin/zinc"
    -  [ ! -f "${_DIR}/${zinc_path}" ] && ZINC_INSTALL_FLAG=1
    -  local TYPESAFE_MIRROR=${TYPESAFE_MIRROR:-https://downloads.lightbend.com}
    +  ZINC_VERSION=0.3.15
    --- End diff --
    
    Oh, thanks! Sure.


---

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