Skip to content

Commit c8ff4e7

Browse files
authored
feat(models): add typed NemoClient models foundation (#993)
* feat(models): add typed NemoClient models foundation Introduces the typed Models service client that the AIRCORE-876 consumer migration will build on: request/response DTOs (types), PreparedRequest endpoint builders (endpoints), and the sync/async ModelsClient surface (client). Purely additive: no existing code imports it yet, so it changes no runtime behavior and carries zero risk to current consumers. Also carries the method() descriptor fix that this client requires -- class-level attribute access now resolves without invoking the wrapped callable, so Mock(spec=ModelsClient) and other introspection no longer break -- plus a response docstring note on distinguishing 202/204 deletes. The consumer repoint, the packages/models resources rewrite, and the vendored SDK sync land separately as the breaking, coupled steps. Covered by endpoint-builder, client-surface, and descriptor tests. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(models): async-mockable client, non-stale delete status, guarded provider id Address review findings on the typed Models client foundation: - method() class-level access returns a per-owning-class callable stub (async def for async clients, def for sync) instead of the raw descriptor, so unittest.mock classifies async endpoints as AsyncMock. Previously Mock(spec=AsyncModelsClient).create_model was a sync MagicMock and could not be awaited, and create_autospec yielded non-callable stubs -- defeating the typed async client's purpose. - delete_deployment appends a DELETING entry to status_history so the client (which reads status_history[-1] as current) no longer sees a stale status after a delete request. - get_provider_route_openai_url_for_deployment guards a model_provider_id that lacks the workspace/ prefix with a clear ValueError instead of an opaque unpack crash. Adds regression tests for all three. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * test(models): use top-level imports instead of dynamic __import__ Replace two call-time __import__("...types", fromlist=[...]) lookups in the endpoint tests with normal names added to the existing top-level import block (CreateModelAdapterRequest, UpdateModelDeploymentConfigRequest). Addresses a CodeRabbit maintainability nit on PR #993. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(models): ruff-format client.py and suppress class-level ty errors CI Lint all caught two issues from the earlier commits: - ruff format collapses the malformed-model_provider_id message onto one line in client.py. - ty reports call-non-callable / invalid-argument-type on the new class-level-access tests: EndpointMethod.__get__(obj=None) is typed as the descriptor because the overload cannot distinguish the sync vs async owning class, so ty cannot see the callable stub returned at runtime. The three tests deliberately exercise that runtime stub, so they carry targeted ty: ignore suppressions with an explanatory note. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> --------- Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
1 parent 725ad48 commit c8ff4e7

10 files changed

Lines changed: 3298 additions & 1 deletion

File tree

‎packages/nemo_platform_plugin/src/nemo_platform_plugin/client/method.py‎

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,92 @@ class EndpointMethod(Generic[P, SyncReturnT, AsyncReturnT]):
7171
the response type that ``send()`` returns for each endpoint marker.
7272
"""
7373

74+
# Copied from the endpoint in __init__ so help() and autodoc describe the
75+
# endpoint. Declared here so the descriptor's introspection surface is part of
76+
# its type rather than something callers have to discover at runtime.
77+
__wrapped__: Callable[P, PreparedRequest]
78+
__name__: str
79+
__qualname__: str
80+
__doc__: str | None
81+
__module__: str
82+
7483
def __init__(self, endpoint_fn: Callable[P, PreparedRequest]) -> None:
7584
self._endpoint_fn = endpoint_fn
85+
# Per-owning-class callable stubs handed out on *class*-level access, so
86+
# unittest.mock / inspect can classify each endpoint (sync vs coroutine)
87+
# and autospec it as callable. Keyed and cached by objtype so repeated
88+
# class access is stable. See __get__.
89+
self._class_stubs: dict[type | None, Callable[..., object]] = {}
90+
# Carry the endpoint's name, docstring, and annotations onto the descriptor
91+
# so help() and autodoc describe the endpoint rather than the descriptor.
92+
# Set directly rather than via functools.update_wrapper, which expects a
93+
# callable wrapper; a descriptor is not one, and which would also copy
94+
# __dict__ and with it the endpoint's __isabstractmethod__ marker.
95+
#
96+
# inspect.signature(SomeClient.method) works because class-level access
97+
# returns a functools.wraps'd stub (see __get__), not the raw descriptor.
98+
self.__wrapped__ = endpoint_fn
99+
for attr in functools.WRAPPER_ASSIGNMENTS:
100+
try:
101+
setattr(self, attr, getattr(endpoint_fn, attr))
102+
except AttributeError:
103+
pass
104+
105+
def _class_level_stub(self, objtype: type | None) -> Callable[..., object]:
106+
"""Callable stub returned on class-level access, matched to ``objtype``.
107+
108+
``unittest.mock`` (both ``Mock(spec=...)`` and ``create_autospec``) reads
109+
each attribute off the *class* and classifies it with ``callable()`` and
110+
``asyncio.iscoroutinefunction()``. A bare descriptor is neither callable
111+
nor a coroutine function, so every endpoint on an async client would be
112+
mocked as a sync ``MagicMock`` and could not be awaited. This hands back a
113+
real function -- ``async def`` for async clients, ``def`` for sync -- that
114+
wraps the endpoint (so ``inspect.signature`` works and autospec validates
115+
call signatures), but refuses to run unbound: endpoints only mean anything
116+
against a client instance.
117+
"""
118+
cached = self._class_stubs.get(objtype)
119+
if cached is not None:
120+
return cached
121+
is_async = objtype is not None and issubclass(objtype, AsyncNemoClient)
122+
if is_async:
123+
124+
@functools.wraps(self._endpoint_fn)
125+
async def stub(*args: object, **kwargs: object) -> object:
126+
raise TypeError(f"{self.__name__} must be called on a client instance, not the class")
127+
else:
128+
129+
@functools.wraps(self._endpoint_fn)
130+
def stub(*args: object, **kwargs: object) -> object:
131+
raise TypeError(f"{self.__name__} must be called on a client instance, not the class")
132+
133+
# __isabstractmethod__ rides along in the endpoint's __dict__ via wraps;
134+
# drop it so the stub is never mistaken for an abstract member.
135+
stub.__dict__.pop("__isabstractmethod__", None)
136+
self._class_stubs[objtype] = stub
137+
return stub
76138

139+
@property
140+
def endpoint(self) -> Callable[P, PreparedRequest]:
141+
"""The endpoint function this descriptor binds."""
142+
return self._endpoint_fn
143+
144+
@overload
145+
def __get__(self, obj: None, objtype: type | None = None) -> EndpointMethod[P, SyncReturnT, AsyncReturnT]: ...
77146
@overload
78147
def __get__(self, obj: NemoClient, objtype: type | None = None) -> Callable[P, SyncReturnT]: ...
79148
@overload
80149
def __get__(self, obj: AsyncNemoClient, objtype: type | None = None) -> Callable[P, Awaitable[AsyncReturnT]]: ...
81150

82151
def __get__(self, obj: NemoClient | AsyncNemoClient | None, objtype: type | None = None) -> object:
83-
assert obj is not None
152+
if obj is None:
153+
# Class-level access. Anything that inspects a client class rather than
154+
# an instance -- Mock(spec=...), autospec, inspect, help(), autodoc --
155+
# lands here. Hand back a callable stub matched to the owning client
156+
# type so mock classifies sync vs async correctly and autospec sees a
157+
# callable. The raw descriptor is still reachable via
158+
# inspect.getattr_static, which never invokes __get__.
159+
return self._class_level_stub(objtype)
84160
if isinstance(obj, AsyncNemoClient):
85161

86162
@functools.wraps(self._endpoint_fn)

‎packages/nemo_platform_plugin/src/nemo_platform_plugin/client/response.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ class NemoResponse(Generic[ResponseT]):
7979
resp.http_response # full httpx.Response
8080
8181
user = resp.data() # raises on non-2xx, otherwise returns body
82+
83+
When several 2xx codes share one typed body (e.g. a delete that returns 202
84+
Accepted for async teardown or 204 No Content when already gone, both typed
85+
``None``), inspect ``resp.http_response.status_code`` to tell them apart.
8286
"""
8387

8488
http_response: httpx.Response

0 commit comments

Comments
 (0)