Hao Hao 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) 
-> {
> Oh, I missed that usage. In that case, please move this initialization back
Done


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() {
             :   }
> Right, I missed that there's another constructor. My bad.
Ack


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 RuleChai
Yeah, I think it is not needed here when paired with 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.
Same here.


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
Ah, sorry that actually it is not working... the test exits in the middle as 
what you understand. Updated it be another test.



--
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: Thu, 06 Feb 2020 05:43:04 +0000
Gerrit-HasComments: Yes

Reply via email to