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


Thanks for incorporating the review comments Hari. A couple of 
questions/comments:


flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java
<https://reviews.apache.org/r/5166/#comment17359>

    If rounding gets disabled after reconfiguration, this will continue to 
force rounding regardless. To fix it you could either test needRounding before 
calling this, or pass the needRounding into the roundDown() method.



flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java
<https://reviews.apache.org/r/5166/#comment17360>

    nit: the util methods have precondition checks to assert if the passed in 
values are valid or not. In case of incorrect configuration, it will result in 
exception on every bucket path calculation.
    
    Instead, if you could do upfront validation here, then you can disable the 
rounding with a warning message. 


- Arvind


On 2012-05-19 10:03:37, Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5166/
> -----------------------------------------------------------
> 
> (Updated 2012-05-19 10:03:37)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> Created a utility that does the rounding. 
> 
> 
> This addresses bug FLUME-1213.
>     https://issues.apache.org/jira/browse/FLUME-1213
> 
> 
> Diffs
> -----
> 
>   
> flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java 
> da80545 
>   
> flume-ng-core/src/main/java/org/apache/flume/tools/TimestampRoundDownUtil.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/formatter/output/TestBucketPath.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/tools/TestTimestampRoundDownUtil.java
>  PRE-CREATION 
>   
> flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java
>  6d26f47 
> 
> Diff: https://reviews.apache.org/r/5166/diff
> 
> 
> Testing
> -------
> 
> Added unit tests.
> 
> 
> Thanks,
> 
> Hari
> 
>

Reply via email to