Remove S3Store._from_native (Revert #203)#287
Merged
kylebarron merged 4 commits intomainfrom Feb 27, 2025
Merged
Conversation
kylebarron
changed the title
S3Store._from_nativeS3Store._from_native (Revert #203)
|
Yeah this makes more sense! Credential vending should be lightweight anyways, so not much penalty to keep it in python |
This was referenced Mar 7, 2025
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
2 participants
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.
Change list
S3Store._from_nativepickle_safeflag, since nowS3Storeis always pickle safe, as long as the Python credential provider is pickle-safe.Background
In version 0.4 in #203 we added
S3Store._from_native. This statically links to theaws-configcrateIt caused the size of the wheel to go from 3.4MB (8.22MB uncompressed) to 4.4MB (11.3MB uncompressed) on macOS. That's not huge on its own, but natively linking to GCS and Azure crates in the future would add additional bloat.
More importantly, however,
_from_nativeexpanded the scope of the library. Now that we support Python-based credential-providers (#234), we no longer need to manage all sorts of custom credential handling ourselves in Rust.We'll continue to support any credential handling that
object_storedoes natively, but we won't add any additional credential handling on the rust side ourselves.In practice, removing
_from_nativemainly means that disk-based credential handling no longer happens on the Rust side.cc @ion-elgreco