Philip Zeyliger 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 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/13020/2/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/13020/2/bin/impala-config.sh@312 PS2, Line 312: export S3GUARD_METADATASTORE_IMPL="org.apache.hadoop.fs.s3a.s3guard.NullMetadataStore" I looked into whether there were other interesting impls here and couldn't find any. hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java doesn't work across processes. If we had something that went cross-process, we wouldn't really need Dynamo in our world. http://gerrit.cloudera.org:8080/#/c/13020/2/bin/jenkins/release_cloud_resources.sh File bin/jenkins/release_cloud_resources.sh: http://gerrit.cloudera.org:8080/#/c/13020/2/bin/jenkins/release_cloud_resources.sh@29 PS2, Line 29: # This is currently only implemented for s3. Consider asserting S3GUARD_DYNAMODB_TABLE and S3_BUCKET are non-empty... http://gerrit.cloudera.org:8080/#/c/13020/2/bin/jenkins/release_cloud_resources.sh@37 PS2, Line 37: aws s3 rm --recursive --quiet s3://${S3_BUCKET}${TEST_WAREHOUSE_DIR} This line and line 33 is useful enough that we should either do set +x or echo what we're about to do. http://gerrit.cloudera.org:8080/#/c/13020/2/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/13020/2/tests/common/impala_test_suite.py@177 PS2, Line 177: cls.filesystem_client = S3Client(S3_BUCKET_NAME) I think we could coalesce this to use HDFS/Hadoop always, thereby maybe removing the S3Client code? http://gerrit.cloudera.org:8080/#/c/13020/2/tests/common/impala_test_suite.py@178 PS2, Line 178: elif IS_ABFS: : # ABFS is implemented via HDFS command line client : cls.filesystem_client = HdfsCommandLineClient("ABFS") : elif IS_ADLS: : cls.filesystem_client = ADLSClient(ADLS_STORE_NAME) Do you know which, if any, of these are getting tested these days? http://gerrit.cloudera.org:8080/#/c/13020/2/tests/util/hdfs_util.py File tests/util/hdfs_util.py: http://gerrit.cloudera.org:8080/#/c/13020/2/tests/util/hdfs_util.py@149 PS2, Line 149: # This client is a wrapper around the hdfs command line. This is useful for filesystems Use pydoc? class Foo(bla): """bla bla bla""" -- 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: 2 Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Comment-Date: Mon, 29 Apr 2019 18:15:20 +0000 Gerrit-HasComments: Yes