> On 2011-11-23 00:23:02, Francis Liu wrote: > > storage-drivers/hbase/src/java/org/apache/hcatalog/hbase/HBaseDirectOutputStorageDriver.java, > > line 41 > > <https://reviews.apache.org/r/2809/diff/1/?file=57620#file57620line41> > > > > I left it to the subclass to do the serialization since the subclass > > may need to add more information to outputJobInfo before it gets > > serialized. Such as in HBaseBulkOutputStorageDriver where the intermediate > > location has to be stored in OutputJobInfo.
I see. In that case, it will be better to add a comment in HBaseBaseOutputStorageDriver::initialize() that anyone extending this class must necessarily override initialize() and then overwrite the outputJobInfo in it. Also, patch doesn't apply cleanly, can you refresh it? - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2809/#review3450 ----------------------------------------------------------- On 2011-11-11 18:24:59, Francis Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2809/ > ----------------------------------------------------------- > > (Updated 2011-11-11 18:24:59) > > > Review request for hcatalog, Sushanth Sowmyan, Vandana Ayyalasomayajula, and > David Capwell. > > > Summary > ------- > > HBaseDirectOutputStorageDriver missed serializing the updated OutputJobInfo, > fixed that. > > > This addresses bug hcatalog-160. > https://issues.apache.org/jira/browse/hcatalog-160 > > > Diffs > ----- > > > storage-drivers/hbase/src/java/org/apache/hcatalog/hbase/HBaseDirectOutputStorageDriver.java > 65dfccb > > storage-drivers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseBulkOutputStorageDriver.java > c25e56d > > storage-drivers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseDirectOutputStorageDriver.java > d612584 > > Diff: https://reviews.apache.org/r/2809/diff > > > Testing > ------- > > Updated unit tests to such a scenario and it passes now. > > > Thanks, > > Francis > >
