> On Sept. 11, 2012, 11:49 p.m., Cheolsoo Park wrote: > > I only have a minor comment. Please see inline.
Thank you for your review Cheolsoo, I appreciate your time. > On Sept. 11, 2012, 11:49 p.m., Cheolsoo Park wrote: > > src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java, line 142 > > <https://reviews.apache.org/r/7014/diff/1/?file=152587#file152587line142> > > > > 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. Make sense to me, I'll update the patch. - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7014/#review11361 ----------------------------------------------------------- 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 > >
