You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Arvind Prabhakar <ar...@apache.org> on 2012/02/13 03:32:29 UTC

Re: Review Request: customizable avro flume interchange - DefaultAvroSource/DefaultAvroSink added

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


Thanks for the patch Karthik. Can you please rebase the patch on the latest source and add some tests? That will help speed up the review as well. Also, some minor feedback below from a quick glance.


http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java
<https://reviews.apache.org/r/3359/#comment11068>

    No tabs please. Use spaces instead.



http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSourceProtocolSinkHandler.java
<https://reviews.apache.org/r/3359/#comment11069>

    Please use setter instead of field access.



http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSourceProtocolSinkHandler.java
<https://reviews.apache.org/r/3359/#comment11070>

    Please use getter for headers.


- Arvind


On 2012-01-19 20:44:49, Karthik K wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3359/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 20:44:49)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> AvroSource / AvroSink concrete classes has injectible eventHandlers (SourceEventHandler and SinkEventHandler), to allow for diffrerent avro protocols to be sent over flume NG.  ( not restricted to AvroSourceProtocol , as it stands today). 
> 
> Default implementation refers to AvroSourceProtocol still though.
> 
> 
> This addresses bug FLUME-918.
>     https://issues.apache.org/jira/browse/FLUME-918
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/event/SinkEventHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/event/SourceEventHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AbstractSink.java 1233531 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 1233531 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSourceProtocolSinkHandler.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java 1233531 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/source/AvroSourceProtocolImpl.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/source/TransactionManager.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3359/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Karthik
> 
>