Mike Percy has posted comments on this change. Change subject: Add RegexpKuduOperationsProducer class ......................................................................
Patch Set 4: Code-Review+2 (7 comments) Cool idea! http://gerrit.cloudera.org:8080/#/c/3883/4/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java: Line 93: */ Needs a very simple example here somewhere, say with 2 fields. Also, note that this relies on JDK7 "named-capturing groups" which are documented in {@link Pattern}. Add: @see Pattern Line 209: private void CoerceAndSet(PartialRow row, String rawVal, String colName, Type type) Needs doc comment. Also, prefer the following arg order: CoerceAndSet(String rawVal, String colName, Type type, PartialRow row) Since row is an in-out param and the rest are in params http://gerrit.cloudera.org:8080/#/c/3883/4/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java File java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java: Line 53: "(?<key>\\d+),(?<byte>\\d+),(?<short>\\d+),(?<int>\\d+)," + As in the other patch, I think naming the fields something other than a type name might make it a little clearer. Just byteField or something would help understandability, I think Line 70: new CreateTableOptions().addHashPartitions(ImmutableList.of("key"), 3).setNumReplicas(1); style nit: indent 4 spaces for continuation Line 136: // remove Line 152: String mismatchInInt = "|1,2,taco,4,5,x,y,true,1.0.2.0,999|"; https://media.giphy.com/media/dLwB8eG7wwUDe/giphy.gif :) Line 173: eventCount * perEventRowCount, indent 4 spaces for continuation -- To view, visit http://gerrit.cloudera.org:8080/3883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibd64542095f0064a21635ec76c46d6a1be98b7ea Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Ara Ebrahimi <ara.ebrah...@argyledata.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-HasComments: Yes
