Will Berkeley has posted comments on this change.

Change subject: Add AvroKuduEventProducer to Kudu-Flume integration

Patch Set 1:


File java/kudu-flume-sink/pom.xml:

Line 74:       <artifactId>hadoop-client</artifactId>
> is this typical for Flume sinks? does this get shaded? Or would we expect i
To be honest, I wasn't sure about this, so I copied off of the map/reduce 
integration's pom. I think we do expect it to be on the classpath. The 
dependency is only needed because I'd like users to be able to load the schema 
from HDFS, not just the local filesystem.


Line 51:  * <tr><td>producer.upsert</td><td>false</td><td>No</td><td>Whether to 
upsert or insert rows into Kudu.</td></tr>
> - can you wrap these to multiple lines for better readability?

PS1, Line 52: producer.schema
> this sounds like it would be a schema rather than a path. perhaps schema_pa

Line 80:       FileSystem fs = FileSystem.get(path.toUri(), new 
> I think this should be path.getFileSystem(new Configuration())

Line 93:   public void initialize(Event event, KuduTable table) {
> unrelated suggestion: this is a very strange method name to me, since it ge
Yup, it's weird.

Line 126:               String.format("Column %s is not nullable but was 
decoded as null", name));
> does this happen if you have a missing column? if so, I think a fallback to

Line 133:             row.addBoolean(name, (boolean) value);
> all of these could generate ClassCastExceptions, right? Do we want to do an
We could improve the API, but it would mean changing KuduSink and the 
KuduEventProducer interface, and therefore the other event producers. I don't 
think the user-facing configuration would need to change. Assuming I'm right, 
separate patch or broaden the scope of this one?

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

Reply via email to