You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2017/12/20 17:02:54 UTC
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/20037
[SPARK-22849] ivy.retrieve pattern should also consider `classifier`
## What changes were proposed in this pull request?
In the previous PR https://github.com/apache/spark/pull/5755#discussion_r157848354, we dropped `(-[classifier])` from the retrieval pattern. We should add it back; otherwise,
> If this pattern for instance doesn't has the [type] or [classifier] token, Ivy will download the source/javadoc artifacts to the same file as the regular jar.
## How was this patch tested?
The existing tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark addClassifier
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20037.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 #20037
----
commit 331ba338ce020a927fcfd88b3bd7e536fc8d3b66
Author: gatorsmile <ga...@...>
Date: 2017-12-20T16:59:25Z
fix
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20037
Thanks! Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20037
**[Test build #85197 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85197/testReport)** for PR 20037 at commit [`331ba33`](https://github.com/apache/spark/commit/331ba338ce020a927fcfd88b3bd7e536fc8d3b66).
* This patch **fails PySpark unit 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 #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20037
**[Test build #85197 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85197/testReport)** for PR 20037 at commit [`331ba33`](https://github.com/apache/spark/commit/331ba338ce020a927fcfd88b3bd7e536fc8d3b66).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20037#discussion_r158112362
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -1271,7 +1271,7 @@ private[spark] object SparkSubmitUtils {
// retrieve all resolved dependencies
ivy.retrieve(rr.getModuleDescriptor.getModuleRevisionId,
packagesDirectory.getAbsolutePath + File.separator +
- "[organization]_[artifact]-[revision].[ext]",
+ "[organization]_[artifact]-[revision](-[classifier]).[ext]",
--- End diff --
Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20037
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20037
@srowen I think we are resolving a different issue. Your PR https://github.com/apache/spark/pull/17416 is trying to resolve the issues raised by `spark-submit et al via --packages`, in which users explicitly specify the classifier.
However, we still face another scenarios, the package has the external dependencies. In the dependencies, they have different classifiers. For example, [zookeeper 3.4.6](https://repo1.maven.org/maven2/org/apache/zookeeper/zookeeper/3.4.6/) has tests.jar and regular jar. In the current solution, we will download both to the same file and ivy will return an exception.
```
zookeeper-jar: {artifact=zookeeper, ext=jar, module=zookeeper, classifier=tests, organisation=org.apache.zookeeper, type=test-jar, revision=3.4.6}
zookeeper-jar: {artifact=zookeeper, ext=jar, module=zookeeper, organisation=org.apache.zookeeper, type=jar, revision=3.4.6}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20037
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20037#discussion_r158089460
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -1271,7 +1271,7 @@ private[spark] object SparkSubmitUtils {
// retrieve all resolved dependencies
ivy.retrieve(rr.getModuleDescriptor.getModuleRevisionId,
packagesDirectory.getAbsolutePath + File.separator +
- "[organization]_[artifact]-[revision].[ext]",
+ "[organization]_[artifact]-[revision](-[classifier]).[ext]",
--- End diff --
In my example,
```
zookeeper-jar: {artifact=zookeeper, ext=jar, module=zookeeper, classifier=tests, organisation=org.apache.zookeeper, type=test-jar, revision=3.4.6}
zookeeper-jar: {artifact=zookeeper, ext=jar, module=zookeeper, organisation=org.apache.zookeeper, type=jar, revision=3.4.6}
```
Both dependencies will have the same name `org.apache.zookeeper_zookeeper-3.4.6.jar` and cause the collision. After my PR, they will be different names
`org.apache.zookeeper_zookeeper-3.4.6.jar`
`org.apache.zookeeper_zookeeper-3.4.6-tests.jar`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20037#discussion_r158097751
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -1271,7 +1271,7 @@ private[spark] object SparkSubmitUtils {
// retrieve all resolved dependencies
ivy.retrieve(rr.getModuleDescriptor.getModuleRevisionId,
packagesDirectory.getAbsolutePath + File.separator +
- "[organization]_[artifact]-[revision].[ext]",
+ "[organization]_[artifact]-[revision](-[classifier]).[ext]",
--- End diff --
FWIW this looks fine; I needed a similar change as part of https://github.com/srowen/spark/commit/41267020701be4877be352e1678113bd9870ec12
It's possible you may need some other changes from that WIP commit; not sure.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/20037#discussion_r163463718
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -1271,7 +1271,7 @@ private[spark] object SparkSubmitUtils {
// retrieve all resolved dependencies
ivy.retrieve(rr.getModuleDescriptor.getModuleRevisionId,
packagesDirectory.getAbsolutePath + File.separator +
- "[organization]_[artifact]-[revision].[ext]",
+ "[organization]_[artifact]-[revision](-[classifier]).[ext]",
--- End diff --
I tried it today. Somehow, I only got the test jar downloaded. Have you guys seen this issue?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20037
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85197/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20037: [SPARK-22849] ivy.retrieve pattern should also co...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20037#discussion_r158090484
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
@@ -1271,7 +1271,7 @@ private[spark] object SparkSubmitUtils {
// retrieve all resolved dependencies
ivy.retrieve(rr.getModuleDescriptor.getModuleRevisionId,
packagesDirectory.getAbsolutePath + File.separator +
- "[organization]_[artifact]-[revision].[ext]",
+ "[organization]_[artifact]-[revision](-[classifier]).[ext]",
--- End diff --
The reason why I am putting `classifier` at the end. I am just following the [default artifact partern](https://github.com/apache/ant-ivy/blob/12aeeec70feae05a87a5adfe7b9c2c63744be37f/src/java/org/apache/ivy/core/cache/DefaultRepositoryCacheManager.java#L83) in the Apache IVY.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20037: [SPARK-22849] ivy.retrieve pattern should also consider ...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:
https://github.com/apache/spark/pull/20037
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org