Skip to content
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

HDDS-11586. Remove duplicate OzoneManagerProtocolServerSideTranslatorPB#OM_REQUESTS_PACKAGE #7317

Closed
wants to merge 1 commit into from

Conversation

jianghuazhu
Copy link
Contributor

@jianghuazhu jianghuazhu commented Oct 15, 2024

What changes were proposed in this pull request?

The same validation package is defined in OzoneManagerProtocolServerSideTranslatorPB and RequestValidations, so you only need to keep one of them.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11586

How was this patch tested?

ci:
https://github.com/jianghuazhu/ozone/actions/runs/11345726134

@jianghuazhu
Copy link
Contributor Author

@fapifta @adoroszlai , can you help review this PR?
Thanks.

@adoroszlai
Copy link
Contributor

I don't really think these are duplicates. RequestValidations provides a default value for convenience. OzoneManagerProtocolServerSideTranslatorPB sets explicit value for safety. The two values happen to be the same.

@jianghuazhu
Copy link
Contributor Author

Thanks @adoroszlai for the comment and review.
Because the default DEFAULT_PACKAGE has been defined in RequestValidations, and the DEFAULT_PACKAGE has been assigned by default in RequestValidations#validationsPackageName. Therefore, I think it is not necessary to define the same value in OzoneManagerProtocolServerSideTranslatorPB, which seems a bit redundant.

......
private static final String DEFAULT_PACKAGE = "org.apache.hadoop.ozone";
private String validationsPackageName = DEFAULT_PACKAGE;
......

If it is for security reasons, I think it is okay not to modify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants