-
-
Notifications
You must be signed in to change notification settings - Fork 381
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
Added credentials option for iter_bucket. #372
Conversation
This closes issue piskvorky#259
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.
Thank you for your contribution. I had a closer look and left you some comments.
Also, please avoid force-pushing during a review, because it destroys history, making it impossible for the reviewer to distinguish between new changes and changes they've already looked at in the past.
smart_open/s3.py
Outdated
def _list_bucket(bucket_name, prefix='', accept_key=lambda k: True, | ||
aws_access_key_id=None, aws_secret_access_key=None, | ||
aws_session_token=None): | ||
client = boto3.client('s3', |
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.
Hanging indent here please.
@derpferd Ping. Are you able to finish this PR? |
@mpenkov Sorry for the delay, made the formatting suggestion and changed to use key word args instead. |
smart_open/s3.py
Outdated
prefix='', | ||
accept_key=lambda k: True, | ||
**session_kwargs): | ||
client = boto3.client('s3', **session_kwargs) |
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.
You're passing session kwargs to the client, will that really work for all keywords?
Merged. @derpferd Thank you for your effort! |
This closes issue #259