test(vector): address round-1 review nits on #926 offload proxy
- Use the reset_qdrant_singleton fixture in the network-mode test instead of manual save/restore boilerplate. - Add a test locking down the sync-callable forwarding branch (callable returning a non-coroutine is passed through without thread offload). - Document that _drive_local_coroutine treats a bare `yield None` as a non-suspension, and that the proxy is wrapped before the startup migrations so the O(N) backfill scroll also runs off the event loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
1701131017
commit
de960715c6
@@ -129,6 +129,11 @@ def _drive_local_coroutine(coro: Coroutine[Any, Any, Any]) -> Any:
|
||||
A genuine suspension (a non-``None`` yield) would mean the backend started
|
||||
doing real async I/O, which a plain thread cannot drive correctly. Fail
|
||||
loudly in that case rather than spin or silently mis-drive the coroutine.
|
||||
A bare ``yield None`` (e.g. a hand-inserted ``anyio.lowlevel.checkpoint()``)
|
||||
is deliberately treated as a non-suspension and re-driven — consistent with
|
||||
the local backend being purely synchronous. If a qdrant adapter ever starts
|
||||
inserting real checkpoints, prefer wrapping it in network mode over relaxing
|
||||
this guard.
|
||||
"""
|
||||
try:
|
||||
while True:
|
||||
@@ -687,6 +692,11 @@ async def get_qdrant_client() -> AsyncQdrantClient:
|
||||
# work to a worker thread. Network mode (mirrors the ``if
|
||||
# settings.qdrant_url`` branch above) already does non-blocking I/O
|
||||
# and is left untouched.
|
||||
#
|
||||
# Wrap here, before the ``_backfill_doc_id_to_string`` /
|
||||
# ``_ensure_payload_indexes`` migrations below, so that the O(N)
|
||||
# startup scroll runs off the event loop too — not just steady-state
|
||||
# scan/search queries.
|
||||
if not settings.qdrant_url:
|
||||
provisional = cast(
|
||||
AsyncQdrantClient, _ThreadOffloadingQdrantClient(provisional)
|
||||
|
||||
@@ -1105,7 +1105,27 @@ async def test_offloading_proxy_passes_through_non_callables(mocker):
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_get_qdrant_client_does_not_wrap_network_mode(mocker, monkeypatch):
|
||||
async def test_offloading_proxy_forwards_sync_callables_directly(mocker):
|
||||
"""A callable returning a non-coroutine is forwarded without thread offload.
|
||||
|
||||
``_maybe_offload`` only routes through the worker thread when the call
|
||||
produces a coroutine; plain sync methods return their value straight
|
||||
through.
|
||||
"""
|
||||
inner = mocker.Mock() # Mock (not AsyncMock): calls return plain values.
|
||||
inner.get_fastembed_vector_params.return_value = {"size": 384}
|
||||
|
||||
proxy = _ThreadOffloadingQdrantClient(inner)
|
||||
result = proxy.get_fastembed_vector_params()
|
||||
|
||||
assert result == {"size": 384}
|
||||
inner.get_fastembed_vector_params.assert_called_once_with()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_get_qdrant_client_does_not_wrap_network_mode(
|
||||
mocker, monkeypatch, reset_qdrant_singleton
|
||||
):
|
||||
"""Network mode (``QDRANT_URL``) returns the bare client, never the proxy.
|
||||
|
||||
Remote Qdrant already does non-blocking I/O; wrapping it would needlessly
|
||||
@@ -1139,15 +1159,7 @@ async def test_get_qdrant_client_does_not_wrap_network_mode(mocker, monkeypatch)
|
||||
lambda *a, **kw: provisional,
|
||||
)
|
||||
|
||||
original_client = qdrant_module._qdrant_client
|
||||
original_lock = qdrant_module._qdrant_init_lock
|
||||
qdrant_module._qdrant_client = None
|
||||
qdrant_module._qdrant_init_lock = None
|
||||
try:
|
||||
client = await get_qdrant_client()
|
||||
finally:
|
||||
qdrant_module._qdrant_client = original_client
|
||||
qdrant_module._qdrant_init_lock = original_lock
|
||||
client = await get_qdrant_client()
|
||||
|
||||
assert client is provisional
|
||||
assert not isinstance(client, _ThreadOffloadingQdrantClient)
|
||||
|
||||
Reference in New Issue
Block a user