----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5188/#review8180 -----------------------------------------------------------
Mike, Thanks for the patch! Overall, looks good. Some feedback inline. flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java <https://reviews.apache.org/r/5188/#comment17702> Trailing whitespaces. flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java <https://reviews.apache.org/r/5188/#comment17701> We use enums all over to specify types of components. It would be good if this one used it too, at least to be uniform. We should either be using enums throughout or static final strings, not both -- that is what I feel. flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java <https://reviews.apache.org/r/5188/#comment17703> Just for safety, do alias.toUpperCase()? flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java <https://reviews.apache.org/r/5188/#comment17707> If there are no headers in the event, would event.getHeaders return null? If it does, a check might be useful(and creating a map and inserting into it). flume-ng-core/src/test/java/org/apache/flume/interceptor/TestTimestampInterceptor.java <https://reviews.apache.org/r/5188/#comment17706> Trailing whitespaces. - Hari On 2012-05-22 01:53:38, Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5188/ > ----------------------------------------------------------- > > (Updated 2012-05-22 01:53:38) > > > Review request for Flume. > > > Summary > ------- > > Created a timestamp interceptor that sets the current time as the "timestamp" > header of all events that flow through it. > > > This addresses bug FLUME-1215. > https://issues.apache.org/jira/browse/FLUME-1215 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java > abf2f75 > > flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java > PRE-CREATION > > flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java > PRE-CREATION > > flume-ng-core/src/test/java/org/apache/flume/interceptor/TestTimestampInterceptor.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/5188/diff > > > Testing > ------- > > Added unit test. > > > Thanks, > > Mike > >
