impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs
Date Fri, 17 Nov 2017 19:59:53 GMT
Philip Zeyliger 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 3: Code-Review+1

(3 comments)

I'm fine with this as-is; there are very minor nits that you can tackle if you'd like. Please
do answer the question about changing the FE tests to just use s3a so that we can remove the
hackery altogether.

Thanks!

http://gerrit.cloudera.org:8080/#/c/8294/3/bin/check-s3-access.sh
File bin/check-s3-access.sh:

http://gerrit.cloudera.org:8080/#/c/8294/3/bin/check-s3-access.sh@56
PS3, Line 56:   exit 0;
you don't need the semicolon


http://gerrit.cloudera.org:8080/#/c/8294/3/testdata/cluster/admin
File testdata/cluster/admin:

http://gerrit.cloudera.org:8080/#/c/8294/3/testdata/cluster/admin@289
PS3, Line 289:         sed '/<!-- BEGIN s3a security settings/,/END s3a security settings
-->/d' \
You could use "sed -i" to edit this in place, though I know Mac OS X sed behaves slightly
differently than Linux sed, causing some trouble. We seem to have some limited "sed -i" usage
already.

I'm not insisting.


http://gerrit.cloudera.org:8080/#/c/8294/3/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/3/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl@59
PS3, Line 59:   <!-- Fake s3n credentials required for FE tests -->
Is there any reason not to s/s3a/s3n/ in the tests so that we can get rid of this?



-- 
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: 3
Gerrit-Owner: Laszlo Gaal <laszlo.gaal@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <laszlo.gaal@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Nov 2017 19:59:53 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message