> On April 17, 2013, 11:09 p.m., Jarek Cecho wrote: > > Hi Hari, > > thank you very much for your effort to get this in. I think that we're > > almost there!
Thanks for the review, Jarcec. I have posted an updated patch which fixes most of the issues you pointed out. > On April 17, 2013, 11:09 p.m., Jarek Cecho wrote: > > common/src/main/java/org/apache/sqoop/model/MJob.java, lines 121-127 > > <https://reviews.apache.org/r/10281/diff/5/?file=281617#file281617line121> > > > > 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. If that is the case, we should remove the public constructors and move them to a builder/factory model - that clearly shows that these are not meant to be added/removed. Also we should probably return immutable lists in the getters (else I can change anything I want through that). > On April 17, 2013, 11:09 p.m., Jarek Cecho wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java, > > line 634 > > <https://reviews.apache.org/r/10281/diff/5/?file=281625#file281625line634> > > > > Nit: The "WHERE" should not be intended on the same level as "ON". This particular query was removed. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10281/#review19320 ----------------------------------------------------------- On April 18, 2013, 6:58 a.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10281/ > ----------------------------------------------------------- > > (Updated April 18, 2013, 6:58 a.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 > >
