-
Notifications
You must be signed in to change notification settings - Fork 466
Remove boto and pip installation from default installation #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good. How did you test them?
I didn't test the example. |
We can't run automated tests automatically for external PRs for security reasons. Could you run those yourself? Or, if that's too much setup work, could you deploy the example with the new AMI and make sure the cluster comes up OK? Thank you! |
The AWS IAM auth script (and thus the Boto requirement) is not even used at all for starting up a Vault cluster. Are you OK if I test using the IAM Auth example instead? I will see if I can make some more changes to only install Boto/Pip when needed for that example. |
Sure! As long as you're putting the new AMI through its paces and it works, I'd be confident in merging this. Thanks! |
Did a couple of tests:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the PR and thanks for testing it!
Fixes #116