----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4175/#review5698 -----------------------------------------------------------
Hi Juhani, sorry for the delay in getting back to you on this. I took a look at the added docs and it looks great. I think this type of documentation really helps to clarify how the system is supposed to act. I added a few suggestions below. flume-ng-core/src/main/java/org/apache/flume/Sink.java <https://reviews.apache.org/r/4175/#comment12403> 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. flume-ng-core/src/main/java/org/apache/flume/Sink.java <https://reviews.apache.org/r/4175/#comment12408> Maybe we should say: @throws EventDeliveryException if there is a failure delivering any Event to the next-hop destination. (final destination is not guaranteed) flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java <https://reviews.apache.org/r/4175/#comment12404> Missing a component of the package: org.apache.flume.sink.SinkGroup flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java <https://reviews.apache.org/r/4175/#comment12405> 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. flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java <https://reviews.apache.org/r/4175/#comment12407> 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 flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java <https://reviews.apache.org/r/4175/#comment12406> may want to remove this whitespace - Mike 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 > >
