> On Oct. 7, 2016, 2:25 p.m., Mike Percy wrote: > > flume-ng-sinks/flume-ng-kafka-sink/src/main/java/org/apache/flume/sink/kafka/KafkaSink.java, > > line 214 > > <https://reviews.apache.org/r/52598/diff/1/?file=1524421#file1524421line214> > > > > nit: missing space after comma. Did you run checkstyle?
Yes, not sure why it wasn't picked up. > On Oct. 7, 2016, 2:25 p.m., Mike Percy wrote: > > flume-ng-sinks/flume-ng-kafka-sink/src/main/java/org/apache/flume/sink/kafka/KafkaSink.java, > > line 221 > > <https://reviews.apache.org/r/52598/diff/1/?file=1524421#file1524421line221> > > > > Do you really need to catch Exception? If so, then why bother catching > > all the rest as well? // N.B. The producer.send() method throws all sorts of RuntimeExceptions // Catching Exception here to wrap them neatly in an EventDeliveryException // which is what our consumers will expect > On Oct. 7, 2016, 2:25 p.m., Mike Percy wrote: > > flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/util/KafkaConsumer.java, > > line 55 > > <https://reviews.apache.org/r/52598/diff/1/?file=1524424#file1524424line55> > > > > hmm, does this change our user contract or not really? It's only in test, but actually it was a mistake. Reverted. > On Oct. 7, 2016, 2:25 p.m., Mike Percy wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1268 > > <https://reviews.apache.org/r/52598/diff/1/?file=1524420#file1524420line1268> > > > > I am confused about what this means in the context of the source. > > > > 2. 1. Did you really mean to put this in the docs for the source? > > 2. s/channel/source/ on the first line here ? I knew I did that somewhere but couldn't find it. It's entirely wrong - removed. > On Oct. 7, 2016, 2:25 p.m., Mike Percy wrote: > > flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/TestKafkaSink.java, > > line 391 > > <https://reviews.apache.org/r/52598/diff/1/?file=1524423#file1524423line391> > > > > Is this just copy / paste from the other one? Can we find a way to make > > them share code? My Maven fu is not quite there - I've refactored the method out, but need to get them into a shared module that is accessible only for test. - Tristan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52598/#review151795 ----------------------------------------------------------- On Oct. 6, 2016, 12:13 p.m., Tristan Stevens wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52598/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2016, 12:13 p.m.) > > > Review request for Flume and Grant Henke. > > > Repository: flume-git > > > Description > ------- > > This feature is useful for anyone who needs greater control of which > partitions are being written to - normally in a situation where multiple > Flume agents are being deployed in order to horizontally scale, or > alternatively if there is a scenario where there is a skew in data that might > lead to one or more partitions hotspotting. > We also have the ability to specify custom partitions on to the Kafka > Producer itself using the kafka.* configuration properties. > > The Kafka Producer provides the ability to set the partition ID using the > following constructor > (https://kafka.apache.org/090/javadoc/org/apache/kafka/clients/producer/ProducerRecord.html#ProducerRecord%28java.lang.String,%20java.lang.Integer,%20K,%20V%29 > ), this is just a matter of providing the option to use this constructor. > > This is specified in one of two ways: either via the staticPartition > configuration property, which means that every message goes to the specified > partition, or via the partitionHeader configuration property, which directs > the implementation to retrieve the partitionId from one of the event headers. > > > Diffs > ----- > > > flume-ng-channels/flume-kafka-channel/src/main/java/org/apache/flume/channel/kafka/KafkaChannel.java > 66b553a > > flume-ng-channels/flume-kafka-channel/src/main/java/org/apache/flume/channel/kafka/KafkaChannelConfiguration.java > 3ab807b > > flume-ng-channels/flume-kafka-channel/src/test/java/org/apache/flume/channel/kafka/TestKafkaChannel.java > 57c0b28 > flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38 > > flume-ng-sinks/flume-ng-kafka-sink/src/main/java/org/apache/flume/sink/kafka/KafkaSink.java > 89bdd84 > > flume-ng-sinks/flume-ng-kafka-sink/src/main/java/org/apache/flume/sink/kafka/KafkaSinkConstants.java > 1bf380c > > flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/TestKafkaSink.java > 76eca37 > > flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/util/KafkaConsumer.java > d5dfbd6 > > Diff: https://reviews.apache.org/r/52598/diff/ > > > Testing > ------- > > Unit testing done for both Kafka Channel and Kafka Sink. > > > Thanks, > > Tristan Stevens > >
