You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sakky11 <gi...@git.apache.org> on 2016/07/29 12:00:11 UTC
[GitHub] spark pull request #14402: Update JavaSparkContextVarargsWorkaround.java
GitHub user sakky11 opened a pull request:
https://github.com/apache/spark/pull/14402
Update JavaSparkContextVarargsWorkaround.java
Taking common functionality into a Single method to avoid duplication of code.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sakky11/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/14402.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 #14402
----
commit 2ec652c8b6c207c6a9350b4b7700d87db7773d7f
Author: Sakalya Deshpande <de...@gmail.com>
Date: 2016-07-29T11:57:25Z
Update JavaSparkContextVarargsWorkaround.java
Taking a common functionality in one method to avoid duplication of code.
commit 21a8e5a07d96102e7901bc8f76714606334a6e7b
Author: Sakalya Deshpande <de...@gmail.com>
Date: 2016-07-29T11:58:01Z
Merge pull request #1 from sakky11/sakky11-patch-1
Update JavaSparkContextVarargsWorkaround.java
----
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/14402
Merged build finished. Test FAILed.
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/14402
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/14402
@sakky11 you have not even addressed the compilation error. This is wasting everyone's time. Please _compile_ this locally and fix it before pinging 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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/14402
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/14402
Jenkins test this please
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/14402
Did you compile this locally before pushing?
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on the issue:
https://github.com/apache/spark/pull/14402
Any update on the recent change ??
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on the issue:
https://github.com/apache/spark/pull/14402
Hey sorry @srowen ,Actually I have fixed the compilation error but missed it in adding in a pull request.Sorry.
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/14402
@sakky11 I asked you because I can already see that your change doesn't even compile, for a third time. This is a trivial change, and not worth spending more time discussing. Please close this, and read https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark before opening another change. At minimum, you need to make sure your change even builds before considering opening a pull request.
---
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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on a diff in the pull request:
https://github.com/apache/spark/pull/14402#discussion_r72886590
--- Diff: core/src/main/java/org/apache/spark/api/java/JavaSparkContextVarargsWorkaround.java ---
@@ -52,11 +46,16 @@ public JavaDoubleRDD union(JavaDoubleRDD... rdds) {
if (rdds.length == 0) {
throw new IllegalArgumentException("Union called on empty list");
}
- List<JavaPairRDD<K, V>> rest = new ArrayList<>(rdds.length - 1);
+ rest = populateRDDList(rdds);
+ return union(rdds[0], rest);
+ }
+
+ public static final List<T> populateRDDList(T rdds) {
--- End diff --
will this work ??
---
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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on a diff in the pull request:
https://github.com/apache/spark/pull/14402#discussion_r72886550
--- Diff: core/src/main/java/org/apache/spark/api/java/JavaSparkContextVarargsWorkaround.java ---
@@ -52,11 +46,16 @@ public JavaDoubleRDD union(JavaDoubleRDD... rdds) {
if (rdds.length == 0) {
throw new IllegalArgumentException("Union called on empty list");
}
- List<JavaPairRDD<K, V>> rest = new ArrayList<>(rdds.length - 1);
+ rest = populateRDDList(rdds);
+ return union(rdds[0], rest);
+ }
+
+ public static final List<T> populateRDDList(T rdds) {
--- End diff --
I created a build with T...
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on the issue:
https://github.com/apache/spark/pull/14402
Added the required fix.Can you please start the build ??
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/14402
Yeah, still doesn't compile. Are you compiling this locally?
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on the issue:
https://github.com/apache/spark/pull/14402
I think the issue should be fixed now.Sorry for trouble.
---
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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on a diff in the pull request:
https://github.com/apache/spark/pull/14402#discussion_r72886476
--- Diff: core/src/main/java/org/apache/spark/api/java/JavaSparkContextVarargsWorkaround.java ---
@@ -52,11 +46,16 @@ public JavaDoubleRDD union(JavaDoubleRDD... rdds) {
if (rdds.length == 0) {
throw new IllegalArgumentException("Union called on empty list");
}
- List<JavaPairRDD<K, V>> rest = new ArrayList<>(rdds.length - 1);
+ rest = populateRDDList(rdds);
+ return union(rdds[0], rest);
+ }
+
+ public static final List<T> populateRDDList(T rdds) {
--- End diff --
I agree with making the method "private".But rdds is not any varargs so why do we need ...??
---
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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14402#discussion_r72886519
--- Diff: core/src/main/java/org/apache/spark/api/java/JavaSparkContextVarargsWorkaround.java ---
@@ -52,11 +46,16 @@ public JavaDoubleRDD union(JavaDoubleRDD... rdds) {
if (rdds.length == 0) {
throw new IllegalArgumentException("Union called on empty list");
}
- List<JavaPairRDD<K, V>> rest = new ArrayList<>(rdds.length - 1);
+ rest = populateRDDList(rdds);
+ return union(rdds[0], rest);
+ }
+
+ public static final List<T> populateRDDList(T rdds) {
--- End diff --
This doesn't build. You're using `T` like an array type but it isn't. It has to be an array type as far as the method is concerned. `T[]` would be fine as the type.
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14402
**[Test build #63042 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63042/consoleFull)** for PR 14402 at commit [`3e831e5`](https://github.com/apache/spark/commit/3e831e5538ba7960119a1db426072d0f22c85331).
* This patch **fails to build**.
* 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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the issue:
https://github.com/apache/spark/pull/14402
This patch is only a one-line reduction of code and touches a file which hasn't needed to be modified in ages; is this change really necessary / worth spending review time on?
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on the issue:
https://github.com/apache/spark/pull/14402
Any update on this 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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14402
**[Test build #63042 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63042/consoleFull)** for PR 14402 at commit [`3e831e5`](https://github.com/apache/spark/commit/3e831e5538ba7960119a1db426072d0f22c85331).
---
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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14402#discussion_r72859935
--- Diff: core/src/main/java/org/apache/spark/api/java/JavaSparkContextVarargsWorkaround.java ---
@@ -52,11 +46,16 @@ public JavaDoubleRDD union(JavaDoubleRDD... rdds) {
if (rdds.length == 0) {
throw new IllegalArgumentException("Union called on empty list");
}
- List<JavaPairRDD<K, V>> rest = new ArrayList<>(rdds.length - 1);
+ rest = populateRDDList(rdds);
+ return union(rdds[0], rest);
+ }
+
+ public static final List<T> populateRDDList(T rdds) {
--- End diff --
One last thing -- `private`? And doesn't this have to be `T... rdds` to work?
---
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 #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14402#discussion_r72784184
--- Diff: core/src/main/java/org/apache/spark/api/java/JavaSparkContextVarargsWorkaround.java ---
@@ -58,6 +52,14 @@ public JavaDoubleRDD union(JavaDoubleRDD... rdds) {
}
return union(rdds[0], rest);
}
+
+ public final List<T> populateRDDList(T rdds) {
--- End diff --
OK, pretty trivial but a win. You can use this in line 49 too right?
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by sakky11 <gi...@git.apache.org>.
Github user sakky11 commented on the issue:
https://github.com/apache/spark/pull/14402
I know its trivial but it still avoids duplication of code and make it more readable.
---
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 issue #14402: Update JavaSparkContextVarargsWorkaround.java
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/14402
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63042/
Test FAILed.
---
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