Skip to content

Commit 2e6dc46

Browse files
authored
feat: add the models namespace to the existing client (#69)
Model operations get a namespace on the client integrators already construct — client.models on both Comfy and AsyncComfy — instead of a second client object, which would fork credential handling, base URL, transport and timeout configuration. The namespace holds the host client's transport itself rather than a copy of its settings, so a change made on the client after construction (a rotated key, a different timeout) applies through models with no re-wiring. It exposes that shared configuration read-only as base_url and timeout, backed by new read-only properties of the same names on both comfy_low transports so the layer above does not have to reach into private attributes. Nothing is added to the top-level package: from comfy_sdk import Comfy stays the only entry point and client.models is the whole surface. The sync/async parity guard gains the new pair, and the README documents the namespace.
1 parent 4f294c4 commit 2e6dc46

6 files changed

Lines changed: 226 additions & 2 deletions

File tree

README.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,28 @@ with no API key of their own. On a self-hosted proxy it's the content endpoint
254254
backend and never downloads the bytes first. (`AsyncOutput` mirrors all of the
255255
above with `await`.)
256256

257+
## The `models` namespace
258+
259+
Model operations live in a namespace on the client you already constructed —
260+
`client.models` — rather than in a second client object:
261+
262+
```python
263+
client = Comfy(api_key="comfyui-...")
264+
265+
client.models.base_url # the client's own base URL, where model requests go
266+
client.models.timeout # the client's own HTTP timeout
267+
```
268+
269+
The namespace is bound to that client's transport, so it uses the client's
270+
credentials, base URL, connection pool and timeout, and a configuration change
271+
made on the client afterwards applies through `models` as well — there is no
272+
second set of settings to keep in sync. `AsyncComfy` carries the same `models`
273+
namespace, and nothing extra is imported or constructed for it:
274+
`from comfy_sdk import Comfy` stays the only entry point.
275+
276+
`base_url` and `timeout` are a read-only view of that shared configuration;
277+
model operations are added to this namespace as they land.
278+
257279
## Sync and async
258280

259281
`Comfy` and `AsyncComfy` expose the identical surface — swap the import and

src/comfy_low/transport.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,21 @@ def __init__(
203203
self._own_client = client is None
204204
self._client = client or httpx.Client(timeout=timeout, follow_redirects=True)
205205

206+
# -- configuration ----------------------------------------------------
207+
# Read-only views of the settings this transport was built with, so a layer
208+
# above (a ``comfy_sdk`` client namespace) can report the configuration it
209+
# shares without reaching into private attributes. Both read through to the
210+
# live objects, so a later change to either is reflected here.
211+
@property
212+
def base_url(self) -> str:
213+
"""Base URL every relative API path is resolved against."""
214+
return self._p.base_url
215+
216+
@property
217+
def timeout(self) -> httpx.Timeout:
218+
"""The httpx client's default timeout. A per-request ``timeout=`` still wins."""
219+
return self._client.timeout
220+
206221
# -- lifecycle --------------------------------------------------------
207222
def close(self) -> None:
208223
if self._own_client:
@@ -492,6 +507,17 @@ def __init__(
492507
self._own_client = client is None
493508
self._client = client or httpx.AsyncClient(timeout=timeout, follow_redirects=True)
494509

510+
# -- configuration (mirrors :class:`ComfyLow`) -------------------------
511+
@property
512+
def base_url(self) -> str:
513+
"""Base URL every relative API path is resolved against."""
514+
return self._p.base_url
515+
516+
@property
517+
def timeout(self) -> httpx.Timeout:
518+
"""The httpx client's default timeout. A per-request ``timeout=`` still wins."""
519+
return self._client.timeout
520+
495521
async def aclose(self) -> None:
496522
if self._own_client:
497523
await self._client.aclose()

src/comfy_sdk/client.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
"""The clients integrators import: :class:`Comfy` (sync) and :class:`AsyncComfy`.
22
33
Both expose the same surface — ``assets`` / ``workflows`` / ``jobs`` constructor
4-
namespaces plus ``submit`` / ``run`` — over a shared sans-IO core. Only the
4+
namespaces, the ``models`` namespace, plus ``submit`` / ``run`` — over a shared
5+
sans-IO core. Every namespace is bound to the client's own transport, so they
6+
share its credentials, base URL, connection pool and timeout. Only the
57
awaiting methods are duplicated; the rules (idempotency, 429 backoff, asset
68
materialization, UI-format detection) live in ``_core`` and are called from both.
79
@@ -32,6 +34,7 @@
3234
from .assets import AssetFactory, AsyncAssetFactory
3335
from .exceptions import WorkflowFormatUi, to_sdk_error
3436
from .jobs import AsyncJob, AsyncJobFactory, Job, JobFactory
37+
from .models import AsyncModels, Models
3538
from .workflows import Workflow, WorkflowFactory
3639

3740
# How long to keep retrying a full queue before giving up (seconds).
@@ -127,6 +130,9 @@ def __init__(
127130
self.assets = AssetFactory(self._low)
128131
self.workflows = WorkflowFactory()
129132
self.jobs = JobFactory(self._low)
133+
#: ``client.models`` — the model namespace, sharing this client's
134+
#: transport (credentials, base URL, connection pool, timeout).
135+
self.models = Models(self._low)
130136

131137
def close(self) -> None:
132138
"""Release the underlying HTTP connection pool.
@@ -230,6 +236,8 @@ def __init__(
230236
self.assets = AsyncAssetFactory(self._low)
231237
self.workflows = WorkflowFactory()
232238
self.jobs = AsyncJobFactory(self._low)
239+
#: Async counterpart of :attr:`Comfy.models`, on this client's transport.
240+
self.models = AsyncModels(self._low)
233241

234242
async def aclose(self) -> None:
235243
"""Async :meth:`Comfy.close`. Prefer ``async with AsyncComfy(...)``."""

src/comfy_sdk/models.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
"""The ``models`` namespace — ``client.models`` on an existing client.
2+
3+
Reached from a client you already constructed (``Comfy().models`` /
4+
``AsyncComfy().models``) rather than built on its own, so it uses that client's
5+
credentials, base URL, transport and timeout: one connection pool, one
6+
credential, one place to configure both. A separate client object for model
7+
operations would fork all of that, which is what namespacing avoids.
8+
9+
The namespace holds the host client's transport itself — not a copy of its
10+
settings — so a change made on the client after construction (a rotated key, a
11+
different timeout) is visible through ``models`` with no re-wiring. v1 is the
12+
namespace plus a read-only view of that shared configuration; model operations
13+
are added to this object as they land, never to a parallel client.
14+
15+
Callers do not import anything for this: ``from comfy_sdk import Comfy`` stays
16+
the only entry point, and ``client.models`` is the whole surface.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import httpx
22+
23+
from comfy_low.transport import AsyncComfyLow, ComfyLow
24+
25+
26+
class _ModelsBase:
27+
"""Read-only view of the configuration inherited from the host client."""
28+
29+
_low: ComfyLow | AsyncComfyLow
30+
31+
@property
32+
def base_url(self) -> str:
33+
"""The host client's base URL — where model requests are sent."""
34+
return self._low.base_url
35+
36+
@property
37+
def timeout(self) -> httpx.Timeout:
38+
"""The host client's HTTP timeout, read live from its transport."""
39+
return self._low.timeout
40+
41+
def __repr__(self) -> str:
42+
return f"{type(self).__name__}(base_url={self.base_url!r})"
43+
44+
45+
class Models(_ModelsBase):
46+
"""``client.models`` on :class:`~comfy_sdk.client.Comfy`.
47+
48+
Constructed by the client; ``low`` is the client's own transport, which is
49+
what makes the configuration shared rather than duplicated.
50+
"""
51+
52+
def __init__(self, low: ComfyLow) -> None:
53+
self._low = low
54+
55+
56+
class AsyncModels(_ModelsBase):
57+
"""``client.models`` on :class:`~comfy_sdk.client.AsyncComfy` — mirrors :class:`Models`."""
58+
59+
def __init__(self, low: AsyncComfyLow) -> None:
60+
self._low = low

tests/test_models_namespace.py

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
"""``client.models`` — the model namespace on the existing client.
2+
3+
What makes it a namespace rather than a second client object: it is reachable
4+
from a client you already constructed, and it reads that client's *live*
5+
configuration — credentials, base URL, transport, timeout — instead of a copy
6+
taken at construction. Both properties are asserted here, including a config
7+
change made on the client after construction being visible through ``models``.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
import httpx
13+
14+
import comfy_sdk
15+
from comfy_sdk import AsyncComfy, Comfy
16+
from comfy_sdk.models import AsyncModels, Models
17+
18+
19+
def test_models_is_reachable_from_a_constructed_client(server) -> None:
20+
with Comfy() as client:
21+
assert isinstance(client.models, Models)
22+
23+
24+
async def test_async_models_is_reachable_from_a_constructed_client(server) -> None:
25+
async with AsyncComfy() as client:
26+
assert isinstance(client.models, AsyncModels)
27+
28+
29+
def test_models_holds_the_host_clients_transport(server) -> None:
30+
# Identity, not equality: the same transport object means the same
31+
# connection pool, credential, base URL and timeout — nothing to keep in
32+
# sync, and nothing a second client would have forked.
33+
with Comfy() as client:
34+
assert client.models._low is client._low
35+
36+
37+
async def test_async_models_holds_the_host_clients_transport(server) -> None:
38+
async with AsyncComfy() as client:
39+
assert client.models._low is client._low
40+
41+
42+
def test_models_reports_the_host_clients_base_url(server) -> None:
43+
with Comfy() as client:
44+
assert client.models.base_url == client._low.base_url == server.base_url
45+
46+
47+
async def test_async_models_reports_the_host_clients_base_url(server) -> None:
48+
async with AsyncComfy() as client:
49+
assert client.models.base_url == client._low.base_url == server.base_url
50+
51+
52+
def test_a_timeout_change_on_the_client_is_visible_through_models(server) -> None:
53+
with Comfy(timeout=30.0) as client:
54+
assert client.models.timeout.read == 30.0
55+
# Changed on the client *after* construction: models must follow it,
56+
# which a copied-config namespace would not.
57+
client._low._client.timeout = httpx.Timeout(1.25)
58+
assert client.models.timeout.read == 1.25
59+
60+
61+
async def test_a_timeout_change_on_the_async_client_is_visible_through_models(server) -> None:
62+
async with AsyncComfy(timeout=30.0) as client:
63+
assert client.models.timeout.read == 30.0
64+
client._low._client.timeout = httpx.Timeout(1.25)
65+
assert client.models.timeout.read == 1.25
66+
67+
68+
def test_models_sends_the_host_clients_credentials(server) -> None:
69+
server.state.require_auth = True
70+
with Comfy(api_key="k-first") as client:
71+
# The transport a model request would go out on is the client's own,
72+
# so it carries the client's bearer token to the client's base URL.
73+
client.models._low.get_job("job_01")
74+
assert server.state.last_auth_header == "Bearer k-first"
75+
76+
# And a credential rotated on the client is picked up through models.
77+
client._low._p.api_key = "k-rotated"
78+
client.models._low.get_job("job_01")
79+
assert server.state.last_auth_header == "Bearer k-rotated"
80+
81+
82+
async def test_async_models_sends_the_host_clients_credentials(server) -> None:
83+
server.state.require_auth = True
84+
async with AsyncComfy(api_key="k-async") as client:
85+
await client.models._low.get_job("job_01")
86+
assert server.state.last_auth_header == "Bearer k-async"
87+
88+
89+
def test_two_clients_get_independent_namespaces(server) -> None:
90+
with Comfy() as one, Comfy() as two:
91+
assert one.models is not two.models
92+
assert one.models._low is not two.models._low
93+
94+
95+
def test_models_repr_names_the_shared_base_url(server) -> None:
96+
with Comfy() as client:
97+
assert repr(client.models) == f"Models(base_url={server.base_url!r})"
98+
99+
100+
def test_the_namespace_adds_no_new_top_level_import_path() -> None:
101+
# ``from comfy_sdk import Comfy`` stays the only path a caller needs:
102+
# the namespace is reached as ``client.models``, so neither class is
103+
# exported at the top level.
104+
for name in ("Models", "AsyncModels"):
105+
assert name not in comfy_sdk.__all__
106+
assert not hasattr(comfy_sdk, name)

tests/test_sync_async_parity.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Sync/async public-surface parity across the 7 mirrored class pairs.
1+
"""Sync/async public-surface parity across the 8 mirrored class pairs.
22
33
The README promises "swap the import and add ``await``" as the only
44
difference; this asserts the public method names actually match. Regression
@@ -17,6 +17,7 @@
1717
from comfy_sdk.assets import Asset, AssetFactory, AsyncAsset, AsyncAssetFactory
1818
from comfy_sdk.client import AsyncComfy, Comfy
1919
from comfy_sdk.jobs import AsyncJob, AsyncJobFactory, Job, JobFactory
20+
from comfy_sdk.models import AsyncModels, Models
2021
from comfy_sdk.outputs import AsyncOutput, Output
2122

2223
_PAIRS: list[tuple[str, type, type]] = [
@@ -26,6 +27,7 @@
2627
("Job", Job, AsyncJob),
2728
("JobFactory", JobFactory, AsyncJobFactory),
2829
("Output", Output, AsyncOutput),
30+
("Models", Models, AsyncModels),
2931
("ComfyLow", ComfyLow, AsyncComfyLow),
3032
]
3133

0 commit comments

Comments
 (0)