----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28614/#review64573 -----------------------------------------------------------
Nice enhancement, have a few comments, connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java <https://reviews.apache.org/r/28614/#comment107339> nitpick, why not pass thse into the fucntions, less state in the classes is good pattern to avoid any future multi thread issue connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java <https://reviews.apache.org/r/28614/#comment107340> modifiable probably use the ssame terminilogy as transform to keep it consistent connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java <https://reviews.apache.org/r/28614/#comment107341> probably at some point this can be an interface optinally implemented by conenctors. Since even JDBC needs to do this, Right now this code was added in extractor in 1830 and needs to bed added in loader as well, since we found a bug connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java <https://reviews.apache.org/r/28614/#comment107342> please add more help text in the .properites connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java <https://reviews.apache.org/r/28614/#comment107343> nice tests.! - Veena Basavaraj On Dec. 2, 2014, 3:31 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28614/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2014, 3:31 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1678 > https://issues.apache.org/jira/browse/SQOOP-1678 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 3898ce500f7aac0104de05e3c2c33d1c6cf7d13c > Author: Abraham Elmahrek <[email protected]> > Date: Tue Dec 2 15:23:12 2014 -0800 > > SQOOP-1678: Sqoop2: Configurable null values > > :100644 100644 6e369c6... 788dfd2... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConstants.java > :100644 100644 2586f94... d92e296... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > :100644 100644 6c57cf2... e7b1302... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > :100644 100644 352ee17... f3ac167... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java > :100644 100644 509d772... 89ff9aa... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java > :100644 100644 abddbfb... b7a9c3d... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java > :100644 100644 0a6369f... 52846ed... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > :100644 100644 6eae7fd... ac44595... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsBase.java > :100644 100644 63e14ae... 15e3c14... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > :100644 100644 b404c34... be57fa0... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > > > Diffs > ----- > > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConstants.java > 6e369c6 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > 2586f94 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > 6c57cf2 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java > 352ee17 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java > 509d772 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java > abddbfb > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > 0a6369f > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsBase.java > 6eae7fd > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > 63e14ae > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > b404c34 > > Diff: https://reviews.apache.org/r/28614/diff/ > > > Testing > ------- > > 'mvn verify' with local runner. > > > Thanks, > > Abraham Elmahrek > >
