> 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.
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:) - 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 > >
