----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37793/#review96542 -----------------------------------------------------------
Thanks for the patch Dian! Couple of high level points: 1) Can we add tests for the new functionality? repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (lines 100 - 126) <https://reviews.apache.org/r/37793/#comment151989> It seems to very high extent copy&pasted code from the findConnector(String, Connection) method. I"m wondering if we can do slightly better and somehow consolidate those method? Perhaps by doing findConnectorInternal(String query, Connection) that will run entire code here with small "wrappers" on top that will just supply the "right" query? repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java (lines 40 - 50) <https://reviews.apache.org/r/37793/#comment151991> Similarly for the queries - let's perhaps do base "SELECT_FROM_CONFIGURABLE" query without the WHERE clause and use that to define the additional queries? I do not insist on consolidating the queries, so just an idea here. Jarcec - Jarek Cecho On Aug. 26, 2015, 9:30 a.m., Dian Fu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37793/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2015, 9:30 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2530 > https://issues.apache.org/jira/browse/SQOOP-2530 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > We need add some utility methods to translate connectorId/linkId/jobId to the > corresponding connectorName/linkName/jobName. > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java > 9f4a0f8 > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9a4853b > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > f690887 > core/src/main/java/org/apache/sqoop/repository/Repository.java 610876c > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java > af9324f > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java > 4f9e547 > server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 5fcde52 > server/src/main/java/org/apache/sqoop/server/common/ServerError.java > c68ab57 > > Diff: https://reviews.apache.org/r/37793/diff/ > > > Testing > ------- > > > Thanks, > > Dian Fu > >
