Alexey Serbin has posted comments on this change. Change subject: KUDU-2033 (part 2). Add test for Java client failover support. ......................................................................
Patch Set 1: (4 comments) Overall looks good, just a couple of nits. http://gerrit.cloudera.org:8080/#/c/7722/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java: PS1, Line 58: KuduTable table; Move this closer to the point of the usage. PS1, Line 60: + "-" + System.currentTimeMillis(); Is it crucial to have that suffix? I would expect it's not, so consider dropping this and have the name of the table as a constant (may be even a static constant). PS1, Line 61: builder nit: as the builder is not needed in the code below, consider not creating a variable for it: createTable(tableName, basicSchema, getBasicCreateTableOptions()); Line 85: waitUntilRowCount(table, 2*TOTAL_ROWS_TO_INSERT, DEFAULT_SLEEP); Consider dropping the table in the very end, making sure it can be successfully deleted. -- To view, visit http://gerrit.cloudera.org:8080/7722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I228e08429d952f0f5b2657e0b8481366c5c572a4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Edward Fancher <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-HasComments: Yes
