----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37596/#review96093 -----------------------------------------------------------
Ship it! It seems good to me except you're missing some of the upgrade logic. Or perhaps removed fields simply won't be transfered? Either way, a warning printed when upgrading might be a good idea. Or at least something in the logs. connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestGenericJdbcConnectorUpgrader.java (line 35) <https://reviews.apache.org/r/37596/#comment151307> Maybe add a test here to show that the removal works. - Abraham Elmahrek On Aug. 20, 2015, 4:21 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37596/ > ----------------------------------------------------------- > > (Updated Aug. 20, 2015, 4:21 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2441 > https://issues.apache.org/jira/browse/SQOOP-2441 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > I've dropped all code relevant to this feature. > > The patch doesn't deal with upgrade path in case that someone is using this > feature to a world where this feature is not available. As the feature is > completely broken (e.g. doesn't work at all), I'm kind of assuming that it > shouldn't be a problem. > > > Diffs > ----- > > > common/src/main/java/org/apache/sqoop/error/code/GenericJdbcConnectorError.java > 9a9bb66 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java > ad375fd > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ToJobConfig.java > c9651d5 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestGenericJdbcConnectorUpgrader.java > c39aabc > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java > 870ce98 > > Diff: https://reviews.apache.org/r/37596/diff/ > > > Testing > ------- > > Unit tests are passing. > > > Thanks, > > Jarek Cecho > >
