Will Berkeley has posted comments on this change.

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


Patch Set 4:

(7 comments)

Did a significant rewrite to make it more similar to the DataSet Sink 
implementation. Sorry if this pushes it further from the finish line but I 
think it will make it better in the end. It'll get another rewrite when the 
KuduEventProducer API changes, too.

http://gerrit.cloudera.org:8080/#/c/4034/4/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 60:  * <tr><td>producer.schema.path</td>
> should be schemaPath
Done


Line 63:  *     <td>The location of the Avro schema file used to deserialize 
the Avro-encoded event bodies.
> Add: If not specified, the schema must be specified on a per-event basis.
Done


Line 73:   private static final String SCHEMA_HEADER = "schemaPath";
> Let's use the same property as the Kite sink for the event headers so that 
Done


Line 110:           String.format("No schema for event! Specify either property 
%s or event header %s",
> s/property/configuration property/
Done


Line 121:       payloadReader = new DataFileReader<>(new 
SeekableByteArrayInput(payload), reader);
> We should not treat each Flume Event as an Avro DataFile. We should be trea
Did a significant rewrite based on AvroParser.


Line 150:         if (!col.isNullable()) {
> I think this should be:
Done. Made a note to myself fix later.


Line 202:   private DatumReader<GenericRecord> openSchema(String schemaPath) {
> It would be nice to support both URL and literal. See how the DatasetSink i
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: 4
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