> On 2012-04-27 21:27:56, Mike Percy wrote:
> > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java, line 
> > 52
> > <https://reviews.apache.org/r/4809/diff/5/?file=104882#file104882line52>
> >
> >     The RFC5424 version field should be a digit followed by a space. So I 
> > think this pattern should be: "(?:\\d\\s)?"; i.e. no [] character class 
> > around in. Worth adding a unit test w/ the example messages from RFC5424 to 
> > be sure it's compliant.

Actually, I just checked and it works as-is. So, nevermind this comment :)


> On 2012-04-27 21:27:56, Mike Percy wrote:
> > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java, line 
> > 241
> > <https://reviews.apache.org/r/4809/diff/5/?file=104882#file104882line241>
> >
> >     Why would the Scanner be closed? Shouldn't this exception be logged or 
> > re-thrown?

Looks like it's not a closed Scanner but it will throw if match() is called 
when there are no matches. So, please disregard this too.


- Mike


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


On 2012-04-27 14:58:21, Prasad Mujumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4809/
> -----------------------------------------------------------
> 
> (Updated 2012-04-27 14:58:21)
> 
> 
> 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/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
> 
>

Reply via email to