-
-
Notifications
You must be signed in to change notification settings - Fork 367
Description
v3.0 Breaking Changes
This issue is intended to track and discuss all breaking changes to come in v3.0
before landing them in master, in order to not only evolve ideas but also see how impactful they could be to existing code bases.
1. Single way to configure parameters for requests.
Since the introduction of httpx clients as parameters in #1117, it is possible to configure the same parameter in different ways, which not only introduces complexity in merging parameters internally, but also have complicated non-trivial semantics. For instance, httpx clients only support configuration through mutation, which makes sharing them cause troubles (see #1244, #1249).
Instead, I believe that supabase should offer its own way of configuring parameters to requests in a single unified class, shared by all packages of the monorepo, even if there's duplication with httpx parameters, as its the only sane way of not duplicating while still retaining the correct immutable semantics, for sharing configuration between different clients.
2. Stop using dict
/TypedDict
for everything
Of course, dict
is useful in some places, but they should absolutely not be used as parameters to functions, like configuration options, as they do are not good for handling/manipulation. TypedDict
s are mostly syntax sugar, do not offer validation, and do not handle missing keys at all (total=False
just implies all keys are Optional
, ie. accept None
). Furthermore, they're generally mutable, which is not great for typing and sharing.
Instead, dedicated structures should be in place to set up options. pydantic.BaseModel
are strictly better alternatives for they handle missing keys correctly, handle validation, field accessing instead of indexing (a.b
vs a["b"]
), and do not have invariance issues (see this stackoverflow answer for more info).
3. Async vs Sync
Async vs Sync is still going to be a problem, but I think that they should be a much lesser problem than it is needed currently. Currently, all the libraries do is offer pretty and simple to use wrappers to HTTP requests. (Realtime is an outlier in this case but the general argument still applies.) The only concrete difference in how sync vs async is handled is in how the final step of sending the HTTP request, but I wanna make the point that it shouldn't really be Supabase's concern anyway. There's nothing special about httpx
's client that make it crucial for supabase-py, other than it being a particularly nice and featureful implementation of an HTTP client.
What the supabase SDK should really strive to be is a middleware for generating the request parameters (URL, query parameters, headers, etc), and parsing responses from the server, both of these logics are synchronous in general (and internally already is, but most of it is hidden inside async functions). Then, one should be able to plug the requests into his favorite HTTP client and send it away with it, be it synchronously or asynchronously, and pass the response back to a parser that interprets the result. With this view, it is very clear there's no reason for the core logic to have this duality.
I believe that the main reason for the code being so complicated currently is due to a lack of distinction between request parameters, request dispatching, and result parsing, for most of the 'niceties' functions generally try to do all of these at once. However, if it was properly split up, it would be far simpler in general, for we would not need to have to duplicate most of the code. Take for instance the following example:
async def create_signed_upload_url(self, path: str) -> SignedUploadURL:
path_parts = relative_path_to_parts(path)
response = await self._request(
"POST", ["object", "upload", "sign", self.id, *path_parts]
)
data = response.json()
full_url: urllib.parse.ParseResult = urllib.parse.urlparse(
str(self._client.base_url) + cast(str, data["url"]).lstrip("/")
)
query_params = urllib.parse.parse_qs(full_url.query)
if not query_params.get("token"):
raise StorageException("No token sent by the API")
return {
"signed_url": full_url.geturl(),
"signedUrl": full_url.geturl(),
"token": query_params["token"][0],
"path": path,
}
It is not very clear but it does follows the general structure described earlier:
async def create_signed_upload_url(self, path: str) -> SignedUploadURL:
request = self.prepare_signed_upload_url_request(path)
response = await do_request(request)
return parse_signed_upload_response(response)
At which point, it is clear that the function could be written as 3 separate parts, and that the do_request
part is the only one that needs to be sync/async, and the only one that depends on httpx
in general. Thus, if the library was more structured like this - with prepare_request
and parse_response
written only once in non async/sync dependent code - it would be far far simpler overall, as the async/sync parts would be way smaller and leaner. Furthermore, it would be fairly trivial for a third party to plug their own HTTP client, as they could manually invoke prepare_request
and parse_response
and do the plumbing themselves.
This approach in general is called Sans-IO and I believe that it is the approach we should follow for the v3.0 release. For more examples, see https://sans-io.readthedocs.io/ and https://github.com/python-hyper/h11. This also relates to topic 1, of stopping relying so heavily on httpx
configuration schemes.