> On 2012-02-03 15:18:30, Arvind Prabhakar wrote: > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/SinkRunner.java, > > line 25 > > <https://reviews.apache.org/r/3739/diff/1/?file=71899#file71899line25> > > > > Since now we have only a single Sink type, there does not seem to be > > any value in having a SinkRunner interface and a default implementation. > > Instead, I suggest removing the SinkRunner interface and making the > > SinkRunner as a concrete public class.
Thank you for your feedback sir. I thought so, but I wanted to check anyway. I'll try to put together patch updated as soon as possible. - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3739/#review4802 ----------------------------------------------------------- On 2012-02-02 22:10:16, Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3739/ > ----------------------------------------------------------- > > (Updated 2012-02-02 22:10:16) > > > Review request for Flume and Arvind Prabhakar. > > > Summary > ------- > > I've collapsed PollableSink into Sink interface and fixed all other classes. > I'm wondering whether it would make sense to also collapse PollableSinkRunner > into SinkRunner class? > > > This addresses bug FLUME-949. > https://issues.apache.org/jira/browse/FLUME-949 > > > Diffs > ----- > > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/PollableSink.java > 1239790 > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/Sink.java > 1239790 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/SinkRunner.java > 1239790 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java > 1239790 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/LoggerSink.java > 1239790 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/NullSink.java > 1239790 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/PollableSinkRunner.java > 1239790 > > /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/sink/RollingFileSink.java > 1239790 > > /branches/flume-728/flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java > 1239790 > > /branches/flume-728/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > 1239790 > > /branches/flume-728/flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestHDFSEventSink.java > 1239790 > > /branches/flume-728/flume-ng-sinks/flume-irc-sink/src/main/java/org/apache/flume/sink/irc/IRCSink.java > 1239790 > > Diff: https://reviews.apache.org/r/3739/diff > > > Testing > ------- > > mvn test passes correctly for entire flume-ng project > > > Thanks, > > Jarek > >
