----------------------------------------------------------- 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 > >
