✨ Feat/async typed sdk - #225
YashGaykar0309 wants to merge 70 commits into
Conversation
GitHub Actions
Failure
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| super().__init__( | ||
| os.path.join(os.path.dirname(__file__), "resources/outscale.yaml"), **kwargs | ||
|
|
||
| class AsyncOpenAPIActionAPI(OpenAPIActionAPI): |
| self.close() | ||
|
|
||
|
|
||
| class AsyncOpenAPIPathAPI(OpenAPIPathAPI): |
| request: GetKubernetesVersionsRequest | None = None, | ||
| ) -> KubernetesVersionsResponse: | ||
| if request is None: | ||
| request = GetKubernetesVersionsRequest() |
| request: GetCPSubregionsRequest | None = None, | ||
| ) -> CPSubregionsResponse: | ||
| if request is None: | ||
| request = GetCPSubregionsRequest() |
| request: GetControlPlanePlansRequest | None = None, | ||
| ) -> ControlPlanesResponse: | ||
| if request is None: | ||
| request = GetControlPlanePlansRequest() |
| request: GetNetPeeringRequestTemplateRequest | None = None, | ||
| ) -> TemplateResponse_NetPeeringRequest: | ||
| if request is None: | ||
| request = GetNetPeeringRequestTemplateRequest() |
| request: GetNetPeeringAcceptanceTemplateRequest | None = None, | ||
| ) -> TemplateResponse_NetPeeringAcceptance: | ||
| if request is None: | ||
| request = GetNetPeeringAcceptanceTemplateRequest() |
| request: GetQuotasRequest | None = None, | ||
| ) -> quotas__quota_schema__QuotasResponse: | ||
| if request is None: | ||
| request = GetQuotasRequest() |
| request: GetClientIPRequest | None = None, | ||
| ) -> IPResponse: | ||
| if request is None: | ||
| request = GetClientIPRequest() |
| import asyncio | ||
| import copy | ||
| import os | ||
| import sys |
jobs62
left a comment
jobs62
left a comment
There was a problem hiding this comment.
Good start. Despite all comments i made, it whould be nice that public facing methods raise only "owned" exception. also string enum support whould be nice in the generator (that may need support for overlays as well)
| api_version: | ||
| description: 'Outscale API version' | ||
| required: true | ||
| oks_api_url: |
There was a problem hiding this comment.
Both services can have different release plan, so we should be able to build and release one without the other
There was a problem hiding this comment.
Updated the release workflow to support per-service builds. It can now build osc, oks, or all, and the release script only updates/regenerates the selected service.
There was a problem hiding this comment.
also please add struct ruff and ty check for the check workflow
|
|
||
|
|
||
| async def main(): | ||
| async with AsyncClient(profile="profile_1") as client: |
There was a problem hiding this comment.
we should have one client per service
| from datetime import datetime, timezone, timedelta | ||
| import asyncio | ||
| import time | ||
|
|
There was a problem hiding this comment.
same as authentication, limiter should be a httpx middleware
| @@ -0,0 +1,131 @@ | |||
| import asyncio | |||
| import json | |||
There was a problem hiding this comment.
should be a httpx middleware as well
| from ..request import RequestSpec | ||
|
|
||
|
|
||
| class AsyncCall(object): |
There was a problem hiding this comment.
you can probably get ride of Call/Requester. i never understood why we had that much boilerplate for parameters of transports (should be in the session or a middleware ?)
Updated README and docs examples to use Client / AsyncClient and async snake_case operation methods. Added unresolved path placeholder validation in RequestSpec with unit tests. Removed unnecessary IAM v2 secret injection from the CI workflow while keeping SDK credential support unchanged.
3c2fc69 to
3a96751
Compare
Quote OSC_TEST_PASSWORD in local-tests.sh so shell parsing preserves the full value with spaces.
Remove the Python 3.9 tox environment and GitHub Actions mapping because the package metadata requires Python 3.10 or newer.
Raise AttributeError for unknown dynamic OSC and OKS operation attributes so hasattr() reflects the generated operation lists instead of accepting any name. Add regression coverage for valid and invalid dynamic service attributes.
Update the README requirements to state Python 3.10+ so they match the package metadata.
Treat missing OSC_TLS_SKIP_VERIFY as unset instead of false so environment loading does not accidentally override tls_skip_verify from the credentials file.
| self.requests = [] | ||
| self._lock = Lock() | ||
|
|
||
| def acquire(self): |
There was a problem hiding this comment.
i don't like than acquire and async_acquire are basicly duplicated logic. it's bad taste. with only déférence being who the lock and sleep are handled.
| def __init__(self, message, *, request=None, response=None): | ||
| super().__init__(message) | ||
| self.request = request | ||
| self.response = response |
There was a problem hiding this comment.
could we have a helper to extract (at best effort) the request id that lead to failure ? we should also have a repl implémentation that print this information. that whould be a great help for customer support
There was a problem hiding this comment.
Added request ID extraction and exposed it as err.request_id; it also appears in exception string/repr for REPL/debug use.
| if response is not None: | ||
| if 400 <= response.status_code < 500 and response.status_code != 429: | ||
| return False | ||
| return attempt < self.max_retries |
There was a problem hiding this comment.
i know it was my code. but if the request have been sent, and error occure before we get an answer, retrying could be hazardous
There was a problem hiding this comment.
Retries now require an HTTP response, except for httpx.ConnectError, which is retried because the request was not established.
| ) | ||
|
|
||
|
|
||
| class SdkTransport(httpx.BaseTransport): |
There was a problem hiding this comment.
same as limiter. i don't like the fact that we have 2 class that a basicly duplicated.
jobs62
left a comment
jobs62
left a comment
There was a problem hiding this comment.
please type-check, format and order import of your code. setup precommit if you need to.
every call must be generated. other loading for openapi spec should not occure. codegen should not be shiped is source of the sdk. ruamel should not be needed at runtime anymore.
type-hint requirement was misunderstood. enforcing those is of course for us internally but also for developer experience down the line. LSP friendly code is a requirement
i poked at this version of the sdk in the unslop branch. you may the inspiration from it but it's not mergable.
| self.call.close() | ||
|
|
||
|
|
||
| class AsyncOpenAPIActionAPI(OpenAPIActionAPI): |
There was a problem hiding this comment.
dead code. call from raw openapi spec is a deprecated behaviour, all call should be generated those class have no use. AsyncClient is generated (good), but not the synchronous Client.
|
|
||
| # Bootstrap logic for generated mixins. | ||
| # This allows the SDK to load even if specific service code isn't generated yet. | ||
| try: |
There was a problem hiding this comment.
confusing for static checker. need to be guarded with typing.TYPE_CHECKING.
| schema_models: Iterable[Model], | ||
| package_name: str, | ||
| ) -> str: | ||
| schema_models = list(schema_models) |
There was a problem hiding this comment.
this is boundary violation. render_* should not have to deal with model deduplication or second model source. also it should be a dict, not a set + list
| + "\n\n" | ||
| + "from pydantic import BaseModel, ConfigDict, Field\n\n\n" | ||
| + "class GeneratedModel(BaseModel):\n" | ||
| + ' model_config = ConfigDict(populate_by_name=True, extra="allow")\n\n\n' |
There was a problem hiding this comment.
what is the use of model_config ?
| "", | ||
| "def _validate_response(model: type, value: Any) -> Any:", | ||
| " try:", | ||
| " return TypeAdapter(model).validate_python(value)", |
There was a problem hiding this comment.
too strong of a check for return value. we don't want to fail if obscure return field change. that can create BIG problems
| ) | ||
| (output_dir / "__init__.py").write_text( | ||
| render_init(operations, schema_models, package_name) | ||
| ) |
There was a problem hiding this comment.
any generated files MUST be formated and linted post-generation. it should not be the generator task to ensure that it geenrate formated code
| api_version: | ||
| description: 'Outscale API version' | ||
| required: true | ||
| oks_api_url: |
There was a problem hiding this comment.
also please add struct ruff and ty check for the check workflow
4 participants
Description
Implements the foundation for an async-first, OpenAPI-driven Python SDK with generated typed service clients. The architecture now supports multi-service expansion such as OSC/OAPI, OKS and future services through a modular core plus generated service layers.
Main goals:
Fixes: NA
Type of Change
Please check the relevant option(s):
How Has This Been Tested?
Please describe the test strategy:
Commands used (if applicable):
Checklist