----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12774/#review24180 -----------------------------------------------------------
Hi Raghav, thank you for incorporating all the feedback! I've applied the patch and it seems that I'm getting compilation failures (for example at file TestJdbcRepository) due to the changed constructor of MFramework structure. Would you mind fixing those? core/src/main/java/org/apache/sqoop/framework/FrameworkManager.java <https://reviews.apache.org/r/12774/#comment48002> Nit: Looking at how the JDBC Generic connector is handling the version, it seems that is actually using the release version from VersionInfo.getVersion(). I don't think that we have to be necessary consistent here as it's different subsystem, so I'm just sharing the idea for consideration: https://github.com/apache/sqoop/blob/sqoop2/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java#L58 Jarcec - Jarek Cecho On July 29, 2013, 8:25 p.m., Raghav Gautam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12774/ > ----------------------------------------------------------- > > (Updated July 29, 2013, 8:25 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1075 > https://issues.apache.org/jira/browse/SQOOP-1075 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > A summary of changes: > 1. MFramework: added fields, getter & setter method for version. Made changes > to other methods to take version into account. > 2. MConnector: since it extends MFramework, can use his version field and > methods > 3. FrameworkBean, FormSerialization: encode/decode version field to json > 4. DerbyRepositoryHandler, DerbyRepoError: added private methods to > persist/fetch framework version; update framework version when > registering/updating framework > 5. TestFrameworkHandling: added test - the test checks for current version of > framework, changes it to lower version updates the framework and checks the > version of framework once again > 6. TestSqoopClient, TestUtils, TestMFramework, FrameworkManager, > DerbyTestCase: added version parameter to the MFramework constructor call. > > > Diffs > ----- > > client/src/test/java/org/apache/sqoop/client/TestSqoopClient.java 1778cf1 > common/src/main/java/org/apache/sqoop/json/FrameworkBean.java ad4753b > common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java > 98768d6 > common/src/main/java/org/apache/sqoop/model/MConnector.java 1c2c422 > common/src/main/java/org/apache/sqoop/model/MFramework.java 694f022 > common/src/test/java/org/apache/sqoop/json/TestUtil.java b88d7a4 > common/src/test/java/org/apache/sqoop/model/TestMFramework.java a5366ca > core/src/main/java/org/apache/sqoop/framework/FrameworkManager.java ad6cd0f > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > 607b8d5 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java > aeb7533 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > f717abf > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 677b0be > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestFrameworkHandling.java > 66611d4 > > Diff: https://reviews.apache.org/r/12774/diff/ > > > Testing > ------- > > I have added new tests. > All the unit tests are passing. > > > Thanks, > > Raghav Gautam > >
