Mike Percy has posted comments on this change.

Change subject: Add AvroKuduEventProducer to Kudu-Flume integration

Patch Set 5:



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

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


Line 61:   private static final String schemaLiteral =
you can use Guava and do:

  schemaLiteral = Files.toString(new File(schemaPath), Charsets.UTF_8);

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

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

  import static KuduSinkConfigurationConstants.PRODUCER_PREFIX;
  import static AvroKuduEventProducer.SCHEMA_PROP;

    : new Context(ImmutableMap.of(PRODUCER_PREFIX + SCHEMA_PROP, schemaURI));

Line 175:       GenericRecord record = new GenericData.Record(schema);
Would you mind using SpecificRecord instead of GenericRecord when writing the 
events? That way we can be sure writing via SpecificRecord works fine when 
decoding with GenericRecord with Kudu.

You can do it like this:

1. Move the avro schema to src/test/avro/
2. Make the following changes to the pom:

@@ -23,6 +23,7 @@
+    <avro.version>1.8.1</avro.version>

@@ -67,6 +68,19 @@
+      <plugin>
+        <groupId>org.apache.avro</groupId>
+        <artifactId>avro-maven-plugin</artifactId>
+        <version>${avro.version}</version>
+        <executions>
+          <execution>
+            <phase>generate-test-sources</phase>
+            <goals>
+              <goal>schema</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>

@@ -92,6 +106,13 @@

+      <groupId>org.apache.avro</groupId>
+      <artifactId>avro</artifactId>
+      <version>${avro.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Then Maven should generate the AvroKuduEventProducerTestRecord class for you. I 
sort-of tested the codegen and it seems to work.

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