> On Dec. 9, 2014, 6:48 a.m., Jarek Cecho wrote: > > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java, line > > 95 > > <https://reviews.apache.org/r/28828/diff/3/?file=786511#file786511line95> > > > > I'm wondering why this change? It seems reasonable to have default > > value. > > Veena Basavaraj wrote: > if we have that then we need to think of how SPI does not deepend on SDK, > and it creates a cyclic dependency > > Jarek Cecho wrote: > I see, that is a good point. I think that the cyclic dependency is the > reason why Hari put the IDF implementation into the connector-sdk as well in > the first place (as we've discussed in [1]). Let's perhaps not change that at > this point as it obviously requires more thinking as we are quite heavily > changing the dependencies and not just the API as we originally intented to? > > Links: > 1: > https://issues.apache.org/jira/browse/SQOOP-1811?focusedCommentId=14236216&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14236216 > > Veena Basavaraj wrote: > IMO it was a hack Hari did , and it is not right. and IDF API is not part > of SDK, then we should add everything in SPI. > > the only dependency we have is tests using CSV, the tests should use a > TEST CSV or mock it and not rely on a actual class, this is the way it should > have been,
to be more precise, API should never depend on its implementation, it is always the other way round where implementation package depend on its API, at this point, I will defer this to another ticket, but it is pretty big hackery that way it is set up right now. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64374 ----------------------------------------------------------- On Dec. 9, 2014, 12:17 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28828/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2014, 12:17 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1811 > https://issues.apache.org/jira/browse/SQOOP-1811 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see jira > > > Diffs > ----- > > connector/connector-generic-jdbc/pom.xml fc6cab4 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java > c233ed5 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > f70142d > connector/connector-kite/pom.xml 10ed099 > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteConnector.java > c864882 > connector/connector-sdk/pom.xml 38c217a > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java > d6470e6 > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormatError.java > PRE-CREATION > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormat.java > 253dfba > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormatError.java > 4b0dd88 > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java > 3e7c0d1 > core/pom.xml 2b6e436 > core/src/main/java/org/apache/sqoop/driver/JobManager.java f4f5561 > core/src/main/java/org/apache/sqoop/driver/JobRequest.java eed79a5 > execution/mapreduce/pom.xml b23b905 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/io/SqoopWritable.java > 05b731a > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java > b9dd11d > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java > 49a66b9 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java > bbac7d2 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java > a64a4a6 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java > 7c40ad5 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java > 5d5359e > spi/pom.xml 43f17d4 > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > ff6392e > spi/src/main/java/org/apache/sqoop/idf/IntermediateDataFormat.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/28828/diff/ > > > Testing > ------- > > > Thanks, > > Veena Basavaraj > >