HBASE-26542 Apply a `package` to test protobuf files by ndimiduk · Pull Request #3924 · apache/hbase

ndimiduk · 2021-12-06T20:18:29Z

This is needed in a couple places in order to test that traces over the IPC layer carry correct
span names, and it's good hygiene anyway.

ndimiduk · 2021-12-06T20:19:36Z

@busbey any idea if test-patch will pick up on the protobuf change and run unit tests in affected modules?

ndimiduk · 2021-12-06T20:35:05Z

To answer my own question, the answer appears to be, "sort of".

[ERROR] Failed to execute goal on project hbase-protocol-shaded: Could not resolve dependencies for project org.apache.hbase:hbase-protocol-shaded:jar:3.0.0-alpha-2-SNAPSHOT: Could not find artifact org.apache.hbase:hbase-annotations:jar:tests:3.0.0-alpha-2-SNAPSHOT in Nexus (http://repository.apache.org/snapshots)

ndimiduk · 2021-12-06T20:50:09Z

So there's a direct dependency, but test-patch runs mvn install from the module directory, and so maven cannot install the dependency?

[INFO] ---------------< org.apache.hbase:hbase-protocol-shaded >---------------
[INFO] Building Apache HBase - Shaded Protocol 3.0.0-alpha-2-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] [INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hbase-protocol-shaded ---
[INFO] org.apache.hbase:hbase-protocol-shaded:jar:3.0.0-alpha-2-SNAPSHOT
[INFO] +- org.apache.hbase.thirdparty:hbase-shaded-protobuf:jar:3.5.1:compile
[INFO] +- junit:junit:jar:4.13:test
[INFO] | \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] +- javax.annotation:javax.annotation-api:jar:1.2:compile
[INFO] +- org.apache.hbase:hbase-annotations:test-jar:tests:3.0.0-alpha-2-SNAPSHOT:test
[INFO] \- org.apache.yetus:audience-annotations:jar:0.5.0:compile

ndimiduk · 2021-12-06T21:07:25Z

Comparing the output here vs. other PRs I have up, this one seems to be missing section "maven install: master". Maybe this needs added to hbaseprotoc_rebuild in hbase-personality.sh.

ndimiduk · 2021-12-09T01:28:17Z

This is better. It didn't run any tests impacted by the changed protoc files, but it didn't fail either.

Apache9 · 2021-12-09T02:15:28Z

So which PR is effected?

In the past we have a lot of PRs which changes the protobuf files but I haven't seen any problems on the pre commit run...

ndimiduk · 2021-12-09T18:14:46Z

The primary change on this PR (HBASE-26542) is a change to the test protoc files. pre-commit did not handle the change properly and errored out (see the initial jenkins build). I pushed a second commit here, the same commit as is on #3927 / HBASE-26549, which tweaks the hbaseprotoc plugin shipped in our personality file, in a way that I thought might fix the initial build failure.

This PR/HBASE-26542 is need in order to test inclusion of protoc service package names in HBASE-26521 .

Apache9 · 2021-12-13T15:26:36Z

What I mean is that, we used to change the protobuf files a lot in the past and IIRC we haven't seen any problems on the pre commit build.

So I want to know, in which PR you see a broken pre commit result because of the problem the described here? It seems that we do not run 'mvn install' first then the build will be broken?

Thanks.

ndimiduk · 2021-12-13T16:55:55Z

What I mean is that, we used to change the protobuf files a lot in the past and IIRC we haven't seen any problems on the pre commit build.
So I want to know, in which PR you see a broken pre commit result because of the problem the described here? It seems that we do not run 'mvn install' first then the build will be broken?

@Apache9 This same PR is where the problem was seen. See the results of the first CI checks for full details. The exact error I extracted from the build log and posted in this comment,

[ERROR] Failed to execute goal on project hbase-protocol-shaded: Could not resolve dependencies for project org.apache.hbase:hbase-protocol-shaded:jar:3.0.0-alpha-2-SNAPSHOT: Could not find artifact org.apache.hbase:hbase-annotations:jar:tests:3.0.0-alpha-2-SNAPSHOT in Nexus (http://repository.apache.org/snapshots)

One difference between this and previous changes to protobuf files may be that this PR is changing the test protobuf files, not the main ones.

Apache9 · 2021-12-14T00:19:03Z

OK, I guess the problem is that if we only touch the proto without changing any java files, we will not trigger a 'mvn install' first which could lead to the problem...

Let me take a look at the PR here, thanks @ndimiduk for explaining.

ndimiduk · 2021-12-14T17:25:04Z

OK, I guess the problem is that if we only touch the proto without changing any java files, we will not trigger a 'mvn install' first which could lead to the problem...

Oh, of course! Yes, that makes perfect sense. Thanks for taking a look.

Apache-HBase · 2021-12-14T17:34:07Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 24s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+1 💚	mvninstall	3m 56s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 51s	the patch passed
		_ Other Tests _
+1 💚	unit	0m 46s	hbase-protocol-shaded in the patch passed.
		10m 14s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#3924
Optional Tests	unit
uname	Linux 9802e736f8e0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `13f3b17`
Test Results	https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/testReport/
Max. process+thread count	78 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded U: hbase-protocol-shaded
Console output	https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2021-12-14T17:37:22Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 31s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+0 🆗	prototool	0m 0s	prototool was not available.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ master Compile Tests _
+1 💚	mvninstall	4m 41s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	4m 18s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hbaseprotoc	1m 4s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 13s	The patch does not generate ASF License warnings.
		13m 24s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#3924
Optional Tests	dupname asflicense cc hbaseprotoc prototool
uname	Linux f0c2633fde01 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `13f3b17`
Max. process+thread count	65 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded U: hbase-protocol-shaded
Console output	https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2021-12-14T17:45:34Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	7m 10s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+1 💚	mvninstall	5m 52s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	5m 34s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 12s	hbase-protocol-shaded in the patch passed.
		21m 6s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#3924
Optional Tests	unit
uname	Linux 6ddbd12fc09c 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `13f3b17`
Test Results	https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/testReport/
Max. process+thread count	74 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded U: hbase-protocol-shaded
Console output	https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3924/4/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

This is needed in a couple places in order to test that traces over the IPC layer carry correct span names, and it's good hygiene anyway. Signed-off-by: Duo Zhang <zhangduo@apache.org>

ndimiduk requested review from Apache9, anoopsjohn, apurtell, busbey and joshelser December 6, 2021 20:20

This comment has been minimized.
Sign in to view

ndimiduk mentioned this pull request Dec 9, 2021
HBASE-26549 hbaseprotoc plugin should initialize maven #3927
Merged

ndimiduk force-pushed the 26542-test-protobuf-package branch from 0a7f96f to 2867035Compare December 9, 2021 00:32

This comment has been minimized.
Sign in to view

ndimiduk force-pushed the 26542-test-protobuf-package branch from 2867035 to 031b63fCompare December 14, 2021 17:23

Apache9 approved these changes Dec 15, 2021
View reviewed changes

HBASE-26542 Apply a package to test protobuf files
6865cbc
This is needed in a couple places in order to test that traces over the IPC layer carry correct span names, and it's good hygiene anyway. Signed-off-by: Duo Zhang <zhangduo@apache.org>

ndimiduk force-pushed the 26542-test-protobuf-package branch from 031b63f to 6865cbcCompare December 15, 2021 23:52

ndimiduk merged commit c93e457 into apache:masterDec 15, 2021

ndimiduk deleted the 26542-test-protobuf-package branch December 15, 2021 23:52

Uh oh!

Conversation

ndimiduk commented Dec 6, 2021

Uh oh!

ndimiduk commented Dec 6, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ndimiduk commented Dec 6, 2021

Uh oh!

ndimiduk commented Dec 6, 2021

Uh oh!

ndimiduk commented Dec 6, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ndimiduk commented Dec 9, 2021

Uh oh!

Apache9 commented Dec 9, 2021

Uh oh!

ndimiduk commented Dec 9, 2021

Uh oh!

Apache9 commented Dec 13, 2021

Uh oh!

ndimiduk commented Dec 13, 2021

Uh oh!

Apache9 commented Dec 14, 2021

Uh oh!

ndimiduk commented Dec 14, 2021

Uh oh!

Apache-HBase commented Dec 14, 2021

Uh oh!

Apache-HBase commented Dec 14, 2021

Uh oh!

Apache-HBase commented Dec 14, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants