You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mccheah <gi...@git.apache.org> on 2015/11/11 03:57:55 UTC
[GitHub] spark pull request: [SPARK-11081] Shade Jersey and javax.rs.ws
GitHub user mccheah opened a pull request:
https://github.com/apache/spark/pull/9615
[SPARK-11081] Shade Jersey and javax.rs.ws
Jersey and javax.rs.ws are commonly depended on and cause dependency
conflicts in many applications that depend on the Spark maven artifacts.
This patch attempts to shade them.
However this patch set is a work in progress. Currently, the javax.rs.ws shading appears to be broken. When I run the mvn package task and generate the assembly jar, and unpack it, I'm missing the javax.rs.ws classes - they're not under org/spark-project/javax/rs/ws. I would appreciate some assistance in debugging how the shading is being done incorrectly here.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/palantir/spark feature/shading-jersey
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/9615.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 #9615
----
commit 83f98c9bae2e3170e7b6653db3edf1f6da649072
Author: mcheah <mc...@palantir.com>
Date: 2015-11-11T01:42:01Z
[SPARK-11081] Shade Jersey and javax.rs.ws
Jersey and javax.rs.ws are commonly depended on and cause dependency
conflicts in many applications that depend on the Spark maven artifacts.
This patch attempts to shade them.
----
---
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-11081] Shade Jersey and javax.rs.ws
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9615#discussion_r44502409
--- Diff: pom.xml ---
@@ -2165,6 +2166,9 @@
<include>org.eclipse.jetty:jetty-security</include>
<include>org.eclipse.jetty:jetty-util</include>
<include>org.eclipse.jetty:jetty-server</include>
+ <include>com.sun.jersey:jersey-core</include>
+ <include>com.sun.jersey:jersey-json</include>
+ <include>com.sun.jersey:jersey-server</include>
--- End diff --
Do you also need to add the `javax.ws.rs` artifacts to this `<includes>` list?
---
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-11081] Shade Jersey and javax.rs.ws
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155644722
Merged build triggered.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155962272
Regarding testing, the best way is to inspect the contents of the jar to make sure that the shaded version is inlined. If Spark code uses the shaded dependency directly, you can also use `javap` to inspect the byte code and make sure that the references are to the shaded versions of jersey rather than the real one.
@mccheah given the comments by @vanzin, can you say more about the specific incompatibility you are facing? It would be good to make sure that if we shade something it's due to a known incompatibility between some set of versions.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155918321
@vanzin we've hit cases specifically where swapping in the newer version of javax.rs.ws causes the Spark UI to break. I'm not convinced that javax.rs.ws versions are compatible between what Spark uses and the newer versions.
@pwendell great I'll try these suggestions.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155980375
We're seeing that exception handling has changed between Jersey 01 and Jersey 02. In our application, we are including both Jersey 2 and javax.ws.rs-api-2.0.1.jar on the driver and executor classpath.
I simplified it to a Spark shell session:
```
bin/spark-shell --master spark://mcheah-mbp:7077 --jars /Users/mcheah/Downloads/jersey-common-2.19.jar,/Users/mcheah/Downloads/javax.ws.rs-api-2.0.1.jar
scala> new javax.ws.rs.NotFoundException()
java.lang.NoSuchMethodError: javax.ws.rs.ClientErrorException.validate(Ljavax/ws/rs/core/Response;Ljavax/ws/rs/core/Response$Status$Family;)Ljavax/ws/rs/core/Response;
at javax.ws.rs.ClientErrorException.<init>(ClientErrorException.java:64)
at javax.ws.rs.NotFoundException.<init>(NotFoundException.java:60)
```
We have other examples as well where similar errors are happening. I think in this case, ```javax.ws.rs.NotFoundException``` extends a class that is being loaded from the Jersey 1 jar, but the class in the Jersey 1 jar isn't compatible with what ```javax.ws.rs-api.2.0.1```'s equivalent class is.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155678946
Another thing to keep in mind, given the recent issues with `@VisibleForTesting`: JAX-RS APIs use lots of annotations, and if you shade them, that is likely to cause problems with scalac-generated code (see #9585).
The good news is that since new versions of these APIs are backwards compatible, you shouldn't need to shade the `javax.rs` stuff at all; someone who wants to override them just needs to put the newer version in front of Spark's classpath. It's not as easy as including it in your application, but it's doable.
Given that, I wonder if it's even necessary to shade jersey at all; the version of jersey used in Spark is old and doesn't clash with the new package name used by newer releases of Jersey.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/9615#discussion_r44503873
--- Diff: pom.xml ---
@@ -2165,6 +2166,9 @@
<include>org.eclipse.jetty:jetty-security</include>
<include>org.eclipse.jetty:jetty-util</include>
<include>org.eclipse.jetty:jetty-server</include>
+ <include>com.sun.jersey:jersey-core</include>
+ <include>com.sun.jersey:jersey-json</include>
+ <include>com.sun.jersey:jersey-server</include>
--- End diff --
Hey @mccheah - if you look at jetty-server as an example, there are other build changes related to shading that you haven't done here:
1. In the root pom, they should be marked as provided.
2. In core/pom.xml they need to be added in includeArtifactIds.
3. They may also need to be listed in other poms as well due to some compiler bugs.
I'd just go through and look everywhere in the source code you see `jetty-server` and do the same for these 3 artifacts. If it's still not working then, let me know and I can take a look.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155649459
**[Test build #45586 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45586/consoleFull)** for PR 9615 at commit [`83f98c9`](https://github.com/apache/spark/commit/83f98c9bae2e3170e7b6653db3edf1f6da649072).
* 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 pull request: [SPARK-11081] Shade Jersey and javax.rs.ws
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155644768
Merged build started.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155649474
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45586/
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-11081] Shade Jersey and javax.rs.ws
Posted by mccheah <gi...@git.apache.org>.
Github user mccheah commented on a diff in the pull request:
https://github.com/apache/spark/pull/9615#discussion_r44502578
--- Diff: pom.xml ---
@@ -2165,6 +2166,9 @@
<include>org.eclipse.jetty:jetty-security</include>
<include>org.eclipse.jetty:jetty-util</include>
<include>org.eclipse.jetty:jetty-server</include>
+ <include>com.sun.jersey:jersey-core</include>
+ <include>com.sun.jersey:jersey-json</include>
+ <include>com.sun.jersey:jersey-server</include>
--- End diff --
javax.ws.rs is actually inside the Jersey artifacts.
The shading didn't work as I expected regardless if I added the javax.rs.ws stuff to here as well though.
---
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-11081] Shade Jersey and javax.rs.ws
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155977770
My concern is specifically with shading the annotations; we've seen that it causes issues with scalac-generated code. If you manage to fix the PR to shade jax-rs annotations, please do extensive tests with spark-shell to make sure it still works.
I'm also interested in which incompatibilities you ran into. These APIs are generally pretty good at maintaining backwards compatibility; unless you're using some milestone build (I remember pre-release jars of the JAX-RS 2.0 API having some differences from the final version), it should all 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: [SPARK-11081] Shade Jersey and javax.rs.ws
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155648696
**[Test build #45586 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45586/consoleFull)** for PR 9615 at commit [`83f98c9`](https://github.com/apache/spark/commit/83f98c9bae2e3170e7b6653db3edf1f6da649072).
---
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-11081] Shade Jersey and javax.rs.ws
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9615#issuecomment-155649470
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