HADOOP-19951. Remove use of jetty-util-ajax and use jackson instead - #8654
Merged
Merged
Conversation
ajfabbri
reviewed
Aug 3, 2026
ajfabbri
left a comment
ajfabbri
left a comment
Contributor
There was a problem hiding this comment.
LGTM except I had a couple of questions.
pjfanning
marked this pull request as draft
August 3, 2026 19:33
pan3793
reviewed
Aug 4, 2026
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
pjfanning
force-pushed
the
HADOOP-19951
branch
2 times, most recently
from
August 4, 2026 13:00
72979b1 to
181f550
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
pjfanning
marked this pull request as ready for review
August 10, 2026 10:49
Member
Author
|
There now appears to be just one test class failing and I think this is unrelated to this PR: |
ajfabbri
approved these changes
Aug 10, 2026
pan3793
reviewed
Aug 11, 2026
| @Override | ||
| public int hashCode() {return toShort();} | ||
|
|
||
| @JsonValue // For Jackson serialization, use the toString() to represent this object as JSON. |
Member
There was a problem hiding this comment.
It's exposing the Jackson API from the Hadoop API, I'm not sure this is a good choice ...
pan3793
requested changes
Aug 11, 2026
pan3793
left a comment
pan3793
left a comment
Member
There was a problem hiding this comment.
@pjfanning thanks for your effort, I go through the non-test code and pom.xml changes, almost LGTM, but I think we should not add com.fasterxml.jackson.annotation.* to Hadoop public API, and I believe it can be replaced with a caller-side static method or something else that easy to implement.
Member
Author
jackson-annotations is a non-optional dependency of jackson-databind - a heavily used lib in Hadoop This jackson-annotations is already in the Hadoop ecosystem. This PR does not add the dependency. Jackson-Databind is heavily dependent on annotations generally. https://github.com/search?q=repo%3Aapache%2Fhadoop+com.fasterxml.jackson.annotation&type=code |
pjfanning
force-pushed
the
HADOOP-19951
branch
from
August 19, 2026 13:12
692f1e0 to
d284887
Compare
Member
Author
I agree that this is not a change that we should look to backport. |
Member
|
Another finding: Same-class issue still open: |
Member
Author
|
@pan3793 I'd prefer to keep the jackson changes in most places but this RBFMetrics use case feels like it might be best off being fixed by going back to the Jetty JSON behaviour. I could inline a cut down version of the jetty 9.4 class. I know it's a big enough class but it looks manageable enough. I could revert some of the Jackson related changes in the hadoop-hdfs-rbf module. |
pan3793
reviewed
Aug 20, 2026
| * Object --> string (dubious!) | ||
| * </pre> | ||
| */ | ||
| public class JSON { |
Member
There was a problem hiding this comment.
can we narrow it to package level? or @Private
pan3793
reviewed
Aug 20, 2026
| hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/nvd3-1.8.5.* (css and js files) | ||
|
|
||
|
|
||
| Apache Software Foundation License 2.0 and Eclipse Public License 1.0 Dual Licensed |
Member
There was a problem hiding this comment.
for dual/multi licensed project, I think we can just choose one instead of listing all
Member
|
|
Member
|
LGTM, only minor issues |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
pan3793
approved these changes
Aug 20, 2026
|
💔 -1 overall
This message was automatically generated. |
pan3793
reviewed
Aug 21, 2026
| @@ -0,0 +1,287 @@ | |||
| // | |||
| // ======================================================================== | |||
Member
There was a problem hiding this comment.
oh, it should be added to the RAT excluded list
pjfanning
marked this pull request as ready for review
August 21, 2026 08:51
Member
|
thanks, merged to trunk |
|
💔 -1 overall
This message was automatically generated. |
6 tasks
susheelgupta7
pushed a commit
to susheelgupta7/hadoop
that referenced
this pull request
Aug 24, 2026
…pache#8654) Authored-by: PJ Fanning <fanningpj@apache.org> Reviewed-by: Aaron Fabbri <fabbri@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org>
vt921102
added a commit
to vt921102/hadoop
that referenced
this pull request
Aug 30, 2026
JSON.java was added to hadoop-hdfs-rbf by HADOOP-19951 (apache#8654) carrying its upstream Jetty EPL/Apache dual-license header, which apache-rat does not recognise as an ASF header. The asflicense check therefore reports one warning on every pull request targeting trunk, not just this one. Exclude the file via apache-rat-plugin the same way other vendored sources are excluded elsewhere in the project. This commit is separable from the HADOOP-19973 fix and can be dropped if it is preferred to address the breakage under its own JIRA.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
HADOOP-19951
Contains content generated by Claude / Xiaomi Mimo.
How was this patch tested?
CI build
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
If an AI tool was used:
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html