Mike Percy has posted comments on this change. Change subject: Add AvroKuduEventProducer to Kudu-Flume integration ......................................................................
Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/4034/5/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 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? http://gerrit.cloudera.org:8080/#/c/4034/5/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 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", schemaURI)); 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 @@ <properties> + <avro.version>1.8.1</avro.version> <flume.version>1.6.0</flume.version> <hadoop.version>2.7.0</hadoop.version> </properties> <build> @@ -67,6 +68,19 @@ </execution> </executions> </plugin> + <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> </plugins> </build> @@ -92,6 +106,13 @@ </dependency> <dependency> + <groupId>org.apache.avro</groupId> + <artifactId>avro</artifactId> + <version>${avro.version}</version> + <scope>provided</scope> + </dependency> + + <dependency> <groupId>org.apache.hadoop</groupId> <artifactId>hadoop-client</artifactId> <version>${hadoop.version}</version> 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
