-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/AWS CLI profile support for authentication #188
Feature/AWS CLI profile support for authentication #188
Conversation
Paul, I appreciate the contribution, but I'm a little confused about what this is trying to do. How is having awslimitchecker specify a profile name better than simply setting the |
Jason, At least in my own experience where I have many profiles established for various accounts and roles, I find the command line switch/option to be a better experience. While AWS CLI and boto3 both equally respect the environment variables, it provides this alternate means of defining profiles. That said, I'm not 100% positive this is the best method of implementation for this suggestion. It's simply how I've implemented it elsewhere for my own needs, and thought others may appreciate the functionality here. |
@@ -86,7 +86,14 @@ def _boto3_connection_kwargs(self): | |||
:rtype: dict | |||
""" | |||
kwargs = {'region_name': self.region} | |||
if self.account_id is not None: | |||
if self.profile_name is not None: |
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.
I'm a bit confused by these next few lines... if we're using a profile in one of the config files that Boto knows about, shouldn't we just have to pass in the profile name? I'm not clear on why we're reading the credentials instead of just setting kwargs['profile_name'] = self.profile_name
@paulysullivan the implementation seems right to me from a technical standpoint. The issue I'm really grappling with here is adding more code and another argument to a bunch of methods (not to mention the fixes to the tests, which aren't insignificant) just to be able to There's a part of me that considered just setting the environment variable internally from the command line argument. I suppose that on my end, there's also at least a slight expectation that anyone using awslimitchecker for more than one or two accounts will either be using the Python API instead of the command line wrapper, or will be wrapping it in some other automation... maybe something as simple as |
I've pull this in as the pr188 branch, and I'm going to try to rework it and get this released |
This has been released in 0.6.0 and is now live on PyPI |
Adds capability to authenticate against existing AWS CLI profiles.
Sorry for not including the tests.
Contributor License Agreement
By submitting this work for inclusion in awslimitchecker, I agree to the following terms:
is being made under the same license as the awslimitchecker project (the Affero GPL v3,
or any subsequent version of that license if adopted by awslimitchecker).
this pull request grants a perpetual, global, unlimited license for it to be used and distributed
under the terms of awslimitchecker's license.