You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mengxr <gi...@git.apache.org> on 2014/11/13 21:20:19 UTC
[GitHub] spark pull request: [SPARK-4326] fix unidoc
GitHub user mengxr opened a pull request:
https://github.com/apache/spark/pull/3253
[SPARK-4326] fix unidoc
There are two issues:
1. specifying guava 11.0.2 will cause hashInt not found in unidoc (any reason to force the version here?)
2. unidoc doesn't recognize static class defined in a base class
@aarondav @srowen @vanzin
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mengxr/spark SPARK-4326
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3253.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 #3253
----
commit 53967bfc5414331160e19f51429926d8cc29ab03
Author: Xiangrui Meng <me...@databricks.com>
Date: 2014-11-13T20:17:54Z
fix unidoc
----
---
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-4326] fix unidoc
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3253
---
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-4326] fix unidoc
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:
https://github.com/apache/spark/pull/3253#discussion_r20322858
--- Diff: network/shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/OpenBlocks.java ---
@@ -23,6 +23,7 @@
import io.netty.buffer.ByteBuf;
import org.apache.spark.network.protocol.Encoders;
+import org.apache.spark.network.shuffle.protocol.BlockTransferMessage.Type;
--- End diff --
Interesting that `org.apache.spark.network.protocol.Message` did not have this same problem, as it follows the same pattern.
---
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-4326] fix unidoc
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/3253#issuecomment-62966799
LGTM too. As @vanzin said, this may allow us to introduce code that breaks only when run inside the NM, which is pretty bad. Until we can find a better way to manage this issue, we'll just have to closely audit any changes made in this package which touch Guava.
---
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-4326] fix unidoc
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3253#issuecomment-62972225
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23324/
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-4326] fix unidoc
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/3253#issuecomment-62967738
Merged this into master and branch-1.2.0 as it's blocking making a snapshot release.
---
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-4326] fix unidoc
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3253#issuecomment-62960357
Looks good, in that I didn't realize anything was explicitly specifying Guava 11. Is removing that going to break something or was that unintentional? the imports are harmless, no problem there. If it fixes unidoc, sounds good.
---
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-4326] fix unidoc
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3253#discussion_r20320194
--- Diff: network/shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/OpenBlocks.java ---
@@ -23,6 +23,7 @@
import io.netty.buffer.ByteBuf;
import org.apache.spark.network.protocol.Encoders;
+import org.apache.spark.network.shuffle.protocol.BlockTransferMessage.Type;
--- End diff --
IDE suggests that this is an unnecessary import, but it is necessary for unidoc. An alternative solution would be defined `Type` outside `BlockTransferMessage`.
---
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-4326] fix unidoc
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3253#discussion_r20320583
--- Diff: network/common/pom.xml ---
@@ -51,7 +51,6 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
- <version>11.0.2</version> <!-- yarn 2.4.0's version -->
--- End diff --
So, the reason why this re-declaration exists is because network/yarn, which depends on this library, is expected to run inside Yarn's NM, and Yarn depends on Guava 11. So this is trying to avoid using newer Guava APIs so that we don't need to do unsanitary things like shading Guava in the NM plugin jar.
I'm OK with removing this if people feel like we'll always remember to audit the usage of Guava in this module, but I'd prefer an approach that keeps the 11.0.2 dependency at least on the maven build (which generates the official Spark artifacts). That could be done by using a property to hold this version and re-defining it to 11.0.2 in the sbt profile, or re-declaring the dependency in SparkBuild.scala as I tried (see the discussion in the bug).
---
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-4326] fix unidoc
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3253#issuecomment-62972216
[Test build #23324 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23324/consoleFull) for PR 3253 at commit [`53967bf`](https://github.com/apache/spark/commit/53967bfc5414331160e19f51429926d8cc29ab03).
* 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-4326] fix unidoc
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3253#issuecomment-62959554
[Test build #23324 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23324/consoleFull) for PR 3253 at commit [`53967bf`](https://github.com/apache/spark/commit/53967bfc5414331160e19f51429926d8cc29ab03).
* 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