Conversation
menshikh-iv
reviewed
Feb 10, 2021
Contributor
menshikh-iv
left a comment
There was a problem hiding this comment.
Awesome work @mpenkov 🔥
Just one note
menshikh-iv
approved these changes
Feb 10, 2021
Collaborator
Author
|
@piskvorky Can you please have a look at this? I'd like to merge and release so I can use this in one of my projects. |
Collaborator
Author
|
@piskvorky Ping |
piskvorky
reviewed
Feb 22, 2021
Co-authored-by: Radim Řehůřek <radimrehurek@seznam.cz>
Collaborator
Author
|
This is a breaking change. People doing stuff with kwargs will have to switch from using resource paramaters to using client ones. This will mean a major version bump. |
Owner
|
OK. And is this documented somewhere? A migration guide? |
Collaborator
Author
|
Good point. I added a section to the migration guide. Please have a look. |
piskvorky
requested changes
Mar 1, 2021
Co-authored-by: Radim Řehůřek <radimrehurek@seznam.cz>
Collaborator
Author
|
Updated https://github.com/RaRe-Technologies/smart_open/blob/client/MIGRATING_FROM_OLDER_VERSIONS.rst, please have a look. |
piskvorky
reviewed
Mar 1, 2021
piskvorky
approved these changes
Mar 1, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While functionally they are the same, the session/resource stuff is not safe to use across multiple threads and subprocesses, and the client stuff is. The former is a bit easier to use directly than the latter, but this does not impact the user, because smart_open is doing all the work.
I also refactored the way we pass keyword parameters to boto3. Previously, we had a separate parameter for each boto3 function. This was a pain, because this made parameter lists longer for each new function, e.g.
This PR moves all of the above parameters into a single nested dict and introduces a wrapper that transparently injects the parameters into the required function call.
This does break backwards compatibility, so will need a major version bump when releasing.