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 2017/10/23 17:39:19 UTC
[GitHub] spark pull request #19561: [SPARK-22322][CORE] Update FutureAction for compa...
GitHub user srowen opened a pull request:
https://github.com/apache/spark/pull/19561
[SPARK-22322][CORE] Update FutureAction for compatibility with Scala 2.12 Future
## What changes were proposed in this pull request?
Scala 2.12's `Future` defines two new methods to implement, `transform` and `transformWith`. These can be implemented naturally in Spark's `FutureAction` extension and subclasses, but, only in terms of the new methods that don't exist in Scala 2.11. To support both at the same time, reflection is used to implement these.
## 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-22322
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19561.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 #19561
----
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19561: [SPARK-22322][CORE] Update FutureAction for compatibilit...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19561
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 #19561: [SPARK-22322][CORE] Update FutureAction for compa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/19561#discussion_r146342877
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingQuerySuite.scala ---
@@ -744,7 +744,7 @@ class StreamingQuerySuite extends StreamTest with BeforeAndAfter with Logging wi
assert(returnedValue === expectedReturnValue, "Returned value does not match expected")
}
}
- AwaitTerminationTester.test(expectedBehavior, awaitTermFunc)
+ AwaitTerminationTester.test(expectedBehavior, () => awaitTermFunc())
--- End diff --
The rest here are just 2.12 warning cleanups by the by
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19561: [SPARK-22322][CORE] Update FutureAction for compatibilit...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19561
**[Test build #82991 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82991/testReport)** for PR 19561 at commit [`faa6754`](https://github.com/apache/spark/commit/faa6754467152a8aa10fa14f8b8c7d775d75f538).
* 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 issue #19561: [SPARK-22322][CORE] Update FutureAction for compatibilit...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19561
**[Test build #82991 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82991/testReport)** for PR 19561 at commit [`faa6754`](https://github.com/apache/spark/commit/faa6754467152a8aa10fa14f8b8c7d775d75f538).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19561: [SPARK-22322][CORE] Update FutureAction for compa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/19561#discussion_r146342937
--- Diff: core/src/main/scala/org/apache/spark/FutureAction.scala ---
@@ -89,7 +89,11 @@ trait FutureAction[T] extends Future[T] {
*/
override def value: Option[Try[T]]
- // These two methods must be implemented in Scala 2.12, but won't be used by Spark
+ // These two methods must be implemented in Scala 2.12. They're implemented as a no-op here
+ // and then filled in with a real implementation in the two subclasses below. The no-op exists
+ // here so that those implementations can declare "override", necessary in 2.12, while working
+ // in 2.11, where the method doesn't exist in the superclass.
+ // After 2.11 support goes away, remove these two:
--- End diff --
In case it wasn't clear, this change ought to have no effect at all in 2.11
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19561: [SPARK-22322][CORE] Update FutureAction for compa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen closed the pull request at:
https://github.com/apache/spark/pull/19561
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19561: [SPARK-22322][CORE] Update FutureAction for compatibilit...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19561
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82991/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19561: [SPARK-22322][CORE] Update FutureAction for compatibilit...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19561
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 #19561: [SPARK-22322][CORE] Update FutureAction for compa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/19561#discussion_r146342617
--- Diff: core/src/main/scala/org/apache/spark/FutureAction.scala ---
@@ -113,6 +117,42 @@ trait FutureAction[T] extends Future[T] {
}
+/**
+ * Scala 2.12 defines the two new transform/transformWith methods mentioned above. Impementing
+ * these for 2.12 in the Spark class here requires delegating to these same methods in an
+ * underlying Future object. But that only exists in 2.12. But these methods are only called
+ * in 2.12. So define helper shims to access these methods on a Future by reflection.
+ */
+private[spark] object FutureAction {
+
+ private val transformTryMethod =
--- End diff --
Having to access this by reflection is a little annoying but keeping 2.11 + 2.12 compatibility in one source file is nice, and doesn't entail much of any runtime overhead
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19561: [SPARK-22322][CORE] Update FutureAction for compa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/19561#discussion_r146342815
--- Diff: pom.xml ---
@@ -2692,7 +2692,7 @@
<profile>
<id>scala-2.12</id>
<properties>
- <scala.version>2.12.3</scala.version>
+ <scala.version>2.12.4</scala.version>
--- End diff --
This update picks up a fix for another scalac bug that also prevents the 2.12 build from working, but, has no effect on 2.11
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org