Conversation
by using generators, not only we can avoid using callbacks to parsing responses, making the code easier to read, but we also get to compose multiple http calls, by using `yield from`, which is necessary to rewrite `auth` into this style
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
apparently something is wrong with their ceritificates, and it is causing test failures both locally and in CI. commented out for now
There was a problem hiding this comment.
Pull request overview
Refactors Supabase Auth (and related clients) to reduce duplication between sync/async flows by centralizing HTTP execution and introducing a session manager, while updating call sites/tests to use the new APIs and header handling.
Changes:
- Introduces generator-based HTTP IO (
SyncHttpIO/AsyncHttpIO) and a newhandle_http_iodecorator to unify request execution across sync/async clients. - Adds new auth building blocks (
SessionManager, MFA client, Admin API) and updates Supabase clients/tests to usedefault_headersand typed parameter objects. - Updates dependencies/lockfile to consume
supabase_utilsand bumps versions accordingly.
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds editable supabase-utils dependency and bumps version. |
| src/utils/src/supabase_utils/http.py | Reworks request building + introduces generator-based sync/async HTTP IO loop and decorator. |
| src/supabase/src/supabase/client.py | Stops re-exporting legacy auth client types from supabase client surface. |
| src/supabase/src/supabase/init.py | Removes legacy auth client imports (but still lists names in __all__). |
| src/supabase/src/supabase/lib/init.py | Removes unused re-export. |
| src/supabase/src/supabase/_sync/client.py | Switches to supabase_auth.SyncSupabaseAuthClient and new header plumbing. |
| src/supabase/src/supabase/_async/client.py | Switches to supabase_auth.AsyncSupabaseAuthClient and new header plumbing. |
| src/supabase/tests/_sync/test_client.py | Updates assertions to default_headers and new executor plumbing. |
| src/supabase/tests/_async/test_client.py | Updates assertions to default_headers and new executor plumbing. |
| src/storage/src/storage3/client.py | Migrates Storage client to generator-based HTTP IO + default_headers. |
| src/storage/src/storage3/file_api.py | Migrates file APIs to generator-based HTTP IO + central validation helpers. |
| src/storage/src/storage3/vectors.py | Migrates vectors APIs to generator-based HTTP IO + central validation helpers. |
| src/storage/src/storage3/analytics.py | Migrates analytics APIs to generator-based HTTP IO + default_headers. |
| src/storage/src/storage3/exceptions.py | Adds validate_model/validate_adapter helpers that raise StorageApiError on failures. |
| src/storage/tests/test_client.py | Updates tests to check default_headers rather than httpx client headers. |
| src/storage/tests/_sync/test_client.py | Updates executor typing and error mocking to align with new IO. |
| src/storage/tests/_async/test_client.py | Updates executor typing and error mocking to align with new IO. |
| src/storage/tests/_async/conftest.py | Updates ctor usage to explicit headers=... kwarg. |
| src/functions/src/supabase_functions/client.py | Migrates Functions client to generator-based HTTP IO + default_headers. |
| src/functions/tests/test_client.py | Updates tests to assert default_headers. |
| src/functions/tests/_sync/test_function_client.py | Updates tests to assert default_headers. |
| src/functions/tests/_async/test_function_client.py | Updates tests to assert default_headers. |
| src/auth/src/supabase_auth/session.py | Adds new sync/async session manager implementation and storage abstractions. |
| src/auth/src/supabase_auth/mfa.py | Adds new sync/async MFA API client built atop shared HTTP IO. |
| src/auth/src/supabase_auth/admin_api.py | Adds new admin API client built atop shared HTTP IO. |
| src/auth/src/supabase_auth/timer.py | Splits timer into AsyncTimer and SyncTimer. |
| src/auth/src/supabase_auth/helpers.py | Reworks response parsing/error handling helpers and introduces redirect_to_as_query. |
| src/auth/src/supabase_auth/errors.py | Reworks auth error types and raw API error parsing. |
| src/auth/src/supabase_auth/init.py | Re-exports new auth surface (SupabaseAuthAdmin, session storage types). |
| src/auth/tests/test_helpers.py | Updates auth helper tests to new error handler entrypoint and payload format. |
| src/auth/tests/_sync/test_gotrue.py | Updates sync gotrue tests to new typed params + session manager plumbing. |
| src/auth/tests/_sync/test_gotrue_admin_api.py | Updates admin API tests to typed params + new admin client. |
| src/auth/tests/_sync/clients.py | Updates test client builders to new auth/admin client constructors. |
| src/auth/tests/_async/test_gotrue_admin_api.py | Updates async admin API tests to typed params + new admin client. |
| src/auth/tests/_async/clients.py | Updates async test client builders to new auth/admin client constructors. |
| src/auth/scripts/gh-download.py | Minor typing modernization (Optional -> `str |
| src/auth/pyproject.toml | Raises minimum Python to 3.10, adds supabase_utils, adds Ruff config, updates mypy Python version. |
| src/postgrest/tests/_sync/test_client.py | Comments out a test (coverage regression). |
| src/postgrest/tests/_async/test_client.py | Comments out a test (coverage regression). |
Comments suppressed due to low confidence (1)
src/utils/src/supabase_utils/http.py:90
JSONRequest.to_request()usesBaseModel.__pydantic_serializer__, which only exists in Pydantic v2. If this package is intended to support Pydantic v1 as well, this will raiseAttributeErrorat runtime. Either (a) tightensupabase_utils's dependency to Pydantic v2-only (it currently does in pyproject.toml), or (b) add a v1 fallback (e.g.,body.json()/pydantic_encoder) when__pydantic_serializer__is missing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ): | ||
| if self.refresh_token_timer: | ||
| self.refresh_token_timer.cancel() | ||
| self.refresh_token_timer = AsyncTimer( | ||
| (RETRY_INTERVAL ** (self.network_retries * 100)), | ||
| self.recover_and_refresh, | ||
| ) |
There was a problem hiding this comment.
The retry backoff uses RETRY_INTERVAL ** (self.network_retries * 100), which will explode to astronomically large delays after the first retry (and likely isn’t the intended exponential backoff). This looks like a math bug: consider a standard backoff like RETRY_INTERVAL * (2 ** (network_retries - 1)) (and ensure units are consistent with the timer expecting milliseconds).
9eb2717 to
09b0b4c
Compare
What kind of change does this PR introduce?
Refactor auth code to repeat a lot less code between sync and async versions.
I wasn't able to fully de duplicate all methods, like the storage and functions, because auth also has storage IO, and making it sans IO in both HTTP and Storage was significantly more challenging than just HTTP. To summarize the problem, python generators cannot express more than one type being yielded and sent back from the generator; and currently we assume all yielded objects are HTTP requests meant to be sent over the wire. In order to extend it, we'd need to introduce "storage events" representing the
get,setandremovecalls, but in doing that, we'd need to transform both the yielded types as well as the sent ones into unions, making the code a mess to type check. (I'm not entirely convinced that this approach is not possible, but I still don't see a clear way of extending generators to have multiple effect handlers, so I'll stick with the simpler approach for now.)Instead, I decided to de duplicate the HTTP calls (most of the libraries), and keep storage calls duplicated as explicit separate classes. As it turns out,
adminAPIs can be entirely described just by HTTP calls, so they're fine, but both the main client, as well asmfarelated APIs heavily rely on storage. For that, I created aSessionManager(similar to the swift implementation) which deals with most of storing, resetting, and getting the current session -- which also needed to have sync and async counterparts.Other than that, I removed all dict arguments, in favor of plain kwargs. In places where the argument was a union, I introduced proper objects with the correct serialization format, so that they can be directly sent over the wire. I also added helper methods to make it easier for users to use them; eg.
SignInWithEmailAndPasswordcan be created throughSignInWithPassword.email(...).