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.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-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 17 Nov 2017 19:59:53 +0000 Gerrit-HasComments: Yes
