Skip to content

Commit 29eabaf

Browse files
committed
feat(plugin): add EntityUpdateClientProtocol and parent-aware get
The entity-client protocols describe a narrower surface than `EntityClient` actually has, so a plugin needing more either declares a private protocol or types against the concrete class. Two capabilities are missing: - `update`, the read-modify-write against the `db_version` optimistic lock - `get(..., parent=...)`, needed to address a child entity, which is unique within (workspace, entity_type, parent, name) rather than by name alone Add `EntityUpdateClientProtocol` as its own protocol rather than folding `update` into `EntityClientProtocol`. Protocols are structural, so a new member silently invalidates every existing implementer — including every test double — even for services that never call it. Most services only create and read; they keep their narrow surface, and a service needing both composes: class Store(EntityClientProtocol[T], EntityUpdateClientProtocol[T], Protocol[T]): ... `parent` is added to `EntityGetterProtocol.get` instead, because a second protocol declaring a conflicting `get` could not compose with the first. It is optional, so fetching a root entity is unchanged. The existing test doubles gain the argument: each stands in for a client that already accepts it, so their signature was simply inaccurate. A static conformance assertion in the tests fails type-checking if the client and the protocols ever drift apart, which is the situation this change exists to fix. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
1 parent 79ca283 commit 29eabaf

14 files changed

Lines changed: 145 additions & 12 deletions

File tree

packages/nemo_platform_plugin/src/nemo_platform_plugin/entities/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@
6868
from nemo_platform_plugin.entities.base import (
6969
EntityTypeLike as EntityTypeLike,
7070
)
71+
from nemo_platform_plugin.entities.base import (
72+
EntityUpdateClientProtocol as EntityUpdateClientProtocol,
73+
)
7174
from nemo_platform_plugin.entities.base import (
7275
EntityValidationError as EntityValidationError,
7376
)

packages/nemo_platform_plugin/src/nemo_platform_plugin/entities/base.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,21 @@ class EntityToken(Protocol):
231231

232232

233233
class EntityGetterProtocol(Protocol[EntityT]):
234-
"""Protocol for entity clients that can fetch entities by workspace/name."""
234+
"""Protocol for entity clients that can fetch entities by workspace/name.
235235
236-
async def get(self, entity_type: Type[EntityT], *, name: str, workspace: str) -> EntityT: ...
236+
``parent`` addresses a **child** entity, which is unique within
237+
``(workspace, entity_type, parent, name)`` rather than by name alone. It is optional, so
238+
fetching a root entity is unchanged.
239+
"""
240+
241+
async def get(
242+
self,
243+
entity_type: Type[EntityT],
244+
*,
245+
name: str,
246+
workspace: str,
247+
parent: Optional[str] = None,
248+
) -> EntityT: ...
237249

238250

