Will Berkeley has posted comments on this change.

Change subject: Add RegexpKuduOperationsProducer class
......................................................................


Patch Set 4:

(7 comments)

> Cool idea!

Actually, I was talking to Jeremy Beard and mentioned that you had suggested an 
OperationsProducer that parsed Avro. He said he'd rather have one that just 
used regexp so it'd be a little quicker to get moving!

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.
Done


Line 209:   private void CoerceAndSet(PartialRow row, String rawVal, String 
colName, Type type)
> Needs doc comment.
Done


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 typ
Done


Line 70:       new 
CreateTableOptions().addHashPartitions(ImmutableList.of("key"), 
3).setNumReplicas(1);
> style nit: indent 4 spaces for continuation
Done


Line 136:     //
> remove
Done


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 :)
Tacos have successfully infiltrated the Kudu codebase ;D


Line 173:       eventCount * perEventRowCount,
> indent 4 spaces for continuation
Fixed here and elsewhere.


-- 
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-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to