----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4488/#review6363 -----------------------------------------------------------
I think this is a good change, I wish I had done it this way originally. A few minor comments below. flume-ng-core/src/main/java/org/apache/flume/event/EventHelper.java <https://reviews.apache.org/r/4488/#comment13803> already done your null check, no reason to do it again flume-ng-core/src/main/java/org/apache/flume/event/EventHelper.java <https://reviews.apache.org/r/4488/#comment13804> 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 flume-ng-sdk/src/main/java/org/apache/flume/event/SimpleEvent.java <https://reviews.apache.org/r/4488/#comment13806> 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 + " ]"; - Brock On 2012-03-26 20:28:54, Mike Percy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4488/ > ----------------------------------------------------------- > > (Updated 2012-03-26 20:28:54) > > > 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/event/EventHelper.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/sink/LoggerSink.java f6f9d33 > flume-ng-core/src/test/java/org/apache/flume/event/TestEventHelper.java > PRE-CREATION > 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 > >
