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 2014/08/06 10:23:49 UTC
[GitHub] spark pull request: SPARK-1022 [BUILD] Depend on Zookeeper from Ka...
GitHub user srowen opened a pull request:
https://github.com/apache/spark/pull/1804
SPARK-1022 [BUILD] Depend on Zookeeper from Kafka tests
Per discussion at https://github.com/apache/spark/pull/1751 I suggest that the more correct thing to do is to depend on Zookeeper from Kafka tests. The first commit does this.
The second commit reflects the explicit dependency on ZK in core, in order to make `zookeeper.version` do something. Another reasonable change would be to simply remove `zookeeper.version` to reflect that otherwise it does nothing.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/srowen/spark SPARK-1022
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1804.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 #1804
----
commit 109f80e23d2f9909afe57b8598f7cef9c0f6c310
Author: Sean Owen <sr...@gmail.com>
Date: 2014-08-06T08:21:14Z
Directly depend on zookeeper in external/kafka tests, because tests use ZK directly
commit 4cbd04d62eb5e93cab9a237aea7209b66b67b4be
Author: Sean Owen <sr...@gmail.com>
Date: 2014-08-06T08:22:01Z
Depend on ZK directly, with Curator, to pull in version specified by zookeeper.version
----
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51409148
Continuing the discussion in #1751, an alternative solution is to apply this explicit zookeeper dependency on only the spark-stremaing-kafka module (since immediately explicitly needs it) as opposed to the spark-core module (which may make it more convenient to control zookeeper version across all vendors builds).
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51330035
QA results for PR 1804:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18019/consoleFull
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51325551
QA tests have started for PR 1804. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18019/consoleFull
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51321185
QA results for PR 1804:<br>- This patch FAILED unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18018/consoleFull
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-52945190
Bah, I give up on this. Not clear whether the test failure is a false positive. At this point, the point of the patch is really just "zookeeper.properties is not used, but the build acts like it thinks it is used", and that's minor. Somebody zap that someday...
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-52909994
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19052/consoleFull) for PR 1804 at commit [`eb92ed8`](https://github.com/apache/spark/commit/eb92ed8142f9dbcc26438dde63c6fa6f516d92be).
* 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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51321089
QA tests have started for PR 1804. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18018/consoleFull
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51320964
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by srowen <gi...@git.apache.org>.
Github user srowen closed the pull request at:
https://github.com/apache/spark/pull/1804
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-52914255
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19052/consoleFull) for PR 1804 at commit [`eb92ed8`](https://github.com/apache/spark/commit/eb92ed8142f9dbcc26438dde63c6fa6f516d92be).
* This patch **fails** unit tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `In multiclass classification, all `$2^`
* `public final class JavaDecisionTree `
---
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-1022 [BUILD] Depend on Zookeeper from Ka...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/1804#issuecomment-51606301
@tdas also 'porting' my comment -- the problem is that right now, without the dependency declared in `core`, there is no way to control which version of ZK goes into Spark core. You get what Curator happens to depend on. Core already depends on ZK indirectly.
However, I see no reason that Spark cares about ZK per se. Vendors can control the ZK versions downstream if it maters. So it's probably less confusing to just remove `zookeeper.version` too. Right now it's overridden in the MapR profile, as if it does something, when it doesn't.
The explicit dependency in Kafka tests can directly specify a version of, say, 3.4.5 for its own purpose. That fully contains the ZK dependency issue and seems entirely correct to me. Updated PR coming...
---
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