feat(platform): support external token providers and simplify caching#513
Open
feat(platform): support external token providers and simplify caching#513
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for supplying an external bearer-token provider to the Platform Client, and refactors operation caching to key per-user via an explicit token_provider callable rather than relying on a global auth helper.
Changes:
- Introduces
_AuthenticatedApi+_OAuth2TokenProviderConfigurationin a new_api.pyto surfacetoken_provideras a top-level attribute (avoiding circular imports). - Updates
cached_operationto accepttoken_provider(replacing the previoususe_tokenflag) and wires it throughClientand resource modules. - Updates and expands unit tests to cover external token providers and the revised caching behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/aignostics/platform/_api.py |
Adds _AuthenticatedApi wrapper + configuration class to lift token_provider out of codegen internals. |
src/aignostics/platform/_client.py |
Adds token_provider parameter to Client, introduces external-provider singleton caching, and passes token_provider into cached_operation. |
src/aignostics/platform/_operation_cache.py |
Replaces use_token with token_provider in the caching decorator API and key generation. |
src/aignostics/platform/resources/runs.py |
Updates resource API type hints and passes token_provider=self._api.token_provider into cached operations. |
src/aignostics/platform/resources/applications.py |
Same as above for applications/versions resources. |
tests/aignostics/platform/client_token_provider_test.py |
Adds unit tests for external token providers and adjusts existing tests to the new API wiring. |
tests/aignostics/platform/client_cache_test.py |
Updates cache tests to use mocked token_provider for token-aware cache keys. |
tests/aignostics/platform/nocache_test.py |
Updates decorator calls to the new cached_operation signature. |
tests/aignostics/platform/conftest.py |
Ensures new external-client singleton cache is cleared between tests; adds token_provider to mocked API clients. |
tests/aignostics/platform/resources/runs_test.py |
Updates resource mocks to include token_provider/api_client attributes. |
tests/aignostics/platform/resources/applications_test.py |
Updates resource mocks to include token_provider/api_client attributes. |
Codecov Report❌ Patch coverage is
|
akunft
force-pushed
the
feat/enable_external_token_provider
branch
from
ab1533d to
b407d30
Compare
akunft
force-pushed
the
feat/enable_external_token_provider
branch
from
7e0a0b9 to
de2c81d
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
akunft
force-pushed
the
feat/enable_external_token_provider
branch
from
de2c81d to
7e3fac5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/aignostics/platform/resources/runs.py:137
Run.for_run_id()always constructs its API client viaClient.get_api_client(cache_token=...), so there is currently no way to use it with an externaltoken_provider(the new auth path). Consider adding an optionaltoken_providerparameter (and documentingcache_tokenas ignored when it’s provided) so this convenience constructor remains usable for M2M/service-account flows.
@classmethod
def for_run_id(cls, run_id: str, cache_token: bool = True) -> "Run":
"""Creates an Run instance for an existing run.
Args:
run_id (str): The ID of the application run.
cache_token (bool): Whether to cache the API token.
Returns:
Run: The initialized Run instance.
"""
from aignostics.platform._client import Client # noqa: PLC0415
return cls(Client.get_api_client(cache_token=cache_token), run_id)
olivermeyer
reviewed
Apr 1, 2026
Collaborator
|
I believe this impacts https://github.com/aignostics/python-sdk/blob/main/specifications/SPEC_PLATFORM_SERVICE.md, therefore it needs a CR? |
Contributor
Yes, I concur... -> SW requirement + SIS update is in order here :/ |
Collaborator
Author
Agreed. I will look into this, but only on Monday. |
Add a `token_provider` parameter to `Client.__init__()` so callers can supply their own bearer-token callable (e.g. for M2M / service-account flows) instead of relying on the built-in OAuth device-code flow. The operation cache needs a token to build per-user cache keys. Three approaches were explored: 1. **CachedApiMixin with Protocol** (explored, discarded) — A `_HasTokenProvider` Protocol plus `CachedApiMixin` base class that provided a `_cached()` convenience method. Saved one kwarg per call site but added multiple-inheritance, a Protocol, and competing `_api` annotations on every resource class. Over-engineering for what amounts to avoiding `token_provider=self._api.token_provider`. 2. **`TokenProvider` type alias** (explored, discarded) — `TokenProvider = Callable[[], str]` exported as public API. Added no value over the raw `Callable[[], str]` since every parameter is already named `token_provider`. Removed to avoid unnecessary imports and indirection. 3. **`_AuthenticatedApi` subclass + explicit `token_provider` at each call site** (chosen) — A thin `_AuthenticatedApi(PublicApi)` subclass in `_api.py` lifts `token_provider` from the deeply-nested codegen `Configuration` to a top-level attribute. Each cached method passes `token_provider=self._api.token_provider` to `@cached_operation(...)`. One kwarg of boilerplate per call site, but no new types, no inheritance, and trivially greppable. `_api.py` exists solely to break the circular import between `_client.py` and the resource modules.
akunft
force-pushed
the
feat/enable_external_token_provider
branch
from
f2296b9 to
262ac88
Compare
|
olivermeyer
reviewed
Apr 7, 2026
| | Output Type | Destination | Format/Type | Success Criteria | Code Location | | ||
| | ---------------- | ------------------- | ---------------------------- | --------------------------------------------------- | ---------------------------------------------------- | | ||
| | JWT Access Token | Token cache/memory | String | Valid JWT with required claims and unexpired | `_authentication.py::get_token()` return value | | ||
| | API Client | Client applications | `_AuthenticatedApi` object | Authenticated and configured for target environment | `_client.py::Client.get_api_client()` factory method | |
Collaborator
There was a problem hiding this comment.
Maybe should not be "private" (i.e. AuthenticatedApi) if it's part of the module outputs?
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
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.
Add a
token_providerparameter toClient.__init__()so callers can supply their own bearer-token callable (e.g. for M2M / service-account flows) instead of relying on the built-in OAuth device-code flow.