> On 2012-03-26 20:41:13, Brock Noland wrote:
> > flume-ng-core/src/main/java/org/apache/flume/event/EventHelper.java, line 42
> > <https://reviews.apache.org/r/4488/diff/2/?file=95918#file95918line42>
> >
> >     already done your null check, no reason to do it again

Good catch, I actually changed it back to pretty much how it was originally.


> On 2012-03-26 20:41:13, Brock Noland wrote:
> > flume-ng-core/src/main/java/org/apache/flume/event/EventHelper.java, line 45
> > <https://reviews.apache.org/r/4488/diff/2/?file=95918#file95918line45>
> >
> >     This is not in the original code, but the in this setup, the number of 
> > bytes could be configurable and passed to dumpEvent from LoggerSink

I don't want to add features as part of this patch, since this is just to fix 
the dependency issues, but I did just add an overloaded method that takes a 
param for # of bytes of the body to include, and left a default of 16. Adding 
configuration of this param to LoggerSink should be done as a separate ticket 
please.


> On 2012-03-26 20:41:13, Brock Noland wrote:
> > flume-ng-sdk/src/main/java/org/apache/flume/event/SimpleEvent.java, line 66
> > <https://reviews.apache.org/r/4488/diff/2/?file=95921#file95921line66>
> >
> >     Something in this toString() would be nice since it will probably be 
> > used for debugging by users at some point.  Can we print something like 
> > "[Event headers = " + headers + ", body.length = " + body.length + " ]";

Good call, I've incorporated this basically verbatim.


- Mike


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


On 2012-03-26 21:05:49, Mike Percy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4488/
> -----------------------------------------------------------
> 
> (Updated 2012-03-26 21:05:49)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> Created a static utility method to do the HexDump that the LoggerSink uses as 
> output for events. This lives in the core so that the SDK no longer has any 
> undeclared dependencies.
> 
> 
> This addresses bug FULME-1047.
>     https://issues.apache.org/jira/browse/FULME-1047
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/sink/LoggerSink.java f6f9d33 
>   flume-ng-sdk/src/main/java/org/apache/flume/event/SimpleEvent.java 9099206 
>   flume-ng-sdk/src/test/java/org/apache/flume/event/TestSimpleEvent.java 
> 385cb7e 
> 
> Diff: https://reviews.apache.org/r/4488/diff
> 
> 
> Testing
> -------
> 
> mvn clean package
> Unpacked tarball, ran: ./bin/flume-ng node -c conf -f 
> conf/flume-conf.properties.template -n foo
> Lots of stuff dumped to stdout
> 
> 
> Thanks,
> 
> Mike
> 
>

Reply via email to