----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22034/#review44322 -----------------------------------------------------------
Couple of other comments: repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java <https://reviews.apache.org/r/22034/#comment78697> I would put the debug message as the first statement in the method rather then at the end. repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java <https://reviews.apache.org/r/22034/#comment78696> Removing the finally section do not seem to be completely accurate here, we still want to close the baseConnectorFetchStmt, right? repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java <https://reviews.apache.org/r/22034/#comment78698> Nit: Please use the one line comment that starts with "//" repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java <https://reviews.apache.org/r/22034/#comment78699> Super nit: please put space between "//" and "Retrieve" - Jarek Cecho On May 29, 2014, 7:53 p.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22034/ > ----------------------------------------------------------- > > (Updated May 29, 2014, 7:53 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Adding a method to get all connectors to the repository API. > > Note that while this patch adds a new method, it also modifies the existing > findConnector(String shortName) to avoid duplicating code. > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 7768b13 > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > 9299484 > core/src/main/java/org/apache/sqoop/repository/Repository.java 4c7b866 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java > e4b30f9 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 4f002bb > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > c470211 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > a1ad40d > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > cc3fe60 > > Diff: https://reviews.apache.org/r/22034/diff/ > > > Testing > ------- > > Added unit-test for the new method. Did basic repository functionality > testing on a cluster. > > > Thanks, > > Gwen Shapira > >