239251
class EntityDeleteClientProtocol(EntityGetterProtocol[EntityT], Protocol[EntityT]):
@@ -266,6 +278,20 @@ class EntityClientProtocol(EntityDeleteClientProtocol[EntityT], Protocol[EntityT
266278
async def create(self, entity: EntityT) -> EntityT: ...
267279

268280

281+
class EntityUpdateClientProtocol(Protocol[EntityT]):
282+
"""Protocol for entity clients that can update an existing entity.
283+
284+
Separate from :class:`EntityClientProtocol` rather than folded into it: ``update`` is a
285+
read-modify-write against the ``db_version`` optimistic lock, and most services never need it.
286+
Compose it with the CRUD protocol where a service does::
287+
288+
class Store(EntityClientProtocol[MyEntity], EntityUpdateClientProtocol[MyEntity], Protocol):
289+
...
290+
"""
291+
292+
async def update(self, entity: EntityT, *, original_name: Optional[str] = None) -> EntityT: ...
293+
294+
269295
class AnyEntityGetterProtocol(Protocol):
270296
"""Protocol for clients that can fetch any entity model type."""
271297

packages/nemo_platform_plugin/src/nemo_platform_plugin/entity_client.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@
5454
from nemo_platform_plugin.entities import (
5555
EntityNotFoundError as NemoEntityNotFoundError,
5656
)
57+
from nemo_platform_plugin.entities import (
58+
EntityUpdateClientProtocol as NemoEntityUpdateClientProtocol,
59+
)
5760
from nemo_platform_plugin.entities import (
5861
EntityValidationError as NemoEntityValidationError,
5962
)
@@ -65,6 +68,7 @@
6568
"NemoEntitiesClient",
6669
"NemoEntitiesClientProtocol",
6770
"NemoAnyEntityDeleteClientProtocol",
71+
"NemoEntityUpdateClientProtocol",
6872
"NemoAnyEntityGetterProtocol",
6973
"NemoEntityDeleteClientProtocol",
7074
"NemoEntityGetterProtocol",
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
"""The entity-client protocols must describe the client they stand in for.
5+
6+
A protocol that has drifted from its implementation is worse than no protocol: a service typed
7+
against it either fails type-checking on correct code, or type-checks against a method the real
8+
client does not have. These tests pin that relationship so the two cannot silently diverge.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import inspect
14+
from typing import Protocol, TypeVar
15+
16+
from nemo_platform_plugin.entities import (
17+
EntityBase,
18+
EntityClient,
19+
EntityClientProtocol,
20+
EntityGetterProtocol,
21+
EntityUpdateClientProtocol,
22+
)
23+
24+
EntityT = TypeVar("EntityT", bound=EntityBase)
25+
26+
27+
class _Entity(EntityBase):
28+
__entity_type__ = "protocol_conformance_probe"
29+
30+
31+
class _ReadWriteStore(
32+
EntityClientProtocol[EntityT],
33+
EntityUpdateClientProtocol[EntityT],
34+
Protocol[EntityT],
35+
):
36+
"""The shape a service needing the wider surface composes for itself.
37+
38+
Exists here to prove the pieces *compose*: ``update`` is a separate protocol precisely so a
39+
service can opt into it alongside the CRUD one, rather than declaring a private protocol that
40+
restates the whole surface.
41+
"""
42+
43+
44+
def _static_conformance(client: EntityClient) -> _ReadWriteStore[_Entity]:
45+
"""Static assertion, checked by ``ty`` rather than at runtime.
46+
47+
If ``EntityClient`` ever stops satisfying the composed protocols — a renamed method, a changed
48+
signature — this return fails type-checking. The runtime tests below document *which* parts
49+
matter and why; this is what actually catches drift, because structural conformance is a
50+
type-level property no ``hasattr`` check can verify.
51+
"""
52+
return client
53+
54+
55+
def _signature(owner: object, method: str) -> inspect.Signature:
56+
return inspect.signature(getattr(owner, method))
57+
58+
59+
def test_update_is_its_own_protocol() -> None:
60+
"""``update`` is opt-in. Most services never modify an entity in place, and folding ``update``
61+
into the CRUD protocol would force each of them — and every one of their test doubles — to
62+
satisfy a method they do not use."""
63+
assert hasattr(EntityUpdateClientProtocol, "update")
64+
assert not hasattr(EntityClientProtocol, "update")
65+
66+
67+
def test_update_protocol_matches_the_client() -> None:
68+
protocol = _signature(EntityUpdateClientProtocol, "update").parameters
69+
client = _signature(EntityClient, "update").parameters
70+
assert set(protocol) == set(client)
71+
assert protocol["original_name"].kind is inspect.Parameter.KEYWORD_ONLY
72+
73+
74+
def test_getter_accepts_parent_for_child_entities() -> None:
75+
"""Child records are unique within ``(workspace, entity_type, parent, name)``, so the parent is
76+
part of their address. It is optional, so fetching a root entity is unchanged."""
77+
getter = _signature(EntityGetterProtocol, "get").parameters
78+
assert "parent" in getter
79+
assert getter["parent"].default is None
80+
assert "parent" in _signature(EntityClient, "get").parameters

plugins/nemo-evaluator/tests/api/service/test_metric_service.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ def __init__(self) -> None:
6565
self.delete_error: Exception | None = None
6666
self.list_filter_operations: list[FilterOperation | None] = []
6767

68-
async def get(self, entity_type: type[MetricBundleEntity], *, workspace: str, name: str) -> MetricBundleEntity:
68+
async def get(
69+
self, entity_type: type[MetricBundleEntity], *, workspace: str, name: str, parent: str | None = None
70+
) -> MetricBundleEntity:
6971
key = (workspace, name)
7072
if key not in self.entities:
7173
raise NemoEntityNotFoundError(f"{workspace}/{name} not found")

plugins/nemo-evaluator/tests/api/service/test_result_service.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ def seed(self, entity: _ResultEntityT) -> _ResultEntityT:
3737
self.entities[(entity.__entity_type__, entity.workspace, entity.name)] = entity
3838
return entity
3939

40-
async def get(self, entity_type: type[_ResultEntityT], *, workspace: str, name: str) -> _ResultEntityT:
40+
async def get(
41+
self, entity_type: type[_ResultEntityT], *, workspace: str, name: str, parent: str | None = None
42+
) -> _ResultEntityT:
4143
key = (entity_type.__entity_type__, workspace, name)
4244
if key not in self.entities:
4345
raise NemoEntityNotFoundError(f"{workspace}/{name} not found")

plugins/nemo-evaluator/tests/api/service/test_task_service.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ async def create(self, entity: TaskEntity) -> TaskEntity:
5454
self.entities[key] = entity
5555
return entity
5656

57-
async def get(self, entity_type: type[TaskEntity], *, workspace: str, name: str) -> TaskEntity:
57+
async def get(
58+
self, entity_type: type[TaskEntity], *, workspace: str, name: str, parent: str | None = None
59+
) -> TaskEntity:
5860
key = (entity_type.__entity_type__, workspace, name)
5961
if key not in self.entities:
6062
raise NemoEntityNotFoundError(f"{workspace}/{name} not found")

plugins/nemo-evaluator/tests/api/service/test_taskset_service.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ async def create(self, entity: TasksetEntity) -> TasksetEntity:
4343
self.entities[key] = entity
4444
return entity
4545

46-
async def get(self, entity_type: type[TasksetEntity], *, workspace: str, name: str) -> TasksetEntity:
46+
async def get(
47+
self, entity_type: type[TasksetEntity], *, workspace: str, name: str, parent: str | None = None
48+
) -> TasksetEntity:
4749
key = (entity_type.__entity_type__, workspace, name)
4850
if key not in self.entities:
4951
raise NemoEntityNotFoundError(f"{workspace}/{name} not found")

plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ def __init__(self) -> None:
7878
self.bump_version_on_next_delete = False
7979
self.delete_expected_db_versions: list[int | None] = []
8080

81-
async def get(self, entity_type: type[MetricBundleEntity], *, workspace: str, name: str) -> MetricBundleEntity:
81+
async def get(
82+
self, entity_type: type[MetricBundleEntity], *, workspace: str, name: str, parent: str | None = None
83+
) -> MetricBundleEntity:
8284
key = (workspace, name)
8385
if key not in self.entities:
8486
raise NemoEntityNotFoundError(f"{workspace}/{name} not found")

plugins/nemo-evaluator/tests/api/v2/test_results_routes.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ def seed(self, entity: _ResultEntityT) -> _ResultEntityT:
3939
self.entities[(entity.__entity_type__, entity.workspace, entity.name)] = entity
4040
return entity
4141

42-
async def get(self, entity_type: type[_ResultEntityT], *, workspace: str, name: str) -> _ResultEntityT:
42+
async def get(
43+
self, entity_type: type[_ResultEntityT], *, workspace: str, name: str, parent: str | None = None
44+
) -> _ResultEntityT:
4345
key = (entity_type.__entity_type__, workspace, name)
4446
if key not in self.entities:
4547
raise NemoEntityNotFoundError(f"{workspace}/{name} not found")

0 commit comments

Comments
 (0)