----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5166/#review7987 -----------------------------------------------------------
Thanks for the patch Hari. Some feedback follows. flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java <https://reviews.apache.org/r/5166/#comment17351> This timestamp will be 0 valued if rounding is disabled. A few test cases to make sure that the BucketPath is resolving the paths correctly would be a great help. flume-ng-core/src/main/java/org/apache/flume/tools/TimestampRoundDownUtil.java <https://reviews.apache.org/r/5166/#comment17353> A brief javadoc on what the utility functions accept and return would be great. flume-ng-core/src/main/java/org/apache/flume/tools/TimestampRoundDownUtil.java <https://reviews.apache.org/r/5166/#comment17352> Internally Calendar uses long timestamp, so would preferable to call cal.setTimeInMillis(timestamp) instead. flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java <https://reviews.apache.org/r/5166/#comment17348> please use equalsIgnoreCase("hour") instead. flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java <https://reviews.apache.org/r/5166/#comment17349> please use equalsIgnoreCase("minute") instead. flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java <https://reviews.apache.org/r/5166/#comment17350> If the unit is not configured correctly, it will be better to print a warning and disable rounding altogether. Also the patch needs to be rebased on the latest sources as it is not applying cleanly. - Arvind On 2012-05-19 02:36:22, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5166/ > ----------------------------------------------------------- > > (Updated 2012-05-19 02:36:22) > > > 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/tools/TestTimestampRoundDownUtil.java > PRE-CREATION > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > 8ce2bac > > Diff: https://reviews.apache.org/r/5166/diff > > > Testing > ------- > > Added unit tests. > > > Thanks, > > Hari > >
