----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69433/#review210809 -----------------------------------------------------------
Hi Szabi, The whole change looks good to me, haven't spotted any mistakes, though still need to run tests. Just some questions to clarify my understanding of the change: I see the build.xml contains the default values for the connection strings. 1. How do these get picked up by the docker images? I'm guessing that I can see portforwarding in the yml that's the input for docker compose, this would answer it. 2. And how does gradle pick 'em up? I think this is why you've modified the util classes throughout Sqoop. Is that correct? So, what are the modifications in the build.xml needed for? build.xml Line 193 (original), 197 (patched) <https://reviews.apache.org/r/69433/#comment295568> I guess localhost could have stayed (just the port had to be added), or was there a problem with it? src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java Line 56 (original), 56 (patched) <https://reviews.apache.org/r/69433/#comment295569> uppercase / lowercase typo :) mysqlDbNAme > mysqlDbName - Fero Szabo On Nov. 22, 2018, 3:59 p.m., Szabolcs Vasas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69433/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2018, 3:59 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-3289 > https://issues.apache.org/jira/browse/SQOOP-3289 > > > Repository: sqoop-trunk > > > Description > ------- > > The patch includes the following changes: > - Changed the default DB connection parameters to Docker image defaults so > the test tasks can be started without specifying connection parameters > - Connection parameter settings duplications are removed > - Most of the JDBC drivers are downloaded from Maven repositories the only > exception is Oracle. Contributors have to upload ojdbc6.jar to a public drive > and make it available to the CI job by setting the ORACLE_DRIVER_URL in Travis > - Introduced separate test tasks for each databases > - An Oracle Express Edition Docker image is added to > sqoop-thirdpartytest-db-services.yml so Oracle tests which does not require > Oracle EE features can be executed much easier > - The ports for MySQL and PostgreSQL Docker containers are changed because > the default ones were used in the Travis VM already. > - Introduced OracleEe test category for tests requiring Oracle EE database. > These tests won't be executed on Travis. The good news is that only a few > tests require Oracle EE > > Documentation is still coming feel free to provide a feedback! > > > Diffs > ----- > > .travis.yml PRE-CREATION > build.gradle efe980d67 > build.xml a0e25191e > gradle.properties 722bc8bb2 > src/scripts/thirdpartytest/docker-compose/oraclescripts/ee-healthcheck.sh > PRE-CREATION > src/scripts/thirdpartytest/docker-compose/oraclescripts/healthcheck.sh > fb7800efe > > src/scripts/thirdpartytest/docker-compose/sqoop-thirdpartytest-db-services.yml > b4cf48863 > src/test/org/apache/sqoop/manager/cubrid/CubridTestUtils.java 4fd522bae > > src/test/org/apache/sqoop/manager/db2/DB2ImportAllTableWithSchemaManualTest.java > ed949b98f > src/test/org/apache/sqoop/manager/db2/DB2ManagerImportManualTest.java > 32dfc5eb2 > src/test/org/apache/sqoop/manager/db2/DB2TestUtils.java PRE-CREATION > src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java > 494c75b08 > src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java be205c877 > src/test/org/apache/sqoop/manager/oracle/ExportTest.java a60168719 > src/test/org/apache/sqoop/manager/oracle/ImportTest.java 5db9fe34e > src/test/org/apache/sqoop/manager/oracle/OraOopTestCase.java 1598813d8 > src/test/org/apache/sqoop/manager/oracle/OraOopTypesTest.java 1f67c4697 > src/test/org/apache/sqoop/manager/oracle/OracleConnectionFactoryTest.java > 34e182f4c > src/test/org/apache/sqoop/manager/oracle/TimestampDataTest.java be086c5c2 > src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 14b57f91a > > src/test/org/apache/sqoop/manager/postgresql/DirectPostgreSQLExportManualTest.java > 7dd6efcf9 > > src/test/org/apache/sqoop/manager/postgresql/PGBulkloadManagerManualTest.java > 1fe264456 > src/test/org/apache/sqoop/manager/postgresql/PostgresqlExportTest.java > eb798fa99 > > src/test/org/apache/sqoop/manager/postgresql/PostgresqlExternalTableImportTest.java > 8c3d2fd90 > src/test/org/apache/sqoop/manager/postgresql/PostgresqlTestUtil.java > e9705e5da > src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java bd12c5566 > src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerExportTest.java > ab1e8ff2d > src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerImportTest.java > 3c5bb327e > src/test/org/apache/sqoop/metastore/db2/DB2JobToolTest.java 81ef5fce6 > > src/test/org/apache/sqoop/metastore/db2/DB2MetaConnectIncrementalImportTest.java > 5403908e2 > src/test/org/apache/sqoop/metastore/db2/DB2SavedJobsTest.java b41eda110 > src/test/org/apache/sqoop/metastore/postgres/PostgresJobToolTest.java > 59ea151a5 > > src/test/org/apache/sqoop/metastore/postgres/PostgresMetaConnectIncrementalImportTest.java > afc6bd232 > src/test/org/apache/sqoop/metastore/postgres/PostgresSavedJobsTest.java > 9f9e865b9 > src/test/org/apache/sqoop/testcategories/thirdpartytest/OracleEeTest.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/69433/diff/1/ > > > Testing > ------- > > The testing was done in my own Sqoop fork with Travis: > https://travis-ci.org/szvasas/sqoop/builds/458464720 > > > Thanks, > > Szabolcs Vasas > >
