You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by elmer-garduno <gi...@git.apache.org> on 2015/01/02 04:49:09 UTC
[GitHub] spark pull request: [SPARK-5052] Add common/base classes to fix gu...
GitHub user elmer-garduno opened a pull request:
https://github.com/apache/spark/pull/3874
[SPARK-5052] Add common/base classes to fix guava methods signatures.
Fixes problems with incorrect method signatures related to shaded classes. For discussion see the jira issue.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/elmer-garduno/spark fix_guava_signatures
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3874.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 #3874
----
commit 596ddbaa8c2f9bf6d2099e799b682d2b15ad9dc6
Author: Elmer Garduno <el...@google.com>
Date: 2015-01-02T03:45:35Z
Add common/base classes to fix guava methods signatures.
----
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71562776
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26121/
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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68777890
Let me try just adding `Function` without `Function*`
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68900567
Not from your side, you just need a committer to notice this PR. :-) @pwendell ?
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno closed the pull request at:
https://github.com/apache/spark/pull/3874
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3874
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71562768
[Test build #26121 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26121/consoleFull) for PR 3874 at commit [`aa5d8e0`](https://github.com/apache/spark/commit/aa5d8e0fd88d9c742b2daf20265da809fa919d2a).
* This patch **fails Spark unit tests**.
* 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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68789982
Although further creep of the unshading-of-the-shading feels risky, it seems to resolve a problem, and is in principle OK on the same grounds that unshading `Optional` is. I'm still puzzled about why using Guava 14 isn't working but wouldn't argue with solving a problem this way.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68616993
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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68765892
I tried adding Function before, but it had some runtime dependencies.
Please see the last part of the third comment on
https://issues.apache.org/jira/plugins/servlet/mobile#issue/SPARK-5052.
On Mon, Jan 5, 2015, 10:27 AM Marcelo Vanzin <no...@github.com>
wrote:
> Damn, exposing Optional in the API turned out to be a minefield... I see
> the problem and I don't see a good solution other than adding Function to
> the list of exclusions.
>
> That being said, I think only Function should be added. The source
> <https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/base/Function.java?name=master>
> indicates it has no references to other types, so hopefully it shouldn't
> cause any other issues...
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/3874#issuecomment-68751824>.
>
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71600458
Thanks for merging!
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68785782
Thanks, that worked, I updated the PR to reflect those changes. And here is a list of the actual classes that get included into the jar:
jar tf assembly/target/scala-2.10/spark-assembly-1.3.0-SNAPSHOT-hadoop1.2.1.jar |grep com.google.common.base
com/google/common/base/
com/google/common/base/Absent.class
com/google/common/base/Function.class
com/google/common/base/Optional$1$1.class
com/google/common/base/Optional$1.class
com/google/common/base/Optional.class
com/google/common/base/Present.class
com/google/common/base/Supplier.class
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68507620
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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71572558
[Test build #26126 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26126/consoleFull) for PR 3874 at commit [`aa5d8e0`](https://github.com/apache/spark/commit/aa5d8e0fd88d9c742b2daf20265da809fa919d2a).
* This patch **passes all tests**.
* 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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68773133
The only risk here is that this increases surface area for Guava version conflict, which was the whole point of the shading. Elmer has some analysis of what happens when you do this in the JIRA: https://issues.apache.org/jira/browse/SPARK-5052?focusedCommentId=14262740&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14262740
I'm still not sure why the heck the `Partition` class isn't found if you just bundle your own Guava, which is how it's "supposed" to be done (right?). That seems like the path of least resistance. I haven't had a chance to look at that more.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71555711
LGTM although it's now been rolled into #3658 (@vanzin - maybe we can merge this one first just so we give credit correctly to @elmer-garduno).
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68772197
What runtime dependencies does `Function` have? I linked to its source above. It doesn't reference any other Guava types at all.
Also, because of the `or()` method, `Supplier` should also be added to the exclusions. As with `Function`, it doesn't reference any other Guava type, so those two extra exclusions should be enough. If they fail your test for some reason, please post more information about what exactly is being referenced and causing problems.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71572569
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26126/
Test PASSed.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71555577
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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71411925
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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71556298
Yeah, no problem. I just didn't want to cause a regression later when my changes go in.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68617200
@vanzin, any input here? Running with the patched code seems to solve the problem. Running on 1.2.0 standalone cluster generates the following errors:
a) `java.lang.NoSuchMethodError: com.google.common.base.Optional.transform(Lcom/google/common/base/Function;)Lcom/google/common/base/Optional;` as explained on the issue.
b) `java.lang.NoClassDefFoundError: org/apache/spark/Partition;` when running with `spark.files.userClassPathFirst=true`.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68774936
In fact what happens is the following:
If you embed guava on your jar you still get `java.lang.NoSuchMethodError: com.google.common.base.Optional.transform(Lcom/google/common/base/Function;)Lcom/google/common/base/Optional;` as it seems to be loading the Optional from Spark first.
It is when you specify `spark.files.userClassPathFirst=true` that you get `java.lang.NoClassDefFoundError: org/apache/spark/Partition;` given that option is marked as experimental, I don't know to which extent it is usable.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68616943
Reopening, the problem is still there when running on the standalone cluster.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
GitHub user elmer-garduno reopened a pull request:
https://github.com/apache/spark/pull/3874
[SPARK-5052] Add common/base classes to fix guava methods signatures.
Fixes problems with incorrect method signatures related to shaded classes. For discussion see the jira issue.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/elmer-garduno/spark fix_guava_signatures
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3874.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 #3874
----
commit 596ddbaa8c2f9bf6d2099e799b682d2b15ad9dc6
Author: Elmer Garduno <el...@google.com>
Date: 2015-01-02T03:45:35Z
Add common/base classes to fix guava methods signatures.
----
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68535926
I tried that before using spark.files.userClassPathFirst, but it resulted in a java.lang.NoClassDefFoundError: org/apache/spark/Partition ([full stack trace](https://gist.github.com/elmer-garduno/e65e3d992357253c6111)), which seemed bad enough to not go that way, but maybe someone else here knows the correct way to achieve it.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68607699
The problem seems to be fixed when using `spark-submit` instead of `spark-class`, that seems to set the appropriate precedence on the classpaths, so no need for any special flags or changes here.
Thanks
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71555803
[Test build #26121 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26121/consoleFull) for PR 3874 at commit [`aa5d8e0`](https://github.com/apache/spark/commit/aa5d8e0fd88d9c742b2daf20265da809fa919d2a).
* This patch merges cleanly.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68786110
Latest version LGTM. Thanks!
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68751824
Damn, exposing `Optional` in the API turned out to be a minefield... I see the problem and I don't see a good solution other than adding `Function` to the list of exclusions.
That being said, I think only `Function` should be added. The [source](https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/base/Function.java?name=master) indicates it has no references to other types, so hopefully it shouldn't cause any other issues...
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68891252
Thanks all, anything else left to be done here?
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68775012
Yeah, that's why I'd like to restrict the exclusions as much as possible. Exposing more Guava types makes it harder and harder for people to use a different Guava version in their apps.
I'm still trying to wrap my head around the `VerifyError` in the bug. It doesn't really make a lot of sense. But it might be related to the fact that, if you add `<include>com/google/common/base/Function*</include>` to the exclusions, it's picking up more than just `Function.class` (i.e. it's also picking up `FunctionalEquivalence.class`).
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71564058
Jenkins, retest 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 pull request: [SPARK-5052] Add common/base classes to fix gu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68514968
You're right that I think this is too broad. I think I misspoke earlier. Isn't the theory here that you can bring a later version if Optional with you in your app? Spark barely uses its API. If your copy of Optional hides the one in Spark, which is only there to keep the signature the same, is that OK?
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-68698508
This one always confuses me, but here's what I think I know:
The compiled `Optional` in Spark won't have the "correct" (meaning, matching the Google Guava `Optional`) signatures on its methods since other Guava classes are shaded. It's there for the Spark API to compile against the Guava class in the package that the user app expects.
Apps that uses the API method that uses `Optional` will be bundling Guava. Spark uses Guava 14, although in theory you can use any version... that still has the very few methods on `Optional` that Spark actually calls, I suppose. Because Spark will be using the user app's copy of `Optional` at runtime.
You say you tried it and got `java.lang.NoClassDefFoundError: org/apache/spark/Partition` though. That's weird. `Optional` will be in a different classloader (?) but shouldn't refer to Spark classes. Right? if there's a problem it's somewhere in there, since that's where how I thought this works seems to not match your experience. Or else, there's something else subtly not-quite-right about how the user app is run here.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-71564353
[Test build #26126 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26126/consoleFull) for PR 3874 at commit [`aa5d8e0`](https://github.com/apache/spark/commit/aa5d8e0fd88d9c742b2daf20265da809fa919d2a).
* This patch merges cleanly.
---
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: [SPARK-5052] Add common/base classes to fix gu...
Posted by elmer-garduno <gi...@git.apache.org>.
Github user elmer-garduno commented on the pull request:
https://github.com/apache/spark/pull/3874#issuecomment-69871141
Ping?
---
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