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 3: (3 comments) > I also don't want to block progress on making this good improvement > to our S3 development, but my fear is that this script will get > more and more out of control if we don't draw a line somewhere > about what belongs in it. That's a valid point. I moved the logic to bin/check-s3-access.sh, which actually lets us reuse the the same check elsewhere if it becomes necessary or useful. http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh@294 PS2, Line 294: export LOCAL_FS="file:${WAREHOUSE_LOCATION_PREFIX}" > Yeah, I'd question whether "aws ls" belongs in this script either. Done, moved to bin/check-s3-access.sh. http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh@303 PS2, Line 303: set AWS_A > This variable will leak out into the user's shell. Done, I moved these checks into a separate script, check-s3-access.sh http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh@307 PS2, Line 307: > Good point; I'll check if wget can be set up the same way. Done. Although the check was moved to bin/check-s3-access.sh, I have replaced curl with wget, using equivalent parameters for silencing and short timeouts. -- 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: Mon, 13 Nov 2017 22:26:57 +0000 Gerrit-HasComments: Yes
