YARN-11897: Make NMWebservices API backward-compatible after Jersey 2…#8227
YARN-11897: Make NMWebservices API backward-compatible after Jersey 2…#8227Hean-Chhinling wants to merge 9 commits intoapache:trunkfrom
Conversation
.../main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/jsonprovider/NMJsonProvider.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @Hean-Chhinling for the fix! LGTM! |
|
Why the Hadoop Yetus has not commented at all? Seems to be a problem it could not detect that there is a new commit in the trunk_YARN-11897 branch. |
daf50cb to
a666c4b
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. |
|
Tested running the tests locally and all pass: |
|
💔 -1 overall
This message was automatically generated. |
|
Very strange the test pass when running locally but failed when running on CI: Update: This is because the file style does not guarantee order. Those updating the test to only use one file like previously. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Hello @K0K0V0K @p-szucs @slfan1989 ! |
|
@K0K0V0K Mind doing a first-pass review on this PR? I’m a bit bandwidth-constrained lately. Thank you very much! |
K0K0V0K
left a comment
There was a problem hiding this comment.
Thanks @Hean-Chhinling for the change this seems mostly fine by me.
I just had a one bigger question left.
...nager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/dao/NMResourceInfo.java
Show resolved
Hide resolved
...ager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/JAXBContextResolver.java
Outdated
Show resolved
Hide resolved
.../main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/jsonprovider/NMJsonProvider.java
Show resolved
Hide resolved
...anager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
@K0K0V0K If it is all looks good to you, could I get a tick on this PR? |
|
Thanks Ling! This looks fine to me. |
|
One minor thing i just saw: may i ask you to fix these styles? |
|
@K0K0V0K Thanks for your proactive review and suggestions; @Hean-Chhinling Thanks for your contributions and support. I’ll complete the review of this PR tomorrow. |
Thank you! I fixed it in my new commit. |
upgrade
Description of PR
Retrieving the logs for a running application fails as the logs CLI expects a JSON object as a response when calling the NM REST API, and it provides a JSON Array
Actual response with Jersey 2:
Expected response with Jersey 1:
How was this patch tested?
Tested on a cluster:
Response format for
/ws/v1/node/containers/{container-id}/logsendpoint seems compatible with the Jersey 1 format comparing them with a nightly version:JSON:
XML:
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