----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10281/#review19320 -----------------------------------------------------------
Hi Hari, thank you very much for your effort to get this in. I think that we're almost there! common/src/main/java/org/apache/sqoop/model/MJob.java <https://reviews.apache.org/r/10281/#comment40043> I do not feel comfortable with exposing this interface. Model classes are (will be) part of public API, so I would prefer not to expose ability to switch forms in place and rather create new objects during the upgrade procedure. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java <https://reviews.apache.org/r/10281/#comment39996> I think that the upgrader should not be responsible for changing the form structures, just for transferring the actual values (e.g the upgrade method should just call "setValue()" on the forms). connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java <https://reviews.apache.org/r/10281/#comment39997> I think that the upgrader should not be responsible for changing the form structures, just for transferring the actual values (e.g the upgrade method should just call "setValue()" on the forms). core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java <https://reviews.apache.org/r/10281/#comment40042> I would recommend to merge those two methods together as it do not make much sense to use them separately. core/src/main/java/org/apache/sqoop/repository/Repository.java <https://reviews.apache.org/r/10281/#comment40044> What about just: 1) Get all connections for connector 2) Get all jobs for connector 3) Wipe out previous forms,inputs,connection inputs, job inputs 4) Update connector in the repository (Connector upgrade most likely will change form and input ids!) 5) for each connection call upgrader to transfer values and save output to the repository 6) for each job call upgrader to transfer values and save output to the repository repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/10281/#comment40048> Nit: Extra indent. repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/10281/#comment40047> Nit: Extra indent. repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/10281/#comment40046> Nit: Extra indent. repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/10281/#comment40045> Nit: The "WHERE" should not be intended on the same level as "ON". Jarcec - Jarek Cecho On April 16, 2013, 8:58 p.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10281/ > ----------------------------------------------------------- > > (Updated April 16, 2013, 8:58 p.m.) > > > Review request for Sqoop. > > > Description > ------- > > Metadata upgrade for connector upgrades. This is an initial patch soliciting > feedback. Limited testing has been done. I will add some unit tests once I > get feedback. > > > This addresses bug SQOOP-659. > https://issues.apache.org/jira/browse/SQOOP-659 > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42 > common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java > c315e48 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5 > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > ca51313 > core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java > 95f6570 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 32cef8a > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > ea458ac > spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java > PRE-CREATION > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > 540303a > > Diff: https://reviews.apache.org/r/10281/diff/ > > > Testing > ------- > > > Thanks, > > Hari Shreedharan > >
