Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-25863 Shade javax.ws.rs package for use with shaded Jersey #51

Merged
merged 2 commits into from
Nov 29, 2021

Conversation

ndimiduk
Copy link
Member

@ndimiduk ndimiduk commented May 7, 2021

From the About text,

Jersey RESTful Web Services 2.x framework is open source, production quality, framework for
developing RESTful Web Services in Java that provides support for JAX-RS APIs and serves as a
JAX-RS (JSR 311 & JSR 339 & JSR 370) Reference Implementation.

javax.ws.rs is defined by the JSRs, so it doesn't make sense that we could have multiple
implementations of that JSR on the classpath simultaniously (via jersey-server-1.x and
jersey-server-2.x jars) without them colliding. By shading over the JSR package space in the
jersey-server-2.x implementation jars, we achieve a completely isolated JSR runtime.

@ndimiduk
Copy link
Member Author

ndimiduk commented May 7, 2021

Parking this here for now. Will be back with a PR vs. hbase-core that makes use of this, so we can verify it's all working the way we intend.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

ndimiduk added a commit to ndimiduk/hbase that referenced this pull request May 7, 2021
Copy link
Contributor

@busbey busbey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the relocated jaxrs provider is intimately tied to our use of jersey, can we fold this into our packaged relocation of jersey in the hbase-shaded-jersey module?

hbase-shaded-jackson-jaxrs-json-provider/pom.xml Outdated Show resolved Hide resolved
@ndimiduk
Copy link
Member Author

Since the relocated jaxrs provider is intimately tied to our use of jersey, can we fold this into our packaged relocation of jersey in the hbase-shaded-jersey module?

I chose intentionally not to do that, because we don't currently use this dependency in the hbase-server classpath. It is only used by hbase-rest. I believe we want to finish the purge of our use of jackson, which means eventually removing this jar and replacing it with an EntityWriter based on Gson.

@busbey
Copy link
Contributor

busbey commented May 10, 2021

Since the relocated jaxrs provider is intimately tied to our use of jersey, can we fold this into our packaged relocation of jersey in the hbase-shaded-jersey module?

I chose intentionally not to do that, because we don't currently use this dependency in the hbase-server classpath. It is only used by hbase-rest. I believe we want to finish the purge of our use of jackson, which means eventually removing this jar and replacing it with an EntityWriter based on Gson.

Please put a comment to this end in the module pom

@ndimiduk ndimiduk force-pushed the 25863-shade-jersey-javax.ws.rs branch from fdd0a55 to 74b2d75 Compare May 10, 2021 21:26
@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 35s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+0 🆗 mvndep 0m 6s Maven dependency ordering for branch
+1 💚 mvninstall 0m 23s master passed
+1 💚 compile 0m 15s master passed
+1 💚 javadoc 0m 9s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 4s Maven dependency ordering for patch
+1 💚 mvninstall 0m 37s the patch passed
+1 💚 compile 0m 19s the patch passed
+1 💚 javac 0m 19s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 xml 0m 1s The patch has no ill-formed XML file.
+1 💚 javadoc 0m 13s the patch passed
_ Other Tests _
+1 💚 unit 0m 5s hbase-shaded-jersey in the patch passed.
+1 💚 unit 0m 4s hbase-shaded-jackson-jaxrs-json-provider in the patch passed.
+1 💚 unit 0m 29s root in the patch passed.
+1 💚 asflicense 0m 12s The patch does not generate ASF License warnings.
3m 44s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Thirdparty-PreCommit/job/PR-51/3/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #51
Optional Tests dupname asflicense javac javadoc unit xml compile
uname Linux f4d1f8dc973d 5.4.0-1047-aws #49~18.04.1-Ubuntu SMP Wed Apr 28 23:08:58 UTC 2021 x86_64 GNU/Linux
Build tool maven
git revision master / 1d73a2e
Default Java Oracle Corporation-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Thirdparty-PreCommit/job/PR-51/3/testReport/
Max. process+thread count 396 (vs. ulimit of 1000)
modules C: hbase-shaded-jersey hbase-shaded-jackson-jaxrs-json-provider . U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Thirdparty-PreCommit/job/PR-51/3/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@ndimiduk ndimiduk marked this pull request as ready for review May 18, 2021 23:36
@ndimiduk ndimiduk force-pushed the 25863-shade-jersey-javax.ws.rs branch from 74b2d75 to 48e61a5 Compare August 25, 2021 22:32
ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Aug 25, 2021
@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 51s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+0 🆗 mvndep 0m 20s Maven dependency ordering for branch
+1 💚 mvninstall 0m 34s master passed
+1 💚 compile 0m 48s master passed
+1 💚 javadoc 0m 36s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 23s master passed
+0 🆗 mvndep 0m 28s Maven dependency ordering for patch
+1 💚 mvninstall 1m 27s the patch passed
+1 💚 compile 0m 51s the patch passed
+1 💚 javac 0m 51s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 xml 0m 5s The patch has no ill-formed XML file.
+1 💚 javadoc 0m 39s the patch passed
_ Other Tests _
+1 💚 unit 0m 9s hbase-shaded-protobuf in the patch passed.
+1 💚 unit 0m 5s hbase-shaded-netty in the patch passed.
+1 💚 unit 0m 4s hbase-shaded-gson in the patch passed.
+1 💚 unit 0m 4s hbase-shaded-miscellaneous in the patch passed.
+1 💚 unit 0m 5s hbase-shaded-jetty in the patch passed.
+1 💚 unit 0m 5s hbase-shaded-jersey in the patch passed.
+1 💚 unit 0m 4s hbase-shaded-jackson-jaxrs-json-provider in the patch passed.
+1 💚 unit 0m 25s hbase-noop-htrace in the patch passed.
+1 💚 unit 0m 30s root in the patch passed.
+1 💚 asflicense 0m 38s The patch does not generate ASF License warnings.
9m 19s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Thirdparty-PreCommit/job/PR-51/4/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #51
Optional Tests dupname asflicense javac javadoc unit xml compile
uname Linux 716d9236acd4 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / c28a235
Default Java Oracle Corporation-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Thirdparty-PreCommit/job/PR-51/4/testReport/
Max. process+thread count 414 (vs. ulimit of 1000)
modules C: hbase-shaded-protobuf hbase-shaded-netty hbase-shaded-gson hbase-shaded-miscellaneous hbase-shaded-jetty hbase-shaded-jersey hbase-shaded-jackson-jaxrs-json-provider hbase-noop-htrace . U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Thirdparty-PreCommit/job/PR-51/4/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Copy link
Contributor

