-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-15984. Update jersey from 1.19 to 2.x. #7019
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
Changes from 12 commits
92160ab
fc8f03e
fff1b6a
3237736
31d28a7
9601cb9
c1eca7e
96a29fc
f8530cb
c2484ae
290cae1
ffd73e4
dde4dee
0dee8ac
842886f
29fdc9c
8d636ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,7 +55,7 @@ pipeline { | |
| environment { | ||
| YETUS='yetus' | ||
| // Branch or tag name. Yetus release tags are 'rel/X.Y.Z' | ||
| YETUS_VERSION='rel/0.14.0' | ||
| YETUS_VERSION='a7d29a6a72750a0c5c39512f33945e773e69303e' | ||
| } | ||
|
|
||
| parameters { | ||
|
|
@@ -71,7 +71,7 @@ pipeline { | |
| checkout([ | ||
| $class: 'GitSCM', | ||
| branches: [[name: "${env.YETUS_VERSION}"]], | ||
| userRemoteConfigs: [[ url: 'https://github.com/apache/yetus.git']]] | ||
| userRemoteConfigs: [[ url: 'https://github.com/ayushtkn/yetus.git']]] | ||
|
||
| ) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,6 +159,24 @@ | |
| <exclude>org.xerial.snappy:*</exclude> | ||
| <!-- leave out kotlin classes --> | ||
| <exclude>org.jetbrains.kotlin:*</exclude> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do these no longer need to be cut?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have completed the fixes in the latest version. |
||
| <exclude>org.glassfish.jersey.test-framework:*</exclude> | ||
| <exclude>org.glassfish.jersey.media:*</exclude> | ||
| <exclude>org.glassfish.jersey.containers:*</exclude> | ||
| <exclude>org.glassfish.jersey.test-framework.providers:*</exclude> | ||
| <exclude>org.glassfish.hk2:*</exclude> | ||
| <exclude>org.glassfish.jersey.inject:*</exclude> | ||
| <exclude>org.glassfish.grizzly:*</exclude> | ||
| <exclude>org.glassfish.jersey.core:*</exclude> | ||
| <exclude>org.glassfish.hk2.external:*</exclude> | ||
| <exclude>org.glassfish.jaxb:*</exclude> | ||
| <exclude>jakarta.ws.rs:*</exclude> | ||
| <exclude>jakarta.annotation:*</exclude> | ||
| <exclude>jakarta.validation:*</exclude> | ||
| <exclude>jakarta.servlet:*</exclude> | ||
| <exclude>javax.annotation:*</exclude> | ||
| <exclude>org.hamcrest:*</exclude> | ||
| <exclude>aopalliance:*</exclude> | ||
| <exclude>javassist:*</exclude> | ||
| </excludes> | ||
| </artifactSet> | ||
| <filters> | ||
|
|
@@ -184,13 +202,6 @@ | |
| <exclude>org/apache/jasper/compiler/Localizer.class</exclude> | ||
| </excludes> | ||
| </filter> | ||
| <!-- We rely on jersey for our web interfaces. We want to use its java services stuff only internal to jersey --> | ||
| <filter> | ||
| <artifact>com.sun.jersey:*</artifact> | ||
| <excludes> | ||
| <exclude>META-INF/services/javax.*</exclude> | ||
| </excludes> | ||
| </filter> | ||
| <filter> | ||
| <!-- skip french localization --> | ||
| <artifact>org.apache.commons:commons-math3</artifact> | ||
|
|
@@ -247,6 +258,7 @@ | |
| <exclude>META-INF/versions/9/module-info.class</exclude> | ||
| <exclude>META-INF/versions/11/module-info.class</exclude> | ||
| <exclude>META-INF/versions/18/module-info.class</exclude> | ||
| <exclude>META-INF/versions/9/javax/xml/bind/ModuleUtil.class</exclude> | ||
| </excludes> | ||
| </filter> | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like the wrong license group
https://mvnrepository.com/artifact/org.glassfish.jersey.core/jersey-common/2.46
jersey 2 is licensed under Apache License (very important) and 3 other licenses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your reply! I will improve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if jersey 2 is dual licensed. Reading its license and notice makes it look like it is EPL-2.0 licensed by with some parts licensed under 1 or more of the other 3 licenses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can wait for input from the other members, as I'm also unsure about the best way to improve it. Once we have everyone's feedback, we can decide on the changes together. It's a good sign that we've already started reviewing this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in the "merge this and then deal with the details" mode right now. Get this in and then we can start the real burndown for java17.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@steveloughran Thank you for the acknowledgment! If there are no further comments, I plan to merge this PR in two days. After multiple rounds of testing, everything is performing as expected. I’ve personally debugged each unit test improvement to ensure they work in most scenarios. Of course, there may still be some occasional issues with individual tests (though the likelihood is low), and we can continue refining them in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is OK to located this package to Apache License or MIT if it is dual licensed IMO, if we could not make sure, maybe refer Spark https://github.com/apache/spark/blob/master/LICENSE-binary and write it under Apache License will not be wrong. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Hexiaoqiao @steveloughran In the latest commit, I have rolled back the Jenkinsfile commit, which may cause the next build to not complete successfully, as the current CI cannot compile code with more than 10,000 lines. However, this is expected, so please don't worry.