You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Boglarka Egyed <bo...@apache.org> on 2018/10/17 15:29:35 UTC
Review Request 69063: SQOOP-3391: Test storing AWS credentials in
Hadoop CredentialProvider during import
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69063/
-----------------------------------------------------------
Review request for Sqoop, Fero Szabo, Ferenc Szabo, and Szabolcs Vasas.
Bugs: SQOOP-3391
https://issues.apache.org/jira/browse/SQOOP-3391
Repository: sqoop-trunk
Description
-------
Test storing AWS credentials in Hadoop CredentialProvider during import in case of
- CredntialProvider with default password
- CredntialProvider with password stored in environment variable
- CredntialProvider with password file
Added test cases for happy and sad paths as well.
Added a new test dependency both in Ant and Gradle for setting environment variables in tests easily.
Diffs
-----
build.gradle 7a0712e3242e31ef2593c34f469f9136cf5dc85d
build.xml f3975317140e66c700d85231669ccb2b70367f80
conf/password-file.txt PRE-CREATION
conf/wrong-password-file.txt PRE-CREATION
gradle.properties 4808ec7d090b9732f9246f21e44bd736adf6efd0
ivy.xml 91157ca74bee3b50269564ddb747638946e45a7e
ivy/libraries.properties 2ca95ee99c09fe1aaff6797a6ee0958ac1977663
src/java/org/apache/sqoop/util/password/CredentialProviderHelper.java 1d6481a0697db2fc0ffeb1b012bb143beb615bc0
src/test/org/apache/sqoop/s3/TestS3ImportWithHadoopCredProvider.java PRE-CREATION
src/test/org/apache/sqoop/testutil/S3TestUtils.java c9d17bc728d6a229e32c157b56268d6418b3de94
Diff: https://reviews.apache.org/r/69063/diff/1/
Testing
-------
./gradlew test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
ant clean test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
Thanks,
Boglarka Egyed
Re: Review Request 69063: SQOOP-3391: Test storing AWS credentials in
Hadoop CredentialProvider during import
Posted by Ferenc Szabo via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69063/#review209741
-----------------------------------------------------------
Ship it!
Ship It!
- Ferenc Szabo
On Oct. 18, 2018, 11:45 a.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69063/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2018, 11:45 a.m.)
>
>
> Review request for Sqoop, Fero Szabo, Ferenc Szabo, and Szabolcs Vasas.
>
>
> Bugs: SQOOP-3391
> https://issues.apache.org/jira/browse/SQOOP-3391
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Test storing AWS credentials in Hadoop CredentialProvider during import in case of
> - CredntialProvider with default password
> - CredntialProvider with password stored in environment variable
> - CredntialProvider with password file
> Added test cases for happy and sad paths as well.
>
> Added a new test dependency both in Ant and Gradle for setting environment variables in tests easily.
>
>
> Diffs
> -----
>
> build.gradle 7a0712e3242e31ef2593c34f469f9136cf5dc85d
> build.xml f3975317140e66c700d85231669ccb2b70367f80
> conf/password-file.txt PRE-CREATION
> conf/wrong-password-file.txt PRE-CREATION
> gradle.properties 4808ec7d090b9732f9246f21e44bd736adf6efd0
> ivy.xml 91157ca74bee3b50269564ddb747638946e45a7e
> ivy/libraries.properties 2ca95ee99c09fe1aaff6797a6ee0958ac1977663
> src/java/org/apache/sqoop/util/password/CredentialProviderHelper.java 1d6481a0697db2fc0ffeb1b012bb143beb615bc0
> src/test/org/apache/sqoop/s3/TestS3ImportWithHadoopCredProvider.java PRE-CREATION
> src/test/org/apache/sqoop/testutil/S3TestUtils.java c9d17bc728d6a229e32c157b56268d6418b3de94
>
>
> Diff: https://reviews.apache.org/r/69063/diff/2/
>
>
> Testing
> -------
>
> ./gradlew test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
> ant clean test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 69063: SQOOP-3391: Test storing AWS credentials in
Hadoop CredentialProvider during import
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/69063/#review209740
-----------------------------------------------------------
Ship it!
lgtm
- Fero Szabo
On Oct. 18, 2018, 11:45 a.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69063/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2018, 11:45 a.m.)
>
>
> Review request for Sqoop, Fero Szabo, Ferenc Szabo, and Szabolcs Vasas.
>
>
> Bugs: SQOOP-3391
> https://issues.apache.org/jira/browse/SQOOP-3391
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Test storing AWS credentials in Hadoop CredentialProvider during import in case of
> - CredntialProvider with default password
> - CredntialProvider with password stored in environment variable
> - CredntialProvider with password file
> Added test cases for happy and sad paths as well.
>
> Added a new test dependency both in Ant and Gradle for setting environment variables in tests easily.
>
>
> Diffs
> -----
>
> build.gradle 7a0712e3242e31ef2593c34f469f9136cf5dc85d
> build.xml f3975317140e66c700d85231669ccb2b70367f80
> conf/password-file.txt PRE-CREATION
> conf/wrong-password-file.txt PRE-CREATION
> gradle.properties 4808ec7d090b9732f9246f21e44bd736adf6efd0
> ivy.xml 91157ca74bee3b50269564ddb747638946e45a7e
> ivy/libraries.properties 2ca95ee99c09fe1aaff6797a6ee0958ac1977663
> src/java/org/apache/sqoop/util/password/CredentialProviderHelper.java 1d6481a0697db2fc0ffeb1b012bb143beb615bc0
> src/test/org/apache/sqoop/s3/TestS3ImportWithHadoopCredProvider.java PRE-CREATION
> src/test/org/apache/sqoop/testutil/S3TestUtils.java c9d17bc728d6a229e32c157b56268d6418b3de94
>
>
> Diff: https://reviews.apache.org/r/69063/diff/2/
>
>
> Testing
> -------
>
> ./gradlew test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
> ant clean test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 69063: SQOOP-3391: Test storing AWS credentials in
Hadoop CredentialProvider during import
Posted by Boglarka Egyed <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69063/
-----------------------------------------------------------
(Updated Oct. 18, 2018, 11:45 a.m.)
Review request for Sqoop, Fero Szabo, Ferenc Szabo, and Szabolcs Vasas.
Changes
-------
Clean up only HADOOP_CREDSTORE_PASSWORD environment variable in tests.
Bugs: SQOOP-3391
https://issues.apache.org/jira/browse/SQOOP-3391
Repository: sqoop-trunk
Description
-------
Test storing AWS credentials in Hadoop CredentialProvider during import in case of
- CredntialProvider with default password
- CredntialProvider with password stored in environment variable
- CredntialProvider with password file
Added test cases for happy and sad paths as well.
Added a new test dependency both in Ant and Gradle for setting environment variables in tests easily.
Diffs (updated)
-----
build.gradle 7a0712e3242e31ef2593c34f469f9136cf5dc85d
build.xml f3975317140e66c700d85231669ccb2b70367f80
conf/password-file.txt PRE-CREATION
conf/wrong-password-file.txt PRE-CREATION
gradle.properties 4808ec7d090b9732f9246f21e44bd736adf6efd0
ivy.xml 91157ca74bee3b50269564ddb747638946e45a7e
ivy/libraries.properties 2ca95ee99c09fe1aaff6797a6ee0958ac1977663
src/java/org/apache/sqoop/util/password/CredentialProviderHelper.java 1d6481a0697db2fc0ffeb1b012bb143beb615bc0
src/test/org/apache/sqoop/s3/TestS3ImportWithHadoopCredProvider.java PRE-CREATION
src/test/org/apache/sqoop/testutil/S3TestUtils.java c9d17bc728d6a229e32c157b56268d6418b3de94
Diff: https://reviews.apache.org/r/69063/diff/2/
Changes: https://reviews.apache.org/r/69063/diff/1-2/
Testing
-------
./gradlew test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
ant clean test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
Thanks,
Boglarka Egyed
Re: Review Request 69063: SQOOP-3391: Test storing AWS credentials in
Hadoop CredentialProvider during import
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69063/#review209737
-----------------------------------------------------------
Fix it, then Ship it!
Hi Bogi,
Thank you for submitting this test case!
I have successfully executed the tests and found one minor thing only.
After fixing that I think it would be good to go.
src/test/org/apache/sqoop/s3/TestS3ImportWithHadoopCredProvider.java
Lines 94 (patched)
<https://reviews.apache.org/r/69063/#comment294289>
This line seems to delete all the environment variables, which could affect other test cases if we run them in the same JVM.
Is this necessary or can we just clear the HADOOP_CREDSTORE_PASSWORD?
- Szabolcs Vasas
On Oct. 17, 2018, 3:29 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69063/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2018, 3:29 p.m.)
>
>
> Review request for Sqoop, Fero Szabo, Ferenc Szabo, and Szabolcs Vasas.
>
>
> Bugs: SQOOP-3391
> https://issues.apache.org/jira/browse/SQOOP-3391
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Test storing AWS credentials in Hadoop CredentialProvider during import in case of
> - CredntialProvider with default password
> - CredntialProvider with password stored in environment variable
> - CredntialProvider with password file
> Added test cases for happy and sad paths as well.
>
> Added a new test dependency both in Ant and Gradle for setting environment variables in tests easily.
>
>
> Diffs
> -----
>
> build.gradle 7a0712e3242e31ef2593c34f469f9136cf5dc85d
> build.xml f3975317140e66c700d85231669ccb2b70367f80
> conf/password-file.txt PRE-CREATION
> conf/wrong-password-file.txt PRE-CREATION
> gradle.properties 4808ec7d090b9732f9246f21e44bd736adf6efd0
> ivy.xml 91157ca74bee3b50269564ddb747638946e45a7e
> ivy/libraries.properties 2ca95ee99c09fe1aaff6797a6ee0958ac1977663
> src/java/org/apache/sqoop/util/password/CredentialProviderHelper.java 1d6481a0697db2fc0ffeb1b012bb143beb615bc0
> src/test/org/apache/sqoop/s3/TestS3ImportWithHadoopCredProvider.java PRE-CREATION
> src/test/org/apache/sqoop/testutil/S3TestUtils.java c9d17bc728d6a229e32c157b56268d6418b3de94
>
>
> Diff: https://reviews.apache.org/r/69063/diff/1/
>
>
> Testing
> -------
>
> ./gradlew test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
> ant clean test -Ds3.bucket.url=<bucket-url> -Ds3.generator.command=<credential-generator-command>
>
>
> Thanks,
>
> Boglarka Egyed
>
>