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




flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java 
(line 211)
<https://reviews.apache.org/r/50232/#comment208797>

    nit (and I know that it was the same before): as a getter for a boolean 
value it'd be better to start with `is` (or if it sounds better `get`) prefix.



flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java 
(line 250)
<https://reviews.apache.org/r/50232/#comment208796>

    My concern with this change is that if a `ChannelException` is thrown then 
it'll end up in the outer `catch (Throwable t)` (line 267) block and it will be 
considered as fatal error which isn't the case currently.
    
    As far as I understood the Jira issue the main goal is to be able to make 
distinction between `ChannelFullException` and other `ChannelException`s and in 
the latter case log the original exception but the current behaviour shouldn't 
be changed.


- Denes Arvay


On July 20, 2016, 1:25 p.m., Balázs Donát Bessenyei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50232/
> -----------------------------------------------------------
> 
> (Updated July 20, 2016, 1:25 p.m.)
> 
> 
> Review request for Flume, Denes Arvay and Attila Simon.
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Spooldir assumes that any ChannelException means that the channel is full and 
> it does not log the exception message.
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java 
> 1cce137 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java 
> d88cc1d 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java
>  82c5351 
> 
> Diff: https://reviews.apache.org/r/50232/diff/
> 
> 
> Testing
> -------
> 
> [INFO] Flume NG Core ...................................... SUCCESS [08:04 
> min]
> 
> 
> Thanks,
> 
> Balázs Donát Bessenyei
> 
>

Reply via email to