Will Berkeley has posted comments on this change.

Change subject: Add AvroKuduEventProducer to Kudu-Flume integration
......................................................................


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/4034/6/java/kudu-flume-sink/pom.xml
File java/kudu-flume-sink/pom.xml:

Line 82:                 
<sourceDirectory>${project.basedir}/src/main/avro/</sourceDirectory>
> this should not be in src/main it should be in src/test since this is only 
Done


Line 85:               </execution>
> nit: this line should be un-indented by 2 spaces
Done


http://gerrit.cloudera.org:8080/#/c/4034/6/java/kudu-flume-sink/src/test/avro/testAvroEventProducer.avsc
File java/kudu-flume-sink/src/test/avro/testAvroEventProducer.avsc:

Line 5:     {"name": "key", "type": "int"},
> I don't think we should name these fields with names that match types exact
Done


http://gerrit.cloudera.org:8080/#/c/4034/6/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduEventProducerTest.java
File 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduEventProducerTest.java:

Line 136:       assertTrue("incorrect status for empty channel", status == 
Sink.Status.BACKOFF);
> assertEquals("incorrect status for empty channel", status, Sink.Status.BACK
Done


Line 138:       assertTrue("incorrect status for non-empty channel", status != 
Sink.Status.BACKOFF);
> assertEquals("incorrect status for non-empty channel", status, Sink.Status.
Done


Line 148:     ArrayList<ColumnSchema> columns = new ArrayList<>(3);
> nit: initial capacity should be 5
Done


Line 179:       record.setLong$(2L * i);
> See my comments about this $ syntax in the .avsc file -- let's just rename 
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wdberke...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to