Mike Percy has posted comments on this change.

Change subject: Add AvroKuduEventProducer to Kudu-Flume integration

Patch Set 6:


Found a few more test nits. I think this is nearly ready to commit now.

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

Line 82:                 
this should not be in src/main it should be in src/test since this is only used 
in the test module.

In fact, that is the default for the generate-test-sources goal and you should 
be able to leave off the configuration section completely from here. I just 
tested this and it's true, you can remove this section.

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

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 exactly. 
It makes the generated Avro method names a bit weird, and people reading the 
tests might be a little confused.

Instread of "long", how about 
"longField" or something? Instead of "null", how about "nullableStringField"?


* key (this is alright, or intKey)
* longField
* doubleField
* nullableStringField
* stringField


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

Line 138:       assertTrue("incorrect status for non-empty channel", status != 
assertEquals("incorrect status for non-empty channel", status, 

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

Line 179:       record.setLong$(2L * i);
See my comments about this $ syntax in the .avsc file -- let's just rename 
these fields not to conflict with type names.

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