Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/13020 )
Change subject: IMPALA-8344: Add support for running the minicluster with S3Guard ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/13020/4/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/13020/4/bin/impala-config.sh@341 PS4, Line 341: export S3GUARD_METADATASTORE_IMPL="org.apache.hadoop.fs.s3a.s3guard.NullMetadataStore" > Is this necessary? I think the default impl uses the NullMetadataStore alre Changed core-site.xml to use the python templates and pushed this in there. http://gerrit.cloudera.org:8080/#/c/13020/4/testdata/bin/load-test-warehouse-snapshot.sh File testdata/bin/load-test-warehouse-snapshot.sh: http://gerrit.cloudera.org:8080/#/c/13020/4/testdata/bin/load-test-warehouse-snapshot.sh@67 PS4, Line 67: hadoop s3guard prune -seconds 1 -meta "dynamodb://${S3GUARD_DYNAMODB_TABLE}" \ > is it necessary to prune a newly initialized S3Guard table? >From what I understand, hadoop s3guard init doesn't clear out a DynamoDB table >if it already exists. If somehow a test configuration aborts without calling >bin/jenkins/release_cloud_resources.sh (e.g. a Jenkins reboot), I prune this >to clear about anything left over from previous runs. http://gerrit.cloudera.org:8080/#/c/13020/4/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/13020/4/tests/common/impala_test_suite.py@a175 PS4, Line 175: > Should we delete the S3Client as well? It doesn't look like it is used anyw Good point, we can always revive it if we need it later. I removed it. I deferrred removing boto3 (but added a comment), because I think we are going to go through a cleanup when switching to toolchain python 2.7. http://gerrit.cloudera.org:8080/#/c/13020/4/tests/util/hdfs_util.py File tests/util/hdfs_util.py: http://gerrit.cloudera.org:8080/#/c/13020/4/tests/util/hdfs_util.py@158 PS4, Line 158: def __init__(self, filesystem_type="HDFS"): > the filesystem_type is just purely for debug messages right? might be usefu Added comment http://gerrit.cloudera.org:8080/#/c/13020/4/tests/util/hdfs_util.py@163 PS4, Line 163: hadoop_command = ['hadoop', 'fs'] + command > nit: hdfs instead of hadoop I renamed this class to the HadoopFsCommandLineClient. If there is a benefit to the hdfs command, we can switch, but I treat hadoop fs and hdfs dfs as mostly interchangeable, and this class is mainly for non-HDFS filesystems. http://gerrit.cloudera.org:8080/#/c/13020/4/tests/util/hdfs_util.py@170 PS4, Line 170: def create_file(self, path, file_data, overwrite=True): > nit: would be nice to have some docs for these methods, e.g. mention that c Added some function comments. -- To view, visit http://gerrit.cloudera.org:8080/13020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c748529a494bb6e70fec96dc031523ff79bf61d Gerrit-Change-Number: 13020 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Comment-Date: Tue, 21 May 2019 17:46:43 +0000 Gerrit-HasComments: Yes
