-
Notifications
You must be signed in to change notification settings - Fork 594
HDDS-14583. Remove dependency on commons-pool2 in hdds-common #9732
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
Conversation
|
Thanks @AdyChechani for the patch. Please enable workflows in your fork. Also, can you please let me know your Apache Jira username? |
Looks like we need to add <dependency>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</dependency> |
|
Thanks @AdyChechani for updating the patch and enabling workflows. CI found one more change is needed due to checkstyle problem: Please copy |
|
Hi @adoroszlai, Thanks for noticing this. I have created a patch for this. Can you please tell me which local checks or scripts I should run before committing/opening a PR in the future, so I can catch these issues earlier? Thanks! |
I recommend running build and checkstyle locally for all changes, these are quick: mvn -DskipRecon -DskipShade -DskipTests clean verify
./hadoop-ozone/dev-support/checks/checkstyle.shThen push to your fork and let CI complete before opening your PR. |
hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/util/package-info.java
Outdated
Show resolved
Hide resolved
|
Thanks @AdyChechani for the patch. |
What changes were proposed in this pull request?
This PR moves server-only serialization utilities from
hadoop-hdds/commontohadoop-hdds/framework, enabling the removal of the commons-pool2 dependency from the common module.Changes:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/util/tohadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/util/:hadoop-hdds/common/pom.xml:hadoop-hdds/framework/pom.xml:What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14583
How was this patch tested?