Skip to content

HDDS-8026. Replace import from shaded Guava#4314

Merged
adoroszlai merged 2 commits intoapache:masterfrom
adoroszlai:HDDS-8026
Feb 24, 2023
Merged

HDDS-8026. Replace import from shaded Guava#4314
adoroszlai merged 2 commits intoapache:masterfrom
adoroszlai:HDDS-8026

Conversation

@adoroszlai
Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai commented Feb 24, 2023

What changes were proposed in this pull request?

Directly use Guava classes instead of the shaded ones from ratis-thirdparty.

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

How was this patch tested?

$ rg --glob '*.java' org.apache.ratis.thirdparty.com.google.common | wc -l
0

Regular CI:
https://github.com/adoroszlai/hadoop-ozone/actions/runs/4262730397

@adoroszlai adoroszlai self-assigned this Feb 24, 2023
Copy link
Copy Markdown
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. But this would be a continuous effort I feel. People will keep on adding it and someone has to regularly volunteer to get rid of them.

Is there some scope to enforce checks to prevent this in future. Hadoop I feel does something like this here:

https://github.com/apache/hadoop/blob/4067facae6eac03ec31612134dc93b113d09ee80/pom.xml#L173-L188

@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ayushtkn for the suggestion, added the restriction.

Without the import replacements:

[ERROR] Rule 1: org.apache.maven.plugins.enforcer.RestrictImports failed with message:
[ERROR] 
[ERROR] Banned imports detected:
[ERROR] 
[ERROR] Reason: Use directly from Guava
[ERROR] 	in file: org/apache/hadoop/hdds/utils/db/RDBStore.java
[ERROR] 		org.apache.ratis.thirdparty.com.google.common.annotations.VisibleForTesting 	(Line: 45, Matched by: org.apache.ratis.thirdparty.com.google.common.**)

Copy link
Copy Markdown
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adoroszlai adoroszlai merged commit aebc06f into apache:master Feb 24, 2023
@adoroszlai adoroszlai deleted the HDDS-8026 branch February 24, 2023 21:55
@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ayushtkn, @kerneltime for the review.

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.

3 participants