----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4047/#review5841 -----------------------------------------------------------
Thanks for the patch Mike. I have one suggestion though: Please modify the default appendBatch() method in the NettyAvroRpcClient() implementation so that it does not throw an IllegalArgumentException if the passed in collection contains more events than the allowed batch size. Instead it should loop over the given collection in chunks of batch sizes until all of it has been drained to the downstream end point. Reason for this ask is that the builder can potentially use the built in default value of batch size that is not visible to the client directly. - Arvind On 2012-03-09 23:44:00, Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4047/ > ----------------------------------------------------------- > > (Updated 2012-03-09 23:44:00) > > > Review request for Flume. > > > Summary > ------- > > Seeking early feedback on some additional APIs to make integrating with Flume > 1.x easier. > > Added the following APIs: > - AvroClient: Friendly Java interface around the Avro API > - AvroClientBuilder: Builder class to allow easy extension of AvroClient > capabilities in the future (i.e. SSL) > - DefaultAvroClient: Implementation of the AvroClient interface > > Created this stuff in a flume-ng-sdk Maven submodule and moved the Event > interface to that submodule. flume-ng-core depends on flume-ng-sdk. > > I also modified AvroSink to use the AvroClient API instead of the bare > AvroSourceProtocol API directly. > > > This addresses bug FLUME-989. > https://issues.apache.org/jira/browse/FLUME-989 > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/pom.xml 9dd31bd > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java > 97f2b9e > flume-ng-core/pom.xml fe6ce0b > flume-ng-core/src/main/avro/flume.avdl 40da3ef > flume-ng-core/src/main/java/org/apache/flume/Event.java 5278fc0 > flume-ng-core/src/main/java/org/apache/flume/EventDeliveryException.java > 1413223 > flume-ng-core/src/main/java/org/apache/flume/FlumeException.java eab5b3d > flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java > 195ba79 > flume-ng-core/src/main/java/org/apache/flume/event/EventBuilder.java > 5d8c3b3 > flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java e0c3b45 > flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 7386d06 > flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java a3f6640 > flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 467785f > flume-ng-core/src/test/java/org/apache/flume/util/TestEventBuilder.java > 7930607 > flume-ng-sdk/pom.xml PRE-CREATION > flume-ng-sdk/src/main/avro/flume.avdl PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/Event.java PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/EventDeliveryException.java > PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/FlumeException.java > PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java > PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java > PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/event/EventBuilder.java > PRE-CREATION > flume-ng-sdk/src/main/java/org/apache/flume/event/SimpleEvent.java > PRE-CREATION > flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java > PRE-CREATION > flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java > PRE-CREATION > flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java > PRE-CREATION > flume-ng-sdk/src/test/java/org/apache/flume/event/TestEventBuilder.java > PRE-CREATION > pom.xml d785762 > > Diff: https://reviews.apache.org/r/4047/diff > > > Testing > ------- > > > Thanks, > > Mike > >
