feat(storage): refactor sync and async methods - #1364
Merged
Conversation
this way, we only need to write the whole client once, and we can derive both IO implementations from a single source of truth, reducing the amount of code needed for each package in half.
lets start already very strict, so that we don't have to strict it up later.
this is because, inside the `http_endpoint` class definition, we have the equivalent of the following assignment: ``` s : ServerEndpoint[Success, Failure] a : ServerEndpoint[Awaitable[Success] | Success, Failure] = s ``` as such, we need to make `Awaitable[Success] | Success` a subtype of `Success`, which means that it needs to be covariant!
I'm currently experimenting with other type checkers, and seeing if they agree on the type stuff done in http. pyrefly accepts all of it, but complains about the TypeAlias annotation in the json part. it isnt necessary so I just removed it.
… TypedDicts this is a partial commit, and is not fully working yet.
this ideally makes it more general, and lets endpoints specify how they want to send information through in multiple ways. this is needed to handle the case where we send bytes through edge functions directly addionally, we're not using json.loads anymore, and relying solely on pydantic's `to_json` machinery, which should be miles faster (written in rust!)
the type ignore is useful for python < 3.11, but mypy doesnt know that
rename classes with `endpoint` in their name. given that this is a client, and not a server, these might not make sense the initial idea was that these were supposed to hit a 'server endpoint', but I think that the name is confusing after trying to use it elsewhere i've also split the one request class into multiple, because it makes more sense in the context of the other packages
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
o-santi
force-pushed
the
storage/sync-async-refactor
branch
from
February 2, 2026 19:25
09438c4 to
9d5a31f
Compare
grdsdev
approved these changes
Feb 4, 2026
this gives us access to supabase-cli v2.75.0, which contains the fix for a small storage-api bug
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.
What kind of change does this PR introduce?
Rewrite
storageto also usesupabase_utils.http, removing the need for_syncand_async.Breaking changes
This PR contains some breaking changes.
TypedDicthave been inlined as method kwargs. This makes the logic for handling them easier, and also makes the type checkers happier. The ones that were 2 levels deep (eg.TransformOptions) were transformed intopydantic.dataclasses.dataclassobjects, so that we have type validation on these inputs.BaseModels.Mixinclasses, for there isn't a clear reason for not just inlining them in the original class, and this makes the codebase easier to read overall.pathfield, which was actually just one of the original arguments to said function. I just removed those, for it is confusing, and it may seem to the user that the returned path is somehow different/special.mypyis nowstrict! No moreAny.Optional,Unions and so on.