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.

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

Merged
merged 7 commits into from
Mar 11, 2020

Conversation

derpferd
Copy link
Contributor

This closes issue #259

Copy link
Collaborator

@mpenkov mpenkov left a 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',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hanging indent here please.

@mpenkov
Copy link
Collaborator

mpenkov commented Jan 8, 2020

@derpferd Ping. Are you able to finish this PR?

@mpenkov mpenkov added the stale No recent activity from author label Jan 8, 2020
@derpferd
Copy link
Contributor Author

@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)
Copy link
Collaborator

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?

@mpenkov mpenkov self-assigned this Mar 8, 2020
@mpenkov mpenkov removed the stale No recent activity from author label Mar 8, 2020
@mpenkov mpenkov merged commit 68a39d9 into piskvorky:master Mar 11, 2020
@mpenkov
Copy link
Collaborator

mpenkov commented Mar 11, 2020

Merged. @derpferd Thank you for your effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants