Will Berkeley has posted comments on this change. Change subject: Add AvroKuduEventProducer to Kudu-Flume integration ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/4034/1/java/kudu-flume-sink/pom.xml File java/kudu-flume-sink/pom.xml: Line 74: <artifactId>hadoop-client</artifactId> > Yeah, I would also like to see this dep shaded. Done http://gerrit.cloudera.org:8080/#/c/4034/1/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: PS1, Line 52: producer.schema > Flume sinks typically use camelCaps like schemaPath for configuration optio Done Line 89: public void configure(ComponentConfiguration conf) { > Hrm. You sure you had to override this? I had to. But now I notice that this method, which is required because KuduEventProducer implements ConfigurableComponent, is empty-bodied for every KuduEventProducer implementation. Maybe we should take it out? Or implement it for real for the KuduEventProducer implementations? Something for the follow-up either way. Line 93: public void initialize(Event event, KuduTable table) { > I don't think there are many users yet and I'd be OK with changing it pre-1 Will improve API and names in a follow-up. Line 133: row.addBoolean(name, (boolean) value); > If we use a single schema for the sink then we could probably do the valida Will do it like this: can optionally specify a global schema, validated up-front against the table (validation will have to wait for the reorg in follow-up patch), or a schema in the event header, which will override any global schema. http://gerrit.cloudera.org:8080/#/c/4034/1/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 149: parameters.put(KuduSinkConfigurationConstants.PRODUCER, "org.apache.kudu.flume.sink.AvroKuduEventProducer"); > nit: long line 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: 1 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
