Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14329 )
Change subject: [java] KUDU-2971: process communicates via protobuf-based protocol ...................................................................... Patch Set 16: (5 comments) http://gerrit.cloudera.org:8080/#/c/14329/15/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java: http://gerrit.cloudera.org:8080/#/c/14329/15/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java@51 PS15, Line 51: private static Function<Throwable, Object> ERROR_HANDLER = (t) -> { > As we have constructor assign errorHandler again(SubprocessExecutor(Functio Oh, I missed that usage. In that case, please move this initialization back to the constructor, remove the static keyword, and mark it as final. It'll just be a regular constructor-initialized class field then. http://gerrit.cloudera.org:8080/#/c/14329/15/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java@59 PS15, Line 59: SubprocessExecutor() { : } > This is needed for non-test case in SubprocessExecutor. Right, I missed that there's another constructor. My bad. http://gerrit.cloudera.org:8080/#/c/14329/16/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestEchoSubprocess.java File java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestEchoSubprocess.java: http://gerrit.cloudera.org:8080/#/c/14329/16/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestEchoSubprocess.java@47 PS16, Line 47: public RetryRule retryRule = new RetryRule(); Does this still need a @Rule annotation? Or not when paired with a RuleChain? http://gerrit.cloudera.org:8080/#/c/14329/16/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageIO.java File java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageIO.java: http://gerrit.cloudera.org:8080/#/c/14329/16/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageIO.java@45 PS16, Line 45: public RetryRule retryRule = new RetryRule(); Same question. http://gerrit.cloudera.org:8080/#/c/14329/16/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageIO.java@116 PS16, Line 116: thrown.expect(IOException.class); : thrown.expectMessage("exceeds maximum message size"); : messageIO.readBytes(); This works mid-test? I thought the call that throws an exception would have to be the last thing in the test. TIL. -- To view, visit http://gerrit.cloudera.org:8080/14329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982 Gerrit-Change-Number: 14329 Gerrit-PatchSet: 16 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 05 Feb 2020 22:23:47 +0000 Gerrit-HasComments: Yes
