-
Notifications
You must be signed in to change notification settings - Fork 14
Add class methods wrapper #331
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
Merged
Merged
Conversation
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
I'm not sure why, but the exported classes don't seem to be the correct (subclassed) ones |
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.
This PR allows all functional methods, like
get
,head
,list
, to also be called as methods on each store class.I want obstore to define an "object storage interface spec" that allows applications to depend on an obstore-like interface, but which allows middlewares in between. So for example, metrics (#105) or caching (#247) could be implemented externally on the Python side. And a library depending on an obstore like interface wouldn't need to know anything about middlewares, as long as the input conforms to the correct protocol.
I started exploring these object storage interface spec ideas in #330. I think the API I'm considering is just a collection of protocols. E.g.
etc. Then a downstream application could pick and choose which APIs they require by doing
which would union the required methods.
But this obspec idea requires a class-based approach, with named functions that can match the given protocol methods.
Hence, this requires revisiting #160, to consider exposing methods on the Python based classes. I think revisiting this is worthwhile.
Change list
S3Store
_store
, so that the functional API supports both the non subclassed and subclassed variants.from_url
, i.e.S3Store.from_url
to respect subclasses. That is, usecls()
to construct the class instance.-> typing.Self
instead of the concrete class itself.from_url
method in Python, so thatobstore.store.from_url
can construct the subclassed versions of each class.parse_scheme
helper function exported from Rust to make sure we parse the URLs in the same way thatobject_store
does.register_store_module
function so that the user ofpyo3-object_store
can decide whether to register.store
or._store
There are a bunch of moving pieces here:
_ObjectStoreMixin
, which defines the method wrappers. So these methods are defined in terms of the pure functions.__module__
attribute points to the top-level Python class, not the core Rust class (does cloudpickle work too?, hopefully)Closes #160
cc @ion-elgreco