You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Szabolcs Vasas <va...@gmail.com> on 2018/11/20 17:29:53 UTC
Review Request 69413: Introduce methods instead of TEMP_BASE_DIR and
LOCAL_WAREHOUSE_DIR static fields
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69413/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-3407
https://issues.apache.org/jira/browse/SQOOP-3407
Repository: sqoop-trunk
Description
-------
BaseSqoopTestCase.TEMP_BASE_DIR and BaseSqoopTestCase.LOCAL_WAREHOUSE_DIR are public static fields which get initialized once at the JVM startup and store the paths for the test temp and warehouse directories.
The problem is that HBase test cases change the value of the test.build.data system property which can cause tests using these static fields to fail.
Since we do not own the code in HBase which changes the system property we need to turn these static fields into methods which evaluate the test.build.data system property every time they invoked which will make sure that the invoking tests will be successful.
Diffs
-----
src/test/org/apache/sqoop/TestIncrementalImport.java dbdd05c13e77af514bd996a92f7ebea3a27aedd5
src/test/org/apache/sqoop/TestMerge.java b283174b8b3df7c16c496795fcbae2f91dd1c375
src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java 9c1e9f9a93323655bc313303bf84d566b551ee00
src/test/org/apache/sqoop/hbase/HBaseImportAddRowKeyTest.java df1840b37ce29ffb303b31e1fcbfe4c5842e7c36
src/test/org/apache/sqoop/io/TestSplittableBufferedWriter.java 71d6971489e489ae501739fdad5a7409375b6ec1
src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerImportTest.java ea7942f62d623895f242e69e77cf9920bbb7e18c
src/test/org/apache/sqoop/orm/TestClassWriter.java 59a8908f13c51b9caca42e8602413ee0b8634b0a
src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java e23aad3ee997780e5708e9180550339d834b74d9
Diff: https://reviews.apache.org/r/69413/diff/1/
Testing
-------
Executed unit and third party tests.
Thanks,
Szabolcs Vasas
Re: Review Request 69413: Introduce methods instead of TEMP_BASE_DIR
and LOCAL_WAREHOUSE_DIR static fields
Posted by Fero Szabo via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69413/#review210754
-----------------------------------------------------------
Ship it!
Lgtm.
- Fero Szabo
On Nov. 20, 2018, 5:29 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69413/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2018, 5:29 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3407
> https://issues.apache.org/jira/browse/SQOOP-3407
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> BaseSqoopTestCase.TEMP_BASE_DIR and BaseSqoopTestCase.LOCAL_WAREHOUSE_DIR are public static fields which get initialized once at the JVM startup and store the paths for the test temp and warehouse directories.
>
> The problem is that HBase test cases change the value of the test.build.data system property which can cause tests using these static fields to fail.
>
> Since we do not own the code in HBase which changes the system property we need to turn these static fields into methods which evaluate the test.build.data system property every time they invoked which will make sure that the invoking tests will be successful.
>
>
> Diffs
> -----
>
> src/test/org/apache/sqoop/TestIncrementalImport.java dbdd05c13e77af514bd996a92f7ebea3a27aedd5
> src/test/org/apache/sqoop/TestMerge.java b283174b8b3df7c16c496795fcbae2f91dd1c375
> src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java 9c1e9f9a93323655bc313303bf84d566b551ee00
> src/test/org/apache/sqoop/hbase/HBaseImportAddRowKeyTest.java df1840b37ce29ffb303b31e1fcbfe4c5842e7c36
> src/test/org/apache/sqoop/io/TestSplittableBufferedWriter.java 71d6971489e489ae501739fdad5a7409375b6ec1
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerImportTest.java ea7942f62d623895f242e69e77cf9920bbb7e18c
> src/test/org/apache/sqoop/orm/TestClassWriter.java 59a8908f13c51b9caca42e8602413ee0b8634b0a
> src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java e23aad3ee997780e5708e9180550339d834b74d9
>
>
> Diff: https://reviews.apache.org/r/69413/diff/1/
>
>
> Testing
> -------
>
> Executed unit and third party tests.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 69413: Introduce methods instead of TEMP_BASE_DIR
and LOCAL_WAREHOUSE_DIR static fields
Posted by Boglarka Egyed <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69413/#review210755
-----------------------------------------------------------
Ship it!
Hi Szabolcs,
Thanks for this fix! Unit and 3rd party tests ran successfully for me.
Cheers,
Bogi
- Boglarka Egyed
On Nov. 20, 2018, 5:29 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69413/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2018, 5:29 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3407
> https://issues.apache.org/jira/browse/SQOOP-3407
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> BaseSqoopTestCase.TEMP_BASE_DIR and BaseSqoopTestCase.LOCAL_WAREHOUSE_DIR are public static fields which get initialized once at the JVM startup and store the paths for the test temp and warehouse directories.
>
> The problem is that HBase test cases change the value of the test.build.data system property which can cause tests using these static fields to fail.
>
> Since we do not own the code in HBase which changes the system property we need to turn these static fields into methods which evaluate the test.build.data system property every time they invoked which will make sure that the invoking tests will be successful.
>
>
> Diffs
> -----
>
> src/test/org/apache/sqoop/TestIncrementalImport.java dbdd05c13e77af514bd996a92f7ebea3a27aedd5
> src/test/org/apache/sqoop/TestMerge.java b283174b8b3df7c16c496795fcbae2f91dd1c375
> src/test/org/apache/sqoop/credentials/TestPassingSecurePassword.java 9c1e9f9a93323655bc313303bf84d566b551ee00
> src/test/org/apache/sqoop/hbase/HBaseImportAddRowKeyTest.java df1840b37ce29ffb303b31e1fcbfe4c5842e7c36
> src/test/org/apache/sqoop/io/TestSplittableBufferedWriter.java 71d6971489e489ae501739fdad5a7409375b6ec1
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerImportTest.java ea7942f62d623895f242e69e77cf9920bbb7e18c
> src/test/org/apache/sqoop/orm/TestClassWriter.java 59a8908f13c51b9caca42e8602413ee0b8634b0a
> src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java e23aad3ee997780e5708e9180550339d834b74d9
>
>
> Diff: https://reviews.apache.org/r/69413/diff/1/
>
>
> Testing
> -------
>
> Executed unit and third party tests.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>