----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7014/#review11361 -----------------------------------------------------------
Ship it! I only have a minor comment. Please see inline. src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java <https://reviews.apache.org/r/7014/#comment24412> Can you move this line to inside the if statement? So it will look like: HTableDescriptor tableDesc = null; ... if (admin.tableExists(tableName)) { tableDesc = new HTableDescriptor(tableName); ... } else { tableDesc = admin.getTableDescriptor(Bytes.toBytes(tableName)); ... } The reasons are: 1) This seems cleaner to me. 2) Currently, the instantiation of HTableDescritor is wasted when the table already exists. That is, we instantiate a new HTableDescriptor and overwrite the reference with another. - Cheolsoo Park On Sept. 11, 2012, 11:03 a.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7014/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2012, 11:03 a.m.) > > > Review request for Sqoop. > > > Description > ------- > > Problem is that we're creating Table descriptor locally, which is not > importing remote table families automatically. I've enhanced the code to > retrieve remote table descriptor prior checking if the table family is > present. > > > This addresses bug SQOOP-600. > https://issues.apache.org/jira/browse/SQOOP-600 > > > Diffs > ----- > > src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2 > > Diff: https://reviews.apache.org/r/7014/diff/ > > > Testing > ------- > > I've done only manual testing on HBase 0.94. Automatic tests are disabled in > different profiles other than hadoopversion={20,100} and I'll work on getting > them working later. > > > Thanks, > > Jarek Cecho > >
