----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68677/#review208509 -----------------------------------------------------------
Hi Bogi, I have taken a look at the patch and I think that this duplication is unfortunately needed because the Sqoop import does not pick up the Hadoop configuration which is created at the beginning of the test. It is misleading that the tests are still passing but that happens only because FileSystem has a static cache, so once the authentication happens in the setup() method the Sqoop import just gets the same FileSystem object from the cache so it does not have to authenticate again. (see: org.apache.hadoop.fs.FileSystem#get(java.net.URI, org.apache.hadoop.conf.Configuration)). I think we should rewrite this patch to set fs.s3a.impl.disable.cache property to true on the Hadoop conf created in the setup() to make sure Sqoop will rely on the S3 parameters specified in the site.xml or in -D properties. - Szabolcs Vasas On Sept. 10, 2018, 1:22 p.m., Boglarka Egyed wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68677/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2018, 1:22 p.m.) > > > Review request for Sqoop, daniel voros, Fero Szabo, and Szabolcs Vasas. > > > Bugs: SQOOP-3383 > https://issues.apache.org/jira/browse/SQOOP-3383 > > > Repository: sqoop-trunk > > > Description > ------- > > In the current S3 test suite the setup phase includes a logic which sets the > AWS credentials in the Hadoop configuration in the > org.apache.sqoop.testutil.S3TestUtils#setS3CredentialsInHadoopConf method. > These credentials then are set via system properties calling the > org.apache.sqoop.testutil.S3TestUtils#getArgumentArrayBuilderForS3UnitTests > method too. > > This is an unnecessary duplication that should be cleaned up to ease further > test implementation. > > > Diffs > ----- > > src/test/org/apache/sqoop/s3/TestS3AvroImport.java > 7f5f5d62c5cab10f932aa22c3a713b13fefc2b58 > src/test/org/apache/sqoop/s3/TestS3IncrementalAppendAvroImport.java > 5faf59ea80c48fe025294cabd100e7d176032138 > src/test/org/apache/sqoop/s3/TestS3IncrementalAppendParquetImport.java > a4f986423ea299716a29f9d02f7c8453a7f2ba02 > src/test/org/apache/sqoop/s3/TestS3IncrementalAppendSequenceFileImport.java > d271588c5af060bbc3d301a845f45c46d0f6a2ba > src/test/org/apache/sqoop/s3/TestS3IncrementalAppendTextImport.java > 52d89c775b5f1219471df44d222fd92a59ed408c > src/test/org/apache/sqoop/s3/TestS3IncrementalMergeParquetImport.java > 39238c5fab56b54a85dde5aed0d4bb2c77382fa6 > src/test/org/apache/sqoop/s3/TestS3IncrementalMergeTextImport.java > 597e3def2cc33adebeeb3bc1ee35ad8a7f4b990d > src/test/org/apache/sqoop/s3/TestS3ParquetImport.java > c9785d816d4a7a5870d74c51a9faa229f6d3818e > src/test/org/apache/sqoop/s3/TestS3SequenceFileImport.java > bba8b74ebe639df26e977abf377f4904144dcfaa > src/test/org/apache/sqoop/s3/TestS3TextImport.java > 114f97cbb8857a7633cae5d030769ac4a90e36aa > src/test/org/apache/sqoop/testutil/S3TestUtils.java > 7724026b0bad25a31aa76c89135a51538b46bf82 > > > Diff: https://reviews.apache.org/r/68677/diff/1/ > > > Testing > ------- > > ant clean test -Ds3.bucket.url=<bucket-url> > -Ds3.generator.command=<credential-generator-command> > > > Thanks, > > Boglarka Egyed > >
