> On Dec. 9, 2014, 2:48 p.m., Jarek Cecho wrote: > > core/pom.xml, lines 42-44 > > <https://reviews.apache.org/r/28828/diff/3/?file=786499#file786499line42> > > > > Wondering why is core depending on joda-time directly? > > Veena Basavaraj wrote: > jobManaager.
I see, the JobManager is explicitly naming some classes from JODA to be included in the job [1]. I think that the change is fine for now, but we should change it in subsequent JIRA. This way the core would have to depend on everything and the idea of having lightweight core would be gone. We should provide a way how the IDF will expose dependent jars and add them into the class path similar way as we are doing for connectors. Or perhaps it should be the connector who will report the dependent jars as it's connector who is picking the IDF. Links: 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/driver/JobManager.java#L402 > On Dec. 9, 2014, 2:48 p.m., Jarek Cecho wrote: > > core/pom.xml, lines 57-59 > > <https://reviews.apache.org/r/28828/diff/3/?file=786499#file786499line57> > > > > I'm wondering why would core depend on connector-sdk? > > Veena Basavaraj wrote: > since there are tests that use CSV I believe that core should not depend on connector-sdk from the same reason why it's not depending on any other connectors. If we have functionality that is required in core, then perhaps the code should not be in connector-sdk. > On Dec. 9, 2014, 2:48 p.m., Jarek Cecho wrote: > > execution/mapreduce/pom.xml, lines 36-39 > > <https://reviews.apache.org/r/28828/diff/3/?file=786502#file786502line36> > > > > Wondering why is execution engine depending on connector-sdk? > > Veena Basavaraj wrote: > since tests use CSV I believe that mapreduce execution should not depend on connector-sdk from the same reason why it's not depending on any other connectors. If we have functionality that is required in execution engine, then perhaps the code should not be in connector-sdk. > On Dec. 9, 2014, 2:48 p.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 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 - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64374 ----------------------------------------------------------- On Dec. 9, 2014, 8: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, 8: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 > >
