> On Jan. 10, 2013, 12:27 a.m., Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, > > line 112 > > <https://reviews.apache.org/r/8864/diff/5/?file=245497#file245497line112> > > > > Should this constructor delegate to the other constructor passing in a > > null?
I don't see this adding any value, all it would do is simply set conf to null, which it already is - right? I am just avoiding that one extra method call overhead. > On Jan. 10, 2013, 12:27 a.m., Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, > > line 304 > > <https://reviews.apache.org/r/8864/diff/5/?file=245497#file245497line304> > > > > Good idea! How about doing same check for base dir and give both checks > > a good error message? Actually if it is null, it perfectly ok - since we simply startup without it: if(zkBaseDir != null){ client = new HBaseClient(zkQuorum, zkBaseDir); } else { client = new HBaseClient(zkQuorum); } Added an error message though. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8864/#review15217 ----------------------------------------------------------- On Jan. 8, 2013, 12:14 a.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8864/ > ----------------------------------------------------------- > > (Updated Jan. 8, 2013, 12:14 a.m.) > > > Review request for Flume. > > > Description > ------- > > AsyncHbasesink can now be configured to use zk quorum and zk root from > configuration file. > > > This addresses bug FLUME-1821. > https://issues.apache.org/jira/browse/FLUME-1821 > > > Diffs > ----- > > flume-ng-doc/sphinx/FlumeUserGuide.rst 63b8f9b > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java > 1598f26 > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java > 463c9c3 > > flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestAsyncHBaseSink.java > c835172 > > Diff: https://reviews.apache.org/r/8864/diff/ > > > Testing > ------- > > All current tests pass. Added test to make sure new code works. > > > Thanks, > > Hari Shreedharan > >
