-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4047/#review5361
-----------------------------------------------------------


Thanks for the patch Mike. The changes look good. Some suggestions follow:

1. Please use 80 char limit on line lengths and wrap the lines where necessary. 
Also the general formatting/style convention is that we should use spaces only, 
two-spaces per tab and no trailing whitespaces in the sources. Any trailing 
whitespaces are highlighted with red in the reviewboard.

2. An event builder in the  org.apache.flume package that can be used to create 
events with a single string or object will be a great complementing addition.

3. Now that the simple to use client API is there, we should refactor the 
AvroCLIClient and Log4jAppender to use it instead of directly going via the 
low-level API.


flume-ng-sdk/src/main/java/org/apache/flume/api/client/AvroClientBuilder.java
<https://reviews.apache.org/r/4047/#comment11668>

    Adding a check on batch size should be good too.


- Arvind


On 2012-02-24 22:47:13, Mike Percy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4047/
> -----------------------------------------------------------
> 
> (Updated 2012-02-24 22:47:13)
> 
> 
> 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-core/pom.xml d753fa1 
>   flume-ng-core/src/main/avro/flume.avdl 40da3ef 
>   flume-ng-core/src/main/java/org/apache/flume/Event.java a017705 
>   flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 5440631 
>   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/api/client/AvroClient.java 
> PRE-CREATION 
>   
> flume-ng-sdk/src/main/java/org/apache/flume/api/client/AvroClientBuilder.java 
> PRE-CREATION 
>   
> flume-ng-sdk/src/main/java/org/apache/flume/api/client/DefaultAvroClient.java 
> PRE-CREATION 
>   pom.xml d785762 
> 
> Diff: https://reviews.apache.org/r/4047/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mike
> 
>

Reply via email to