> On Oct. 18, 2014, 9:27 a.m., Jarek Cecho wrote: > > spi/src/main/java/org/apache/sqoop/connector/spi/ConfigurableUpgrader.java, > > line 50 > > <https://reviews.apache.org/r/26880/diff/2/?file=724556#file724556line50> > > > > Was exactly my concern :) Can we fille a JIRA for that and create put > > the JIRA number there directly? > > Veena Basavaraj wrote: > The JIRA is arelady there, it is part of this JIRA, please see the > comments in the JIRA, this is the point where I needed to discsus if we need > ConnectorUpgrader and DriverUpgrader subclasses? rather than interfaces. > > Jarek Cecho wrote: > I'm confused, if we want to revisit the API as a part of this JIRA, why > the patch is adding note that it will be done in the future? > > Anyway I'm wondering what are your thoughts about separate Connector and > Driver upgrader? > > The current code forces the connector developper to detect whether we > gave him "From" or "To" job forms which doesn't seem right. I think that we > should add a parameter Direction that will specify what direction are we > upgrading. > > Veena Basavaraj wrote: > but direction does not make sense for every config type that we will add > > it is not in the current patch, since I still dont see any comments in > JIRA validating the proposal I have > > Please read my comment in JIRA, it says I have part1 and part2 for the > same reason, I can wait for this discussion to proceed and then upload a full > patch ( both part 1 and part 2) > > I kindly request that you think about this carefully. > > Each configurable behaves differently, for instance Connector and Driver > expose configs but are very different entities, so does it makes sense to > have one interface for all objects that expose configs? > > second thing that I have noticed, the configurables themselves do not > tell what type of config they persist, we decide in SQOOP that it is of LINK/ > JOB, this makes it really hard to have an api that is self aware for these > configurables. > > Let me know if I am making any sense to you:)
Jarcec, I have updated a new patch, that aligns with having a single extensible api for upgrades. Let me know what your thoughts are. This resolves all the issues that was supposed to addressed by SQOOP -1551 - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26880/#review57267 ----------------------------------------------------------- On Oct. 17, 2014, 11:08 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26880/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2014, 11:08 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > #1 - + 1 from Jarek Jarcec Cecho, we shall call it ConfigurableUpgrader > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/MConfigurable.java PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MConfigurableType.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191 > common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java > 87ac2af > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorUpgrader.java > a069b3e > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConfigUpgrader.java > b17aa21 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > 606b9fa > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnectorUpgrader.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java 54bdd13 > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5226926 > core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb > core/src/main/java/org/apache/sqoop/driver/DriverConfigUpgrader.java > 847b73d > core/src/main/java/org/apache/sqoop/driver/DriverConfigValidator.java > 9c3b660 > core/src/main/java/org/apache/sqoop/driver/DriverUpgrader.java PRE-CREATION > core/src/main/java/org/apache/sqoop/driver/JobManager.java df2a5ab > > core/src/main/java/org/apache/sqoop/driver/configuration/DriverConfiguration.java > d4e2254 > > core/src/main/java/org/apache/sqoop/driver/configuration/JobConfiguration.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/repository/Repository.java 74a9e12 > core/src/test/java/org/apache/sqoop/driver/TestDriverConfigUpgrader.java > dc4e8c8 > core/src/test/java/org/apache/sqoop/driver/TestJobManager.java 3b475c6 > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > 34bd8a5 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > c888910 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > 44ec2e3 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 462579c > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 80e65b8 > shell/src/main/java/org/apache/sqoop/shell/core/Constants.java a1bc5d5 > spi/src/main/java/org/apache/sqoop/connector/spi/ConfigurableUpgrader.java > PRE-CREATION > spi/src/main/java/org/apache/sqoop/connector/spi/RepositoryUpgrader.java > 879e428 > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > 5315e1f > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java > 76ebd3b > > Diff: https://reviews.apache.org/r/26880/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