Apache9 commented Nov 28, 2021

So any updates here?

I plan to do a 4.0.0 release for hbase-thirdparty, so we can include this now?

Thanks.

@apurtell
Copy link
Contributor

@Apache9 There are two approvals here, I say just go ahead

@ndimiduk
Copy link
Member Author

This PR is probably fine to merge. I'd wanted to land the dependent feature in HBase before committing here, but I've been pulled onto other projects and haven't gotten back to finishing it.

@ndimiduk ndimiduk force-pushed the 25863-shade-jersey-javax.ws.rs branch from 48e61a5 to fee9e85 Compare November 29, 2021 22:23
… shaded Jersey

From the [About](https://eclipse-ee4j.github.io/jersey/) text,

> Jersey RESTful Web Services 2.x framework is open source, production quality, framework for
> developing RESTful Web Services in Java that provides support for JAX-RS APIs and serves as a
> JAX-RS (JSR 311 & JSR 339 & JSR 370) Reference Implementation.

`javax.ws.rs` is defined by the JSRs, so it doesn't make sense that we could have multiple
implementations of that JSR on the classpath simultaniously (via jersey-server-1.x and
jersey-server-2.x jars) without them colliding. By shading over the JSR package space in the
jersey-server-2.x implementation jars, we achieve a completely isolated JSR runtime.

Signed-off-by: Sean Busbey <busbey@apache.org>
Signed-off-by: Andrew Purtell <apurtell@apache.org>
…use with shaded jersey

Make this provider module compatible with our shaded Jersey.

Signed-off-by: Sean Busbey <busbey@apache.org>
Signed-off-by: Andrew Purtell <apurtell@apache.org>
@ndimiduk ndimiduk force-pushed the 25863-shade-jersey-javax.ws.rs branch from fee9e85 to f902b25 Compare November 29, 2021 22:27
@ndimiduk ndimiduk merged commit f9a7052 into apache:master Nov 29, 2021
@ndimiduk ndimiduk deleted the 25863-shade-jersey-javax.ws.rs branch November 29, 2021 22:31
ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Nov 30, 2021
ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Nov 30, 2021
Adapt to the changes provided by apache/hbase-thirdparty#51

Signed-off-by: Andrew Purtell <apurtell@apache.org>
ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Dec 9, 2021
Adapt to the changes provided by apache/hbase-thirdparty#51

Signed-off-by: Andrew Purtell <apurtell@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants