Skip to content

HADOOP-18022. Add restrict-imports-enforcer-rule for Guava Preconditions and remove remaining usages #3712

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 29, 2021
Merged

HADOOP-18022. Add restrict-imports-enforcer-rule for Guava Preconditions and remove remaining usages #3712

merged 5 commits into from
Nov 29, 2021

Conversation

@virajjasani
Copy link
Contributor

@virajjasani virajjasani commented Nov 23, 2021

Description of PR

Add restrict-imports-enforcer-rule for Guava Preconditions in hadoop-main pom to restrict any new import in future. Remove any remaining usages of Guava Preconditions from the codebase.

How was this patch tested?

Local testing.

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?

@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 23, 2021

Could you please take a look @tasanuma? Thanks

@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 25, 2021

JFYI @tasanuma, I am working on some test failures. Hopefully QA result tomorrow should be clean.

@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 25, 2021

Removed Preconditions usage from hadoop-yarn-server-timelineservice-hbase module because test module hadoop-yarn-server-timelineservice-hbase-tests depends on hbase compatible hadoop-common version and that old version would not have Preconditions included so good to replace Preconditions usages with relevant if conditions.

@virajjasani virajjasani changed the title HADOOP-18022. Add restrict-imports-enforcer-rule for Guava Preconditions in hadoop-main pom HADOOP-18022. Add restrict-imports-enforcer-rule for Guava Preconditions and remove remaining usages Nov 25, 2021
@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani virajjasani mentioned this pull request Nov 25, 2021
Merged
3 tasks
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 26, 2021

Recent test failures are not relevant. Timeline service issue is fixed with this PR.

@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 26, 2021

PR #3722 is blocked on this. Once this PR is merged, will try to run the build on #3722.
Thanks

@tasanuma
Copy link
Member

tasanuma commented Nov 26, 2021

Thanks for working on this, @virajjasani.
I found AbfsLocatedFileStatus.java has a static import of Preconditions of hadoop-thirdparty. Could you remove it? I'm not sure why banned-illegal-imports can't catch it.

@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 26, 2021

Thanks @tasanuma for the review.

I'm not sure why banned-illegal-imports can't catch it.

Yeah indeed good question. Thanks to this incident that I realized that static imports are not caught unless explicitly mentioned so I have fixed pom for Preconditions static imports in the recent commit.

As part of separate Jira, we can add static import tag for all other such banned imports. Sounds good?

@tasanuma
Copy link
Member

tasanuma commented Nov 26, 2021

Thanks for fixing it. Sounds good. Please go ahead.

@tasanuma
Copy link
Member

tasanuma commented Nov 26, 2021

+1, pending Jenkins.

@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 26, 2021 •
edited
Loading

Thanks @tasanuma. Sure, once this PR is merged, will create another Jira and PR to include static imports for the classes we have added so far to avoid merge conflicts :)

@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus
Copy link

hadoop-yetus commented Nov 27, 2021

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 42s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ pathlen 0m 0s /results-pathlen.txt The patch appears to contain 3 files with names longer than 240
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 12m 49s Maven dependency ordering for branch
+1 💚 mvninstall 21m 27s trunk passed
+1 💚 compile 21m 50s trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 compile 19m 4s trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 checkstyle 3m 42s trunk passed
+1 💚 mvnsite 25m 15s trunk passed
+1 💚 javadoc 7m 49s trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javadoc 7m 58s trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 spotbugs 39m 35s trunk passed
+1 💚 shadedclient 51m 34s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 40s Maven dependency ordering for patch
+1 💚 mvninstall 25m 1s the patch passed
+1 💚 compile 21m 19s the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javac 21m 19s the patch passed
+1 💚 compile 19m 12s the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 javac 19m 12s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 3m 37s the patch passed
+1 💚 mvnsite 21m 4s the patch passed
+1 💚 xml 0m 2s The patch has no ill-formed XML file.
+1 💚 javadoc 8m 22s the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javadoc 8m 2s the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 spotbugs 41m 39s the patch passed
+1 💚 shadedclient 52m 4s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 773m 24s /patch-unit-root.txt root in the patch passed.
+1 💚 asflicense 1m 37s The patch does not generate ASF License warnings.
1128m 50s
Reason Tests
Failed junit tests hadoop.fs.http.client.TestHttpFSFWithSWebhdfsFileSystem
hadoop.fs.http.client.TestHttpFSFWithWebhdfsFileSystem
hadoop.tools.dynamometer.TestDynamometerInfra
hadoop.yarn.csi.client.TestCsiClient
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3712/15/artifact/out/Dockerfile
GITHUB PR #3712
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell xml
uname Linux b4bc90d37f72 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 6cbd30e
Default Java Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3712/15/testReport/
Max. process+thread count 3597 (vs. ulimit of 5500)
modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice-hbase/hadoop-yarn-server-timelineservice-hbase-client hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask hadoop-tools/hadoop-azure hadoop-cloud-storage-project/hadoop-cos hadoop-cloud-storage-project/hadoop-huaweicloud . U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3712/15/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

@virajjasani
Copy link
Contributor Author

virajjasani commented Nov 27, 2021

Done, finally only known and irrelevant test failures are present in the latest QA results.

aajisaka
aajisaka approved these changes Nov 29, 2021
View reviewed changes
Copy link
Member

@aajisaka aajisaka left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @virajjasani and @tasanuma

@tasanuma tasanuma merged commit 215388b into apache:trunk Nov 29, 2021
@tasanuma
Copy link
Member

tasanuma commented Nov 29, 2021

Thanks for your contribution, @virajjasani. Thanks for your review, @aajisaka.
Please go ahead with the following tasks.

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022
@virajjasani
HADOOP-18022. Add restrict-imports-enforcer-rule for Guava Preconditi…
9cbebcc 
…ons and remove remaining usages (apache#3712)

Reviewed-by: Akira Ajisaka <aajisaka@apache.org>
Signed-off-by: Takanobu Asanuma <tasanuma@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aajisaka aajisaka aajisaka approved these changes

@tasanuma tasanuma tasanuma approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@virajjasani @hadoop-yetus @tasanuma @aajisaka