----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3741/#review5041 -----------------------------------------------------------
Changes look good. Some feedback follows: * Please remove trailing whitespaces and tabs where present. These are highlighted in red on the review board diff. * The change is missing tests. Since this is an important feature enhancement, it is necessary to have some basic testing in place to assert that it works and continues to work. * The implementation of the decoder depends upon pre-existing headers in the event. While this will work for planned flows that need to end up in HBase, it will not work that well for adhoc flows that the user wishes to route to HBase. In such cases, the requirement to have these headers in place will come in the way of making use of this. It is therefore preferable to have an alternate implementation that uses static configuration to place the events in HBase as well. flume-ng-sinks/flume-hbase-sink/src/main/java/org/apache/flume/sink/hbase/DefaultEventToPutDecoder.java <https://reviews.apache.org/r/3741/#comment11071> The default implementation uses a few hardcoded header names. There should be a mechanism to override these via configuration if necessary. - Arvind On 2012-02-02 23:24:02, Karthik K wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3741/ > ----------------------------------------------------------- > > (Updated 2012-02-02 23:24:02) > > > Review request for Flume and Arvind Prabhakar. > > > Summary > ------- > > HBase Sink, ported to Flume NG. > > Configurable EventToPutDecoder in place. > > > This addresses bug FLUME-888. > https://issues.apache.org/jira/browse/FLUME-888 > > > Diffs > ----- > > flume-ng-sinks/flume-hbase-sink/pom.xml PRE-CREATION > > flume-ng-sinks/flume-hbase-sink/src/main/java/org/apache/flume/sink/hbase/DefaultEventToPutDecoder.java > PRE-CREATION > > flume-ng-sinks/flume-hbase-sink/src/main/java/org/apache/flume/sink/hbase/EventToPutDecoder.java > PRE-CREATION > > flume-ng-sinks/flume-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseEventSink.java > PRE-CREATION > pom.xml 2bf32bb > > Diff: https://reviews.apache.org/r/3741/diff > > > Testing > ------- > > Tested with the DefaultEventToPutDecoder in place, to map Event-s to Put-s. > > > Thanks, > > Karthik > >
