> On 2012-04-20 21:05:23, Mike Percy wrote: > > Nice work Prasad, looks good overall. I have a couple of nits / questions > > inline. > > > > Also, we should file another JIRA to support the RFC5424 timestamp format > > that supports fractional seconds, i.e. 2003-10-11T22:14:15.003Z and > > 2003-08-24T05:14:15.000003-07:00. While we would need to round microsecond > > times to the nearest millisecond since our base format is Java milliseconds > > since the epoch, today we treat these example timestamp strings as invalid.
Create FLUME-1138 to track the millisecond format. > On 2012-04-20 21:05:23, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java, line > > 60 > > <https://reviews.apache.org/r/4809/diff/4/?file=103439#file103439line60> > > > > This variable is not ever read. right, that logic was changed and it's no longer need. will remove it. > On 2012-04-20 21:05:23, Mike Percy wrote: > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java, line > > 267 > > <https://reviews.apache.org/r/4809/diff/4/?file=103439#file103439line267> > > > > Not sure what's going on here. Why are we checking for a dash? And what > > does startedTimeStamp mean? I don't think it means we have started parsing > > the timestamp. The RFC5424 allows a 'null' timestamp which contains a single '-' The code is checking for the dash as first character. I guess that variable name is confusing, will change that. - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4809/#review7093 ----------------------------------------------------------- On 2012-04-20 03:19:50, Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4809/ > ----------------------------------------------------------- > > (Updated 2012-04-20 03:19:50) > > > Review request for Flume, Arvind Prabhakar and Mike Percy. > > > Summary > ------- > > Support for timestamp and hostname parsing. > > > This addresses bug FLUME-1126. > https://issues.apache.org/jira/browse/FLUME-1126 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > b0485b1 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > 732cce5 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java > 653f5eb > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > 3a7c486 > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java > 8b1f7c5 > > Diff: https://reviews.apache.org/r/4809/diff > > > Testing > ------- > > Updated syslog tests to cover timestamp and hostname handling. > Manually tested using syslog4j > > > Thanks, > > Prasad > >
