> On Dec. 5, 2015, 12:35 p.m., Jarek Cecho wrote: > > My apologies for jumping so late on this review. Few high level notes: > > > > 1) It seems that the patch no longer applies cleanly, so you will have to > > rebase it. > > > > 2) I know that we've changed the design quite a bit - would you mind > > updating the JIRA and the review board summary with what exactly we're > > trying to accomplish here? I don't think that the title "Sqoop2: Allow > > connectors to express jar dependencies" is still relevant. It's more about > > wrap connector's dependencies inside the connector's jar, right? > > > > 3) This is rather huge change - I'm wondering if there are independent > > pieces (like changes to ConnectorClassLoader) that can be submitted via > > independent JIRA? It will make the subsequent reviews easier. > > > > And few more tactical:
Hi Jarcec, Thanks a lot for the review and suggestions. 1) Have updated the title and description of this JIRA. The new description should can explain what we're trying to do here. 2) Have moved changes related to ConnectorClassLoader to SQOOP-2635. - Dian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39586/#review108543 ----------------------------------------------------------- On Dec. 10, 2015, 9:45 a.m., Dian Fu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39586/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2015, 9:45 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2634 > https://issues.apache.org/jira/browse/SQOOP-2634 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > The aim of this JIRA is to provide classpath isolation for connectors and its > dependencies. It's achieved in the following method: > 1. Package connector jar with its dependencies. There will be a directory lib > which contains the dependencies of the connector > 2. Improve the ConnectorClassLoader to make it can load classes from the > dependencies which are inside the connector jar (SQOOP-2635) > 3. Load connector class with the connector's own ConnectorClassLoader. > > > Diffs > ----- > > assemblies/pom.xml PRE-CREATION > assemblies/src/main/resources/assemblies/sqoop-connector.xml PRE-CREATION > common/src/main/java/org/apache/sqoop/error/code/ConnectorError.java > 2f17d95 > common/src/main/java/org/apache/sqoop/utils/ConnectorClassLoader.java > 4c42a78 > common/src/main/resources/org.apache.sqoop.connector-classloader.properties > e2936a9 > connector/connector-ftp/pom.xml 41ea026 > connector/connector-generic-jdbc/pom.xml 052f06d > connector/connector-hdfs/pom.xml 022a024 > connector/connector-kafka/pom.xml e0f0684 > > connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java > 923d1aa > connector/connector-kite/pom.xml d8eaa8e > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteFromInitializer.java > 28c5bac > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteToInitializer.java > 50daba0 > connector/connector-oracle-jdbc/pom.xml 8186b3a > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > 85ba8be > connector/connector-sftp/pom.xml 312ac61 > > connector/connector-sftp/src/main/java/org/apache/sqoop/connector/sftp/SftpToInitializer.java > bfb51ac > core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java 1899bb7 > core/src/main/java/org/apache/sqoop/connector/ConnectorManagerUtils.java > 9f9be57 > core/src/main/java/org/apache/sqoop/driver/JobManager.java 15ca796 > > execution/mapreduce/src/main/java/org/apache/sqoop/execution/mapreduce/MapreduceExecutionEngine.java > 3acd4a1 > execution/mapreduce/src/main/java/org/apache/sqoop/job/MRJobConstants.java > 737ceda > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/MRUtils.java > PRE-CREATION > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopInputFormat.java > 0623f7b > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java > 7d20992 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopNullOutputFormat.java > 8c8526b > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java > 623d1f4 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java > 2463643 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java > d0b41d1 > pom.xml 91721ce > server/pom.xml 370a6a2 > shell/src/test/java/org/apache/sqoop/shell/TestCloneCommand.java 7a139ba > shell/src/test/java/org/apache/sqoop/shell/TestCreateCommand.java 0316a60 > shell/src/test/java/org/apache/sqoop/shell/TestUpdateCommand.java d8f061a > > submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java > f396783 > > Diff: https://reviews.apache.org/r/39586/diff/ > > > Testing > ------- > > > Thanks, > > Dian Fu > >
