----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30303/#review71553 -----------------------------------------------------------
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java <https://reviews.apache.org/r/30303/#comment117309> 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. connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java <https://reviews.apache.org/r/30303/#comment117308> this seems such a overkill, whoveever intends to use the object format. We convert it back and forth between csv to OBJECT and then object to CSV and then the same dance hapens on the TO side. Is this going to be a common use case anyone will use? Just wondnering. The whole point of using text format in a connector and then use CSVIDF was to make sure we have very little conversions going on, but this use case seems to just completely do the opposite. Now if I imagine supporting a avro/ parquet in this conenctor in future, I wonder what format it will use. food for thought. connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java <https://reviews.apache.org/r/30303/#comment117305> there is some new lines added >>>> that shows up in RB connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java <https://reviews.apache.org/r/30303/#comment117310> 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 connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java <https://reviews.apache.org/r/30303/#comment117306> why do we need to move this to IDFUtils, if this is speciifc to CSV, then its better to create a SQOOPCSVUtils and not bloat the IDF Utils. IDF Utils has methods to handle each column type and hence it was named so. - Veena Basavaraj On Feb. 6, 2015, 6:23 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30303/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2015, 6:23 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1579 > https://issues.apache.org/jira/browse/SQOOP-1579 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > A question for the reviewer: > # Does the option "unescape" with default to "true" make sense? It did for > me, but I can imagine from the users perspective that it might be confusing. > > 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 > >
