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

Reply via email to