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

[GitHub] spark pull request #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstra...

GitHub user srowen opened a pull request:

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

    [SPARK-25029][BUILD][CORE] Janino "Two non-abstract methods ..." errors

    ## What changes were proposed in this pull request?
    
    Update to janino 3.0.9 to address Java 8 + Scala 2.12 incompatibility
    
    ## How was this patch tested?
    
    Existing tests

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

    $ git pull https://github.com/srowen/spark SPARK-25029

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

    https://github.com/apache/spark/pull/22203.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 #22203
    
----
commit b7c7bdbbac7c04ef9d60c0032f13660890eee227
Author: Sean Owen <se...@...>
Date:   2018-08-23T13:51:10Z

    Update to janino 3.0.9 to address Java 8 + Scala 2.12 incompatibility

----


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    Good point @rednaxelafx though I know some existing tests already definitely failed without this change. They're in the JIRA. It's pervasive enough that we'd see a number of failures due to this issue -- or at least I did when running this in Scala 2.12.


---

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


[GitHub] spark pull request #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstra...

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

    https://github.com/apache/spark/pull/22203#discussion_r212419913
  
    --- Diff: dev/deps/spark-deps-hadoop-2.6 ---
    @@ -98,7 +98,7 @@ jackson-module-jaxb-annotations-2.6.7.jar
     jackson-module-paranamer-2.7.9.jar
     jackson-module-scala_2.11-2.6.7.1.jar
     jackson-xc-1.9.13.jar
    -janino-3.0.8.jar
    +janino-3.0.9.jar
    --- End diff --
    
    It includes your fix https://github.com/janino-compiler/janino/pull/46


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    Thanks @srowen ! In that case could we add a few mentions of such test cases that failed into the PR description itself, perhaps a few lines under the "How was this patch tested?" section to give a few examples of which tests used to fail and are now fixed?


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    I'm wondering, though, either in the same PR or in a separate PR, could we add a test case in Spark that would trigger the "two non-abstract method" error so that we'd show an example scenario in Spark that'd trigger the issue and also show that Janino 3.0.9 has fixed it?


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    Thanks! Merged to master


---

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


[GitHub] spark pull request #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstra...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    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 #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    Done, I added one example test failure to the description here.


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    LGTM


---

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


[GitHub] spark pull request #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstra...

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

    https://github.com/apache/spark/pull/22203#discussion_r212535154
  
    --- Diff: dev/deps/spark-deps-hadoop-2.6 ---
    @@ -98,7 +98,7 @@ jackson-module-jaxb-annotations-2.6.7.jar
     jackson-module-paranamer-2.7.9.jar
     jackson-module-scala_2.11-2.6.7.1.jar
     jackson-xc-1.9.13.jar
    -janino-3.0.8.jar
    +janino-3.0.9.jar
    --- End diff --
    
    Sorry for being late. I confirmed all of changes in the changelog. Two issues https://github.com/janino-compiler/janino/pull/54 and https://github.com/janino-compiler/janino/pull/46 are for Java 8 and later support. Others are bug fixes.
    
    LGTM.



---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    **[Test build #95159 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95159/testReport)** for PR 22203 at commit [`848bf1c`](https://github.com/apache/spark/commit/848bf1c44233a19c1fce87486c154073e4b70baa).


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    **[Test build #95159 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95159/testReport)** for PR 22203 at commit [`848bf1c`](https://github.com/apache/spark/commit/848bf1c44233a19c1fce87486c154073e4b70baa).
     * 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 #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstra...

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

    https://github.com/apache/spark/pull/22203#discussion_r212418839
  
    --- Diff: dev/deps/spark-deps-hadoop-2.6 ---
    @@ -98,7 +98,7 @@ jackson-module-jaxb-annotations-2.6.7.jar
     jackson-module-paranamer-2.7.9.jar
     jackson-module-scala_2.11-2.6.7.1.jar
     jackson-xc-1.9.13.jar
    -janino-3.0.8.jar
    +janino-3.0.9.jar
    --- End diff --
    
    https://janino-compiler.github.io/janino/changelog.html
    
    cc @kiszk @rednaxelafx Please check the changes made in 3.0.9 


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

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


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    **[Test build #95158 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95158/testReport)** for PR 22203 at commit [`b7c7bdb`](https://github.com/apache/spark/commit/b7c7bdbbac7c04ef9d60c0032f13660890eee227).
     * This patch **fails build dependency tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstract meth...

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

    https://github.com/apache/spark/pull/22203
  
    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 #22203: [SPARK-25029][BUILD][CORE] Janino "Two non-abstra...

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

    https://github.com/apache/spark/pull/22203#discussion_r212423334
  
    --- Diff: dev/deps/spark-deps-hadoop-2.6 ---
    @@ -98,7 +98,7 @@ jackson-module-jaxb-annotations-2.6.7.jar
     jackson-module-paranamer-2.7.9.jar
     jackson-module-scala_2.11-2.6.7.1.jar
     jackson-xc-1.9.13.jar
    -janino-3.0.8.jar
    +janino-3.0.9.jar
    --- End diff --
    
    Yep my primary reason is pulling in this fix for Scala 2.12: https://github.com/janino-compiler/janino/pull/54 The other changes I can see appear to be small bug fixes, some for Java 8 / 9 support, which is probably useful for us.


---

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