> On 2012-05-14 23:40:24, Juhani Connolly wrote: > > Thanks for the fixe, looks good, I had missed these two on my check > > yesterday evening, but otherwise everything looks good to me.
Thanks Juhani. I removed the comment mentioned. Regarding the partial success, we should probably look into it later. I will file a jira for that once this is committed. > On 2012-05-14 23:40:24, Juhani Connolly wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java, > > line 210 > > <https://reviews.apache.org/r/5073/diff/5/?file=108844#file108844line210> > > > > You should probably be checking the results here. > > > > Error handling for partial successes seems like it would be awkward, so > > might want to just get this committed and file another ticket for it. Yes. The only reason I didn't do that here was, I don't really know how far we should go with our contract. I don't think we should really bother if the HBase writes fail due to something outside our control. Hbase failures,we cause the transaction to be rolled back, but I don't think we should bother about individual failures. We can file a jira, and discuss it there - once we make a conclusive decision on that we can add it. So for now, I am not going to change this - once we decide, we can put it in a patch. > On 2012-05-14 23:40:24, Juhani Connolly wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/SimpleHbaseEventSerializer.java, > > lines 119-121 > > <https://reviews.apache.org/r/5073/diff/5/?file=108846#file108846line119> > > > > I'm not sure what this is for? Leftovers from a refactoring? Yes, seems like. Removing this and uploading new patch. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5073/#review7873 ----------------------------------------------------------- On 2012-05-14 18:31:15, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5073/ > ----------------------------------------------------------- > > (Updated 2012-05-14 18:31:15) > > > Review request for Flume. > > > Summary > ------- > > Hbase sink. > > > This addresses bug FLUME-1183. > https://issues.apache.org/jira/browse/FLUME-1183 > > > Diffs > ----- > > bin/flume-ng 0108997 > flume-ng-dist/pom.xml 5bdcfe7 > flume-ng-sinks/flume-ng-hbase-sink/pom.xml PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HbaseEventSerializer.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/SimpleHbaseEventSerializer.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/SimpleRowKeyGenerator.java > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestHBaseSink.java > PRE-CREATION > flume-ng-sinks/pom.xml acb3087 > pom.xml 8c11a2d > > Diff: https://reviews.apache.org/r/5073/diff > > > Testing > ------- > > Unit tests added > > > Thanks, > > Hari > >
