> On Aug. 6, 2014, 8:18 p.m., Mike Percy wrote:
> > flume-ng-core/src/main/java/org/apache/flume/source/SyslogParser.java, line 
> > 104
> > <https://reviews.apache.org/r/24202/diff/3/?file=651390#file651390line104>
> >
> >     We should use a SyslogUtils constant for this key, like the ones below
> 
> Abraham Elmahrek wrote:
>     I'm +1 to this in general, but there appears to be exceptions in code. 
> See SyslogUtils Line #16 in this review. The "host" and "timestamp" headers 
> are simply added as string literals.

That's true, but it's prob better to avoid duplicating string literals all over 
the place.


- Mike


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


On Aug. 12, 2014, 1:52 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24202/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2014, 1:52 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2438
>     https://issues.apache.org/jira/browse/FLUME-2438
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> commit 321a62abc29ce495743b174cb8dec648e5448e2f
> Author: Abraham Elmahrek <[email protected]>
> Date:   Fri Aug 1 18:33:11 2014 -0700
> 
>     FLUME-2438 Make Syslog source message body configurable
>     
>     keepFields in the agent configuration can accept a space
>     separated list of fields that include either "timestamp"
>     or "hostname" or both. This will tell the agent to include
>     the specified fields in the message body.
> 
> :100644 100644 427e0e3... 643bd67... M  
> flume-ng-core/src/main/java/org/apache/flume/source/MultiportSyslogTCPSource.java
> :100644 100644 557d121... 996b31b... M  
> flume-ng-core/src/main/java/org/apache/flume/source/SyslogParser.java
> :100644 100644 985949c... a312cb6... M  
> flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java
> :100644 100644 e84e4b6... 03e593c... M  
> flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java
> :100644 100644 01b8905... 40a96a8... M  
> flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java
> :100644 100644 a77bfc9... e966856... M  
> flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java
> :100644 100644 9b97c8c... 72dc5c1... M  
> flume-ng-core/src/test/java/org/apache/flume/source/TestMultiportSyslogTCPSource.java
> :100644 100644 2809163... 594e456... M  
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogParser.java
> :100644 100644 22fa200... d32e9cc... M  
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogTcpSource.java
> :100644 100644 95ee48c... 47f49d5... M  
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java
> :100644 100644 82b7dd0... 5a4db9d... M  
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java
> :100644 100644 daf6e72... d179bf8... M  flume-ng-doc/sphinx/FlumeUserGuide.rst
> :000000 100644 0000000... dc463d5... A  
> flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestMultiportSyslogTCPSource.java
> :000000 100644 0000000... 8e50386... A  
> flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestSyslogTCPSource.java
> 
> 
> Diffs
> -----
> 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/MultiportSyslogTCPSource.java
>  427e0e3 
>   flume-ng-core/src/main/java/org/apache/flume/source/SyslogParser.java 
> 557d121 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java
>  985949c 
>   flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java 
> e84e4b6 
>   flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java 
> 01b8905 
>   flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java 
> a77bfc9 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestMultiportSyslogTCPSource.java
>  9b97c8c 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogParser.java 
> 2809163 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogTcpSource.java 
> 22fa200 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java 
> 95ee48c 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java 
> 82b7dd0 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst daf6e72 
>   
> flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestSyslogSource.java
>  PRE-CREATION 
>   flume-ng-tests/src/test/java/org/apache/flume/test/util/StagedInstall.java 
> a6bd5e9 
>   flume-ng-tests/src/test/java/org/apache/flume/test/util/SyslogAgent.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24202/diff/
> 
> 
> Testing
> -------
> 
> Added integration test and updated unit tests
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>

Reply via email to