Will Berkeley has posted comments on this change.

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


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4034/5/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java
File 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java:

Line 57:  * <p>
> nit: i don't these extra <p>s do anything. Do they?
Done


Line 259:   private Schema schema(Event event) throws FlumeException {
> mind naming this getSchema() for consistency with Java style?
Done


http://gerrit.cloudera.org:8080/#/c/4034/5/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 61:   private static final String schemaLiteral =
> you can use Guava and do:
Done


Line 111:   private void testEvents(int eventCount, String whereIsSchema)
> style nit: let's create a SchemaLocation enum for whereIsSchema instead of 
Done


Line 118:         : new Context(ImmutableMap.of("producer.schemaPath", 
schemaURI));
> style nit: Use constants for config values:
Done


Line 175:       GenericRecord record = new GenericData.Record(schema);
> Would you mind using SpecificRecord instead of GenericRecord when writing t
I think I did this right.


-- 
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: 5
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