----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27004/#review57740 -----------------------------------------------------------
Thank you for taking stab at this one Veena! connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java <https://reviews.apache.org/r/27004/#comment98582> HDFS is actually using the Job configuration, so we should probably use the correct class here, right? (I've seen the same on multiple places, but I've highlighted it only here) connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java <https://reviews.apache.org/r/27004/#comment98583> Why do we need the empty body here? It seems weird. Wouldn't be simpler to just have the class completely empty? Also considering that we have a high level entity of configuration, I would just create "EmptyConfiguration" class and use it at any place where Link/Job are required. It seems to me that "EmptyJobConfiguration" and "EmptyLinkConfiguration" are just copy&paste. Jarcec - Jarek Cecho On Oct. 21, 2014, 11:59 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27004/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2014, 11:59 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA for details. > > also it fixes a lot of warnings in the code base. > > > Diffs > ----- > > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > e63e464 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java > 74b1cb8 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > 2c8b6c8 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java > bb5e353 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > 660418d > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java > f40459f > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java > 5d48a29 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java > c0cd336 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > c6d2f90 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > 552a751 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java > 9d177ec > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java > PRE-CREATION > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java > PRE-CREATION > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java > PRE-CREATION > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java > 6d0dcb4 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java > 665a65b > spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d > spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3 > spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df > > Diff: https://reviews.apache.org/r/27004/diff/ > > > Testing > ------- > > yes tests pass. > > > Thanks, > > Veena Basavaraj > >
