You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dbtsai <gi...@git.apache.org> on 2018/08/20 20:38:47 UTC
[GitHub] spark pull request #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.1...
GitHub user dbtsai opened a pull request:
https://github.com/apache/spark/pull/22160
Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 and 2.12.6"
This reverts commit c7967c6049327a03b63ea7a3b0001a97d31e309d.
We would like to move the Scala upgrade in Spark 3.0 together with Scala 2.12 effort; hence, we revet c7967c6049327a03b63ea7a3b0001a97d31e309d
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dbtsai/spark revert
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22160.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 #22160
----
commit dfa3cb5981dd4f2a5e9582b7fca4e68c0ed31fa2
Author: DB Tsai <d_...@...>
Date: 2018-08-20T20:32:10Z
Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 and 2.12.6"
This reverts commit c7967c6049327a03b63ea7a3b0001a97d31e309d.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94977/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22160
Closed this PR since we're in favor of https://github.com/apache/spark/pull/21749
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94978/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22160
**[Test build #94977 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94977/testReport)** for PR 22160 at commit [`dfa3cb5`](https://github.com/apache/spark/commit/dfa3cb5981dd4f2a5e9582b7fca4e68c0ed31fa2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
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 #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.1...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:
https://github.com/apache/spark/pull/22160#discussion_r211689639
--- Diff: pom.xml ---
@@ -2756,7 +2756,7 @@
<profile>
<id>scala-2.12</id>
<properties>
- <scala.version>2.12.6</scala.version>
+ <scala.version>2.12.4</scala.version>
--- End diff --
Since 2.12.6 removes the hacks we were using to initialize Spark context, if we want to revert SPARK-24418, we have to use older version of 2.12.x
My 2 cents, reverting SPARK-24418 will make 2.12 work harder since we have to deal with Scala Shell issue as part of the tasks.
Alternatively, instead of reverting SPARK-24418, we should consider to merge https://github.com/apache/spark/pull/21749 which fixes the message printing issue.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.1...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22160#discussion_r211670302
--- Diff: pom.xml ---
@@ -2756,7 +2756,7 @@
<profile>
<id>scala-2.12</id>
<properties>
- <scala.version>2.12.6</scala.version>
+ <scala.version>2.12.4</scala.version>
--- End diff --
Oh I get your point @dbtsai -- do you need to revert anything related to 2.12? I think we do need 2.12.6 for fixes for 2.12 to work, but, then again this support isn't released yet. So I figure there's nothing to 'fix' by reverting this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22160
**[Test build #94977 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94977/testReport)** for PR 22160 at commit [`dfa3cb5`](https://github.com/apache/spark/commit/dfa3cb5981dd4f2a5e9582b7fca4e68c0ed31fa2).
* This patch **fails Spark unit tests**.
* This patch **does not merge cleanly**.
* This patch adds the following public classes _(experimental)_:
* `class SparkILoopInterpreter(settings: Settings, out: JPrintWriter) extends IMain(settings, out) `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22160
@dbtsai Have you tried to run it in scala 2.12? We still can do the upgrade after Apache 2.4 RC.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22160
@srowen Just found Scala 2.12 has a separated REPL implementation which suffered from message order printing issue too.
Let me change this PR so the Scala 2.12 version is still on 2.12.6
BTW, https://github.com/apache/spark/pull/21749 should work for both 2.12 and 2.11, so we can consolidate the code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22160
Oh, what went wrong @dbtsai ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22160
I see, doesn't look like we lose a critical fix or improvement with the down-grade, and this wasn't released in 2.3.x. OK. I prefer to fix-forward, too, but am neutral on whether the cost of the workaround is worth it.
But does it affect 2.12? yeah that would be a bigger deal. But the original commit didn't seem to touch the 2.12 REPL.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.1...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai closed the pull request at:
https://github.com/apache/spark/pull/22160
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
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 #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22160
Yeah that's right, we have had two parallel trees for different Scala versions just for reasons like this. There is less forked code now than with 2.10 vs 2.11. If you think we can merge all the code into one tree with some reflection then I think that's really worth it rather than reverting this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
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/2338/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
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/2337/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22160
Because of the `printWelcome` message regression described in `https://issues.apache.org/jira/browse/SPARK-25138`, to have our users best experience, @gatorsmile and I agree that we either revert the Scala upgrade or merge https://github.com/apache/spark/pull/21749 which uses reflection to fix the printing ordering.
I'm leaning toward merging https://github.com/apache/spark/pull/21749 since I worry reverting the scala upgrade will break the work for Scala 2.12. On the other hand, fixing the printing ordering using reflection is not clean.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22160
**[Test build #94978 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94978/testReport)** for PR 22160 at commit [`1ad5d3d`](https://github.com/apache/spark/commit/1ad5d3d20eba0156569f2ae9e605520fcaca471d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class SparkILoopInterpreter(settings: Settings, out: JPrintWriter) extends IMain(settings, out) `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22160
By the way the 2.12 build will still fail, so that won't tell you much.
I think we need 2.12.6 to fix a Java 8 related issue? Don't have it handy though.
Sorry if I'm being dense here but didn't all those REPL changes just concern 2.11? I did not see any 2.12 change except the version
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22160
Thank you! @dbtsai Let us see whether this can pass all the tests?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22160
@srowen We were overwriting `def loadFiles(settings: Settings) ` in https://github.com/scala/scala/blob/v2.11.6/src/repl/scala/tools/nsc/interpreter/ILoop.scala#L853 to initialize the Spark context.
It's not available in newer version of Scala such as [2.12.6](https://github.com/scala/scala/blob/v2.12.6/src/repl/scala/tools/nsc/interpreter/ILoop.scala)
The REPL code are shared by 2.11 and 2.12, right? So reverting the REPL code will fail 2.12 build.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
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 #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/22160
Can you add to the pr description why we are reverting? Just copy paste what you had above. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22160
**[Test build #94978 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94978/testReport)** for PR 22160 at commit [`1ad5d3d`](https://github.com/apache/spark/commit/1ad5d3d20eba0156569f2ae9e605520fcaca471d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22160
@gatorsmile I have not find a time yesterday to run tests with Scala 2.12. I'll try to do it today.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22160
@gatorsmile we also need to run the tests for 2.12 manually.
@srowen This PR will revert Scala 2.12.6 back to 2.12.4 since 2.12.6 also removes the hacks we used to initialize Spark. I worry it will have a negative impact on the recent work of Scala 2.12 support.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22160: Revert "[SPARK-24418][BUILD] Upgrade Scala to 2.11.12 an...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22160
Build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org