> On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote: > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java, > > line 25 > > <https://reviews.apache.org/r/27004/diff/1/?file=728319#file728319line25> > > > > 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)
there is no code in the destroyer that uses the configs. > On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java, > > lines 29-34 > > <https://reviews.apache.org/r/27004/diff/1/?file=728330#file728330line29> > > > > 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. I did this for a reason, not because I wanted to copy paste. In a typed java system such as this, it does not allow the same class to be used. try it out yourself before saying so. If there is way to do it, I am more than happy to remove classes - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27004/#review57740 ----------------------------------------------------------- On Oct. 21, 2014, 4: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, 4: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 > >
