Skip to content
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.

Sign up for GitHub

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

Add ability to pass parameters to the botocore session. #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add ability to pass parameters to the botocore session. #52

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5458d66
Add ability to use pass parameters to the botocore session.
Jan 3, 2020
5652599
Add `profile` as an optional parameter, in response to feedback.
Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions aws_requests_auth/boto_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def get_credentials(credentials_obj=None):

class BotoAWSRequestsAuth(AWSRequestsAuth):

def __init__(self, aws_host, aws_region, aws_service):
def __init__(self, aws_host, aws_region, aws_service, session_params={}):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the pull request @dashstander . Mutable default kwargs are a bit of a Python gotcha e.g.

In [1]: def f(l=[]): 
   ...:     l.append(1) 
   ...:     print(l) 
   ...:                                                                                                                               

In [2]: f()                                                                                                                           
[1]

In [3]: f()                                                                                                                           
[1, 1]

In [4]: f()                                                                                                                           
[1, 1, 1]

In [5]: f()                                                                                                                           
[1, 1, 1, 1]

In [6]: f()                                                                                                                           
[1, 1, 1, 1, 1]

The preferred pattern (as described by core dev Raymond Hettinger) looks something like...

def f(l=None):
    if l is None:
        l = []

allows things like passing in regions, profiles, etc.

Since aws_region is "hardcoded" in BotoAwsRequestsAuth and AWSRequestsAuth, I do not think aws_region will be an appropriate target for us to dynamically override.

I'm not sure if this is true, but does the boto3 profile_name kwarg (e.g. boto3.session.Session(profile_name="my-profile")) restrict itself to only affectingaws_access_key_id, aws_secret_access_key, aws_session_token?

If profile_name indeed only can possibly affect auth, then we might consider allowing profile_name (and only profile_name) to be optionally provided instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the response @DavidMuller , this is the first time I've made a PR against a public repo.

Having the default be mutable was definitely an oversight on my part and I can change the PR.

According to the docs I'm looking at, there are other parameters (including aws_region) that can have defaults configured by different profiles. But I don't think that that would interfere with having it aws_region hardcoded. The Session().get_credentials() call won't ever pull anything except the aws_access_key_id, aws_secret_access_key, aws_session_token, so there's no danger of the aws_region getting overridden or anything like that.

Which is all another way of saying that adding an optional profile_name parameter works for me as well.

"""
Example usage for talking to an AWS Elasticsearch Service:

Expand All @@ -44,7 +44,7 @@ def __init__(self, aws_host, aws_region, aws_service):
http://boto3.readthedocs.io/en/latest/guide/configuration.html#configuring-credentials
"""
super(BotoAWSRequestsAuth, self).__init__(None, None, aws_host, aws_region, aws_service)
self._refreshable_credentials = Session().get_credentials()
self._refreshable_credentials = Session(**session_params).get_credentials()

def get_aws_request_headers_handler(self, r):
# provide credentials explicitly during each __call__, to take advantage
Expand Down