> On Feb. 7, 2015, 7:30 a.m., Veena Basavaraj wrote: > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java, > > line 119 > > <https://reviews.apache.org/r/30303/diff/4/?file=853438#file853438line119> > > > > I would prefer we keep these methods in CSVIDFUtils. > > > > SqoopIDFUtils was named so, because it handles the IDF column types. > > > > If we are extracting away the functionality of CSVIDF to be used in > > other connectors. then it is best to move thsoe methods into its own utils > > so connectors can use it and have correponding tests for it. there. > > Abraham Elmahrek wrote: > Ultimately it would be nice to have our own IDF. Let's consider this a > temporary step in the right direction and create an IDF after the release?
we have Sqoop AvroUtils for the avro stuff, so moving these 2 methods out to CSVUTils is better, dont think it is much effort to add a new class > On Feb. 7, 2015, 7:30 a.m., Veena Basavaraj wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java, > > line 565 > > <https://reviews.apache.org/r/30303/diff/4/?file=853446#file853446line565> > > > > As comments above, lets not bloat the SQoopIDFUtils, please move the > > CSVIDF methods to its own class, since it is doing more than just the > > common column types we support > > Abraham Elmahrek wrote: > A couple of thoughts on this: > 1. I think it belongs in SqoopIDFUtils because most IDFs will need to go > back and forth between CSV and Object form. CSV needs to be implemented by > all IDFs. > 2. If we want to change this, let's move it out in a different Jira. > ``parseCSVString`` was originally part of SqoopIDFUtils any ways. > https://github.com/apache/sqoop/blob/f7efa38005525e2fb22ce1af0323bf265d719c00/connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java#L518 I meant th fromCSV and fromObject - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30303/#review71553 ----------------------------------------------------------- On Feb. 8, 2015, 10:13 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30303/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2015, 10:13 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1579 > https://issues.apache.org/jira/browse/SQOOP-1579 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 74274f1d745215035eb766645b9e8dbb4c591139 > Author: Abraham Elmahrek <[email protected]> > Date: Mon Jan 26 19:21:15 2015 -0800 > > SQOOP-1579: Sqoop2: Data transfer to load into Hive does not work > > :100644 100644 9f3367b... 6b8ae52... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > :100644 100644 556c112... 9dba157... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > :100644 100644 353c1f2... 941c378... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java > :100644 100644 b7a9c3d... aa5f316... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java > :100644 100644 2dca634... 3efc76a... M > connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties > :100644 100644 9a5ec5e... c3b2975... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > :100644 100644 8eefe05... c8d404b... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > :100644 100644 b5ec6da... 62d5eb8... M > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > :100644 100644 c5eb6a3... c2dd812... M > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java > :100644 100644 3350599... c92bf61... M > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/AVROIntermediateDataFormat.java > :100644 100644 b716e79... 71d9534... M > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java > :100644 100644 4c67c1a... a8d186d... M > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/JSONIntermediateDataFormat.java > :100644 100644 588ce29... 1166496... M > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java > :100644 100644 f658d09... a5be42b... M > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java > :000000 100644 0000000... 17148bc... A > test/src/main/java/org/apache/sqoop/test/data/ShortStories.java > :100644 100644 1124cd3... 12bf99c... M > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > :100644 100644 a65d862... f6f7188... M > test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java > :100644 100644 f82abc7... a81b349... M > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java > :100644 100644 aa9f212... bb91a83... M > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > > > Diffs > ----- > > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > f70d56b > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > bdee878 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java > 353c1f2 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java > b7a9c3d > > connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties > 2dca634 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > 9a5ec5e > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > 8eefe05 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > b5ec6da > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java > 71641ed > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java > c233fb2 > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java > 588ce29 > test/src/main/java/org/apache/sqoop/test/data/ShortStories.java > PRE-CREATION > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > 398a051 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java > 397ce6f > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > ced52cc > > Diff: https://reviews.apache.org/r/30303/diff/ > > > Testing > ------- > > mvn clean integration-test > > > Thanks, > > Abraham Elmahrek > >
