Laszlo Gaal has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8294 )

Change subject: IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh@240
PS1, Line 240: #export AWS_SECRET_ACCESS_KEY="${AWS_SECRET_ACCESS_KEY-}"
             : #export AWS_ACCESS_KEY_ID="${AWS_ACCESS_KEY_ID-}"
> Self-inflicted review finding: remove commented-out lines.
Done


http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh@256
PS1, Line 256: if [[ -z ${AWS_ACCESS_KEY_ID-} ]]; then
> (Bash hackery)
Thanks for the tip, Philip, I ended up using it.


http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh@268
PS1, Line 268: test
> tests
Done


http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh@276
PS1, Line 276: curl -sf --connect-timeout 1 --max-time 5
> Are these curl options common across a wide variety of OSs? If they are new
Yes. I have tested the curl options on all the platforms on which the packaging 
build runs. Curl understands these options on all these platforms.


http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh@276
PS1, Line 276: http://169.254.169.254/latest/meta-data/iam/security-credentials/
> Is there a AWS reference page you can add as a comment so I can read up mor
Done


http://gerrit.cloudera.org:8080/#/c/8294/1/bin/impala-config.sh@276
PS1, Line 276: 169.254.169.254
> http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.ht
I have put the URL to the Amazon doc page where this mechanism is described.


http://gerrit.cloudera.org:8080/#/c/8294/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:

http://gerrit.cloudera.org:8080/#/c/8294/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3000
PS1, Line 3000:       AnalysisError(String.format("load data inpath '%s' %s 
into table " +
              :           "tpch.lineitem", 
"s3a://bucket/test-warehouse/test.out", overwrite),
              :           "INPATH location 
's3a://bucket/test-warehouse/test.out' must point to an " +
              :           "HDFS, S3A or ADL filesystem.");
> Impala cannot load data from s3n. I think this test is intended to verify o
That's right; I was pattern-matching too eagerly. Thanks for catching this.
I have in fact reverted all the s3n->s3a changes in the FE tests; looks like 
they don't in fact touch S3 and they are OK with some existing but obviously 
fake credentials in core-site.xml.


http://gerrit.cloudera.org:8080/#/c/8294/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl
File testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl:

http://gerrit.cloudera.org:8080/#/c/8294/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl@61
PS1, Line 61: <!-- BEGIN s3 security settings -->
> Maybe have a comment explaining testdata/cluster/admin needs this (and the
Done,

referred the Amazon doc site as well.



--
To view, visit http://gerrit.cloudera.org:8080/8294
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 1
Gerrit-Owner: Laszlo Gaal <laszlo.g...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Nov 2017 15:24:18 +0000
Gerrit-HasComments: Yes

Reply via email to