> On 2012-05-29 23:46:13, Hari Shreedharan wrote: > > flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java, > > lines 9-11 > > <https://reviews.apache.org/r/5188/diff/1/?file=109767#file109767line9> > > > > Trailing whitespaces.
Fixed > On 2012-05-29 23:46:13, Hari Shreedharan wrote: > > flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java, > > line 32 > > <https://reviews.apache.org/r/5188/diff/1/?file=109767#file109767line32> > > > > 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. Aw Hari you are stifling my creativity man!! ;) Fair enough, I reverted to using an enum. Don't tell me the lack of an OTHER is a bad thing though ;) > On 2012-05-29 23:46:13, Hari Shreedharan wrote: > > flume-ng-core/src/main/java/org/apache/flume/interceptor/InterceptorBuilderFactory.java, > > line 39 > > <https://reviews.apache.org/r/5188/diff/1/?file=109767#file109767line39> > > > > Just for safety, do alias.toUpperCase()? It was higher in the stack but it's there now. > On 2012-05-29 23:46:13, Hari Shreedharan wrote: > > flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java, > > line 49 > > <https://reviews.apache.org/r/5188/diff/1/?file=109768#file109768line49> > > > > 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). Sounds like a bug if anyone ever sets headers to null. Should be ok. > On 2012-05-29 23:46:13, Hari Shreedharan wrote: > > flume-ng-core/src/test/java/org/apache/flume/interceptor/TestTimestampInterceptor.java, > > lines 9-11 > > <https://reviews.apache.org/r/5188/diff/1/?file=109769#file109769line9> > > > > Trailing whitespaces. Fixed - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5188/#review8180 ----------------------------------------------------------- On 2012-05-31 07:40:36, Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5188/ > ----------------------------------------------------------- > > (Updated 2012-05-31 07:40:36) > > > 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 > >
