> On 2012-03-08 00:32:59, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/Sink.java, line 60 > > <https://reviews.apache.org/r/4175/diff/1/?file=88130#file88130line60> > > > > I think this part is a little vague. How about this? > > > > @return Returns READY if 1 or more events were successfully delivered, > > or BACKOFF if no events could be retrieved from the channel feeding this > > sink.
sounds good > On 2012-03-08 00:32:59, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/Sink.java, line 62 > > <https://reviews.apache.org/r/4175/diff/1/?file=88130#file88130line62> > > > > Maybe we should say: > > > > @throws EventDeliveryException if there is a failure delivering any > > Event to the next-hop destination. > > > > (final destination is not guaranteed) yeah, I see how that could be misunderstood, I had meant it as final iff the sink was something like a file sink with no further hops. Taking that out > On 2012-03-08 00:32:59, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java, line 31 > > <https://reviews.apache.org/r/4175/diff/1/?file=88131#file88131line31> > > > > Missing a component of the package: org.apache.flume.sink.SinkGroup oops. We may want to review which packages belong in org.apache.flume, I think some of them don't belong there but in a sub-package. That's an issue for another jira though > On 2012-03-08 00:32:59, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java, line 37 > > <https://reviews.apache.org/r/4175/diff/1/?file=88131#file88131line37> > > > > That is an improvement but I think it kind of refers to the failover > > sink processor's delivery policy (w.r.t. throwing the exception). Maybe > > something like this would be more general? > > > > The processor is expected to call {@linkplain Sink#process()} on > > whatever sink(s) appropriate, handling failures as appropriate and throwing > > {@link EventDeliveryException} when there is a failure to deliver any > > events according to the delivery policy defined by the sink processor > > implementation. See specific implementations of this interface for delivery > > behavior and policies. > > Much better, thanks > On 2012-03-08 00:32:59, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java, line 41 > > <https://reviews.apache.org/r/4175/diff/1/?file=88131#file88131line41> > > > > These old constants should be corrected. How about: > > > > @return Returns {@code READY} if events were successfully consumed, or > > {@code BACKOFF} if no events were available in the channel to consume. > > > > @throws EventDeliveryException if any failure to deliver events occurred Done. I changed the @throws a bit to refer to individual processor behaviors - Juhani ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4175/#review5698 ----------------------------------------------------------- On 2012-03-05 09:30:21, Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4175/ > ----------------------------------------------------------- > > (Updated 2012-03-05 09:30:21) > > > Review request for Flume. > > > Summary > ------- > > An initial pass at documenting the interfaces. > Let me know if I missed anything relevant, or if you feel that this does not > correctly represent our expected behaviors. > > > This addresses bug FLUME-1019. > https://issues.apache.org/jira/browse/FLUME-1019 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/Sink.java 3abeeb6 > flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java 11651ad > flume-ng-core/src/main/java/org/apache/flume/sink/SinkGroup.java 2e80a56 > flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java > 10f9f4e > > Diff: https://reviews.apache.org/r/4175/diff > > > Testing > ------- > > No changes have been made to code > > > Thanks, > > Juhani > >
