----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11041/#review20831 -----------------------------------------------------------
Hi Shruti, thank you very much for working on this, greatly appreciated. I do have couple of high level notes: 1) Would you mind update the documentation describing the new functionality? You can find the docs in src/docs/ directory. 2) Would you mind adding a tests that will exercise the added functionality? 3) Please run "ant checkstyle" to check out the coding style. src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42942> Nit: Is this change necessary? src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42943> Nit: Is this change necessary? src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42944> What about using names suggesting the usage rather than content of the variable? Perhaps DELIMITER_COMMAND_LINE? src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42945> What about using names suggesting the usage rather than content of the variable? Perhaps DELIMITER_HBASE? src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42946> Nit: Is this change necessary? src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42947> Do you think that it would make sense to throw an exception here rather than returning NULL? src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java <https://reviews.apache.org/r/11041/#comment42948> I would advise to abstract the composite key behaviour to the class parent and the the parsing only once when the rowKey is being set rather than on each single row. Jarcec - Jarek Cecho On May 10, 2013, 12:06 p.m., Shruti Joshi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11041/ > ----------------------------------------------------------- > > (Updated May 10, 2013, 12:06 p.m.) > > > Review request for Sqoop. > > > Description > ------- > > Sqoop does not support importing data into hbase when table contains > composite key. > > > This addresses bug https://issues.cloudera.org/browse/SQOOP-107. > > https://issues.apache.org/jira/browse/https://issues.cloudera.org/browse/SQOOP-107 > > > Diffs > ----- > > src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 13c765c > > Diff: https://reviews.apache.org/r/11041/diff/ > > > Testing > ------- > > ant test (default) was a success. > > > Thanks, > > Shruti Joshi > >
