----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9808/#review17647 -----------------------------------------------------------
Hi Abe, thank you very much for working on this. The patch looks good to me. I have just couple of comments: 1) Would you mind removing trailing white space characters? I've highlighted all of them for your convenience. 2) Would you mind adding Validation check in GenericJdbcValidator to ensure that the schema is not used with query based import? 3) I'm seeing failure in integration test TableImportTest, but that will be fixed by SQOOP-919, so it's not a show stopper. Jarcec connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java <https://reviews.apache.org/r/9808/#comment37426> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java <https://reviews.apache.org/r/9808/#comment37427> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java <https://reviews.apache.org/r/9808/#comment37428> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java <https://reviews.apache.org/r/9808/#comment37429> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37430> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37431> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37432> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37433> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37434> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37435> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java <https://reviews.apache.org/r/9808/#comment37436> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java <https://reviews.apache.org/r/9808/#comment37437> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java <https://reviews.apache.org/r/9808/#comment37438> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java <https://reviews.apache.org/r/9808/#comment37439> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java <https://reviews.apache.org/r/9808/#comment37440> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java <https://reviews.apache.org/r/9808/#comment37441> Nit: Trailing whitespace characters connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java <https://reviews.apache.org/r/9808/#comment37442> Nit: Trailing whitespace characters - Jarek Cecho On March 7, 2013, 11:46 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9808/ > ----------------------------------------------------------- > > (Updated March 7, 2013, 11:46 p.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Description > ------- > > commit b3b188e1b7a8fadf981ddaef4ffb8ec3676601dd > Author: Abraham Elmahrek <[email protected]> > Date: Thu Mar 7 15:26:21 2013 -0800 > > SQOOP-903 Add schema support to generic JDBC connector > > Schema is optional and job contexts are updated with proper table names. > New tests for inclusion of schema and exclusion of schema. > > :100644 100644 520b0bb... 6ee9a83... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java > :100644 100644 46c7ee7... 7bc0972... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java > :100644 100644 718d1fb... ee4bb6e... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java > :100644 100644 d150779... 3422a8f... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java > :100644 100644 6ab4296... 44fc984... M > connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties > :100644 100644 bb0c23b... a73f3e8... M > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java > :100644 100644 45835bd... 778fe28... M > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java > > > This addresses bug SQOOP-903. > https://issues.apache.org/jira/browse/SQOOP-903 > > > Diffs > ----- > > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java > 520b0bb > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java > 46c7ee7 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java > 718d1fb > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java > d150779 > > connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties > 6ab4296 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java > bb0c23b > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java > 45835bd > > Diff: https://reviews.apache.org/r/9808/diff/ > > > Testing > ------- > > Created new test cases for testing Import and Export initialization in > Generic JDBC Connector. > Tested import job with postgresql and schemas manually. > > > Thanks, > > Abraham Elmahrek > >
