-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
YARN-11765. Refactor: Move Clock Class from hadoop-mapreduce-project to hadoop-common-project for Reusability #7352
base: trunk
Are you sure you want to change the base?
Conversation
@yangjiandan Thank you for your contribution! The jdiff file cannot be modified as it is the XML file used to mark API changes during the release version. |
@slfan1989 Thank you for the clarification! I will adjust my changes accordingly. |
💔 -1 overall
This message was automatically generated. |
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.
@yangjiandan , that's a nice refactor!
I'm sorry to say it, but I'm not sure we can proceed with this. The YARN classes are annotated as @Public
, and it would be a backward-incompatible change.
https://hadoop.apache.org/docs/current/hadoop-project-dist/hadoop-common/Compatibility.html#Java_API
I know it is used in at least Hive and Tez:
We could theoretically make a change like this in a 4.0.0 release, but I haven't seen any plans for a major version release like that.
💔 -1 overall
This message was automatically generated. |
@cnauroth Thank you for your review and the detailed explanation! To address your concern, I propose the following solution:
|
...arn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
Outdated
Show resolved
Hide resolved
...rn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/WindowsBasedProcessTree.java
Outdated
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
b69ddd2 is my lastest version, and hadoop-yetus has +1. @slfan1989 Could you please review the code again? Let me know if any further changes are needed. Thanks! |
Thank you for your contribution! I need some time to review this PR. |
b69ddd2
to
ee885ef
Compare
The community trunk branch has introduced JUnit 5-related changes, which conflicted with this modification. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Description of PR
see https://issues.apache.org/jira/browse/YARN-11765
How was this patch tested?
current ut