Skip to content

HBASE-27203 Clean up error-prone findings in hbase-client - #4644

Merged
apurtell merged 3 commits into
apache:masterfrom
apurtell:HBASE-27203
Jul 26, 2022
Merged

HBASE-27203 Clean up error-prone findings in hbase-client#4644
apurtell merged 3 commits into
apache:masterfrom
apurtell:HBASE-27203

Conversation

@apurtell

@apurtellapurtell commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@Apache-HBase

Apache-HBase commented Jul 23, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 13sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 28smaster passed
+1 💚compile0m 18smaster passed
+1 💚shadedjars3m 45sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 17smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 19sthe patch passed
+1 💚compile0m 18sthe patch passed
+1 💚javac0m 18sthe patch passed
+1 💚shadedjars3m 43spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 16sthe patch passed
_ Other Tests _
+1 💚unit1m 13shbase-client in the patch passed.
17m 8s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#4644
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 08671c6059b5 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 31fc97e
Default JavaAdoptOpenJDK-1.8.0_282-b08
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/testReport/
Max. process+thread count159 (vs. ulimit of 30000)
modulesC: hbase-client U: hbase-client
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Jul 23, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 40sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 59smaster passed
+1 💚compile0m 17smaster passed
+1 💚shadedjars4m 5sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 20smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 33sthe patch passed
+1 💚compile0m 17sthe patch passed
+1 💚javac0m 17sthe patch passed
+1 💚shadedjars3m 54spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 14shbase-client generated 0 new + 44 unchanged - 11 fixed = 44 total (was 55)
_ Other Tests _
+1 💚unit1m 21shbase-client in the patch passed.
17m 56s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#4644
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 10f5b0b4bad4 5.4.0-1043-aws #45~18.04.1-Ubuntu SMP Fri Apr 9 23:32:25 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 31fc97e
Default JavaAdoptOpenJDK-11.0.10+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/testReport/
Max. process+thread count189 (vs. ulimit of 30000)
modulesC: hbase-client U: hbase-client
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Jul 23, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 36sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 1sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall2m 23smaster passed
+1 💚compile0m 35smaster passed
+1 💚checkstyle0m 14smaster passed
+1 💚spotless0m 42sbranch has no errors when running spotless:check.
+1 💚spotbugs0m 36smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 8sthe patch passed
+1 💚compile0m 33sthe patch passed
-0 ⚠️javac0m 33shbase-client generated 29 new + 5 unchanged - 118 fixed = 34 total (was 123)
+1 💚checkstyle0m 12shbase-client: The patch generated 0 new + 128 unchanged - 75 fixed = 128 total (was 203)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck11m 30sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotless0m 40spatch has no errors when running spotless:check.
+1 💚spotbugs0m 40sthe patch passed
_ Other Tests _
+1 💚asflicense0m 8sThe patch does not generate ASF License warnings.
27m 24s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#4644
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
unameLinux ffd6408f3fe7 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 31fc97e
Default JavaAdoptOpenJDK-1.8.0_282-b08
javachttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-client.txt
Max. process+thread count64 (vs. ulimit of 30000)
modulesC: hbase-client U: hbase-client
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/1/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache9
Apache9 reviewed Jul 23, 2022
View reviewed changes

@Apache9Apache9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall LGTM. Left some comments.

Thanks @apurtell for the hard work!

Comment threadhbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java Outdated
Comment threadhbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java Outdated
Comment threadhbase-client/src/main/java/org/apache/hadoop/hbase/client/Append.java Outdated
Comment threadhbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
Comment threadhbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
Comment thread...java/org/apache/hadoop/hbase/security/provider/AbstractSaslClientAuthenticationProvider.java Outdated
Comment threadhbase-client/src/test/java/org/apache/hadoop/hbase/client/TestScan.java
} catch (IllegalArgumentException iae) {
} catch (Exception e) {
fail("expected IllegalArgumentException to be thrown");
// Expected

@Apache9Apache9Jul 23, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we could file another issue to change these tests to use assertThrows. Anyway, the current modifications make the code better, can open a follow on issue for changing to assertThrows.

Comment threadhbase-client/src/test/java/org/apache/hadoop/hbase/client/TestSimpleRequestController.java Outdated
}

@Test
@SuppressWarnings("ArrayAsKeyOfSetOrMap")

@Apache9Apache9Jul 23, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is a test, let's just use TreeMap with Bytes.BYTES_COMPARATOR instead of HashMap to remove the warning?

@apurtellapurtellJul 25, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I thought I went back and undid my changes related to ArrayAsKeyOfSetOrMap while revising the patch but see that I left this one in place.

Comment threadhbase-client/src/test/java/org/apache/hadoop/hbase/client/TestSimpleRequestController.java Outdated
}

@Test
@SuppressWarnings("ArrayAsKeyOfSetOrMap")

@Apache9Apache9Jul 23, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto.

Comment threadhbase-client/src/test/java/org/apache/hadoop/hbase/client/TestSimpleRequestController.java
@apurtell
apurtell requested a review from Apache9July 25, 2022 23:54
@apurtell

apurtell commented Jul 25, 2022

Copy link
Copy Markdown
ContributorAuthor

Rebase. Address review feedback. More javadoc cleanups.

@Apache-HBase

Apache-HBase commented Jul 26, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 40sDocker mode activated.
-0 ⚠️yetus0m 4sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 21smaster passed
+1 💚compile0m 15smaster passed
+1 💚shadedjars3m 54sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 15smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 9sthe patch passed
+1 💚compile0m 15sthe patch passed
+1 💚javac0m 15sthe patch passed
+1 💚shadedjars3m 56spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 13sthe patch passed
_ Other Tests _
+1 💚unit1m 7shbase-client in the patch passed.
16m 1s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#4644
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 2567c5da8765 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e10c15d
Default JavaAdoptOpenJDK-1.8.0_282-b08
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/testReport/
Max. process+thread count161 (vs. ulimit of 30000)
modulesC: hbase-client U: hbase-client
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Jul 26, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 49sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 31smaster passed
+1 💚compile0m 17smaster passed
+1 💚shadedjars3m 53sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 14smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 31sthe patch passed
+1 💚compile0m 18sthe patch passed
+1 💚javac0m 18sthe patch passed
+1 💚shadedjars3m 58spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 15shbase-client generated 0 new + 44 unchanged - 11 fixed = 44 total (was 55)
_ Other Tests _
+1 💚unit1m 23shbase-client in the patch passed.
17m 12s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#4644
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 78536caeb06c 5.4.0-1043-aws #45~18.04.1-Ubuntu SMP Fri Apr 9 23:32:25 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e10c15d
Default JavaAdoptOpenJDK-11.0.10+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/testReport/
Max. process+thread count195 (vs. ulimit of 30000)
modulesC: hbase-client U: hbase-client
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Jul 26, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 19sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 1sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall2m 46smaster passed
+1 💚compile0m 47smaster passed
+1 💚checkstyle0m 18smaster passed
+1 💚spotless0m 54sbranch has no errors when running spotless:check.
+1 💚spotbugs0m 49smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 47sthe patch passed
+1 💚compile0m 42sthe patch passed
-0 ⚠️javac0m 42shbase-client generated 31 new + 5 unchanged - 118 fixed = 36 total (was 123)
+1 💚checkstyle0m 16shbase-client: The patch generated 0 new + 128 unchanged - 75 fixed = 128 total (was 203)
+1 💚whitespace0m 1sThe patch has no whitespace issues.
+1 💚hadoopcheck13m 7sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotless0m 50spatch has no errors when running spotless:check.
+1 💚spotbugs0m 47sthe patch passed
_ Other Tests _
+1 💚asflicense0m 10sThe patch does not generate ASF License warnings.
31m 14s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#4644
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
unameLinux 7c74a989c870 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e10c15d
Default JavaAdoptOpenJDK-1.8.0_282-b08
javachttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/artifact/yetus-general-check/output/diff-compile-javac-hbase-client.txt
Max. process+thread count64 (vs. ulimit of 30000)
modulesC: hbase-client U: hbase-client
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4644/2/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache9
Apache9 approved these changes Jul 26, 2022
View reviewed changes

@Apache9Apache9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Only a minor nits, can change or not, not a big deal.

+1, thanks @apurtell for the hard work!

Comment threadhbase-client/src/main/java/org/apache/hadoop/hbase/security/SaslUtil.java Outdated
/** Splitting fully qualified Kerberos name into parts */
public static String[] splitKerberosName(String fullName) {
return fullName.split("[/@]");
List<String> result = Splitter.onPattern("[/@]").splitToList(fullName);

@Apache9Apache9Jul 26, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return Splitter.onPattern("[/@]").splitToStream(fullName).toArray(String[]::new);

@apurtellapurtellJul 26, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

You indicated a preference for using a stream to do toArray on the hbase-backup review, will apply the pattern to all such places, no problem

@apurtell
apurtell merged commit 9573466 into apache:masterJul 26, 2022
@apurtell
apurtell deleted the HBASE-27203 branch July 26, 2022 22:08
@Apache9Apache9 mentioned this pull request Jul 27, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Apache9Apache9Apache9 approved these changes
@virajjasanivirajjasaniAwaiting requested review from virajjasani

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@apurtell@Apache-HBase@Apache9