> On Nov. 22, 2018, 4:41 p.m., Fero Szabo wrote: > > 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?
1. Yes, the ports field of the service definition on the yml file defines which ports are exposed. 2. Gradle does not need the system properties to be defined in the build.gradle file the test will automatically pick up the default value in the System.getProperty call. And yes, I modified the default values in all of the System.getProperty calls to return the hosts and ports defined in the docker compose file. 3. Ant requires the system properties to be defined in build.xml so I had to modify the database default host and port values there too. > On Nov. 22, 2018, 4:41 p.m., Fero Szabo wrote: > > build.xml > > Line 193 (original), 197 (patched) > > <https://reviews.apache.org/r/69433/diff/1/?file=2109980#file2109980line197> > > > > I guess localhost could have stayed (just the port had to be added), or > > was there a problem with it? Yes, I remember there was a strange issue with localhost and MySQL direct imports, I don't know how but 127.0.0.1 solved it. - Szabolcs ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69433/#review210809 ----------------------------------------------------------- On Nov. 23, 2018, 10:33 a.m., Szabolcs Vasas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69433/ > ----------------------------------------------------------- > > (Updated Nov. 23, 2018, 10:33 a.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 > COMPILING.txt b399ba825 > 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/2/ > > > Testing > ------- > > The testing was done in my own Sqoop fork with Travis: > https://travis-ci.org/szvasas/sqoop/builds/458464720 > > > Thanks, > > Szabolcs Vasas > >
