Skip to content

Commit b370b5d

Browse files
Close OpenAIProvider httpx clients in typed-event runners
Address PR review feedback: _run_llm_cache_fixture_case constructed an OpenAIProvider but never awaited aclose(), leaking the underlying httpx.AsyncClient connection pool. Fixture 005 and 038 runners elsewhere in this file close the provider in a finally block; the new cache-fixture runner now follows the same convention. Extending the fix to the typed-event runners introduced in PR #139 (_build_simple_llm_graph, _run_typed_event_fanout_case, _run_typed_event_branches_case) — same bug class, same file, CoPilot missed them in the original review. _build_simple_llm_graph now returns (graph, state_cls, provider) so the caller can close the provider in a finally; the fan-out and parallel-branches runners close their inline-constructed providers symmetrically. No behavior change for fixture pass/fail outcomes; the leak was warning-level rather than failure-inducing. Full suite stays at 1191 pass.
1 parent c5a44fd commit b370b5d

1 file changed

Lines changed: 51 additions & 34 deletions

File tree

tests/conformance/test_observability.py

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,12 +2789,13 @@ def _build_simple_llm_graph(
27892789
case: Mapping[str, Any],
27902790
*,
27912791
populate_caller_metadata: bool,
2792-
) -> tuple[Any, type[Any]]:
2792+
) -> tuple[Any, type[Any], Any]:
27932793
"""Build a single-node graph that calls the LLM provider against a
27942794
mock transport. Matches the simple entry → ask → END pattern used
27952795
by fixtures 050, 051, 052, 053, 056. Returns ``(compiled_graph,
2796-
state_cls)`` so the caller can construct State instances without
2797-
re-deriving the class.
2796+
state_cls, provider)`` — the caller owns the provider's lifecycle
2797+
and MUST call ``await provider.aclose()`` after invoke completes
2798+
to release the underlying httpx.AsyncClient connection pool.
27982799
"""
27992800
import json
28002801

@@ -2850,7 +2851,7 @@ async def ask_body(_s: Any) -> dict[str, str]:
28502851
builder = (
28512852
GraphBuilder(state_cls).add_node(entry_name, ask_body).add_edge(entry_name, END).set_entry(entry_name)
28522853
)
2853-
return builder.compile(), state_cls
2854+
return builder.compile(), state_cls, provider
28542855

28552856

28562857
def _make_state_instance(case: Mapping[str, Any], state_cls: type[Any]) -> Any:
@@ -3253,6 +3254,10 @@ async def ask_body(_s: Any) -> dict[str, str]:
32533254
finally:
32543255
await graph.drain()
32553256
observer.shutdown()
3257+
# OpenAIProvider owns an httpx.AsyncClient; closing it releases
3258+
# the connection pool. Matches the convention used by fixture
3259+
# 005 / 038 runners elsewhere in this file.
3260+
await provider.aclose()
32563261

32573262
expected = cast("dict[str, Any]", case["expected"])
32583263

@@ -3312,36 +3317,44 @@ async def _run_typed_event_fixture_case(
33123317
the same surface.
33133318
"""
33143319
collectors, populate_caller_metadata = _parse_typed_observers(case)
3315-
graph, state_cls = _build_simple_llm_graph(case, populate_caller_metadata=populate_caller_metadata)
3316-
extra: _AllEventsCollector | None = None
3317-
if expect_failure and not any(c.filter_event_type is None for c in collectors.values()):
3318-
extra = _AllEventsCollector()
3319-
final, exc = await _invoke_typed_fixture(case, collectors, graph, state_cls, extra_observer=extra)
3320-
3321-
expected = cast("dict[str, Any]", case.get("expected") or {})
3322-
if expect_failure:
3323-
assert exc is not None, "failure-path fixture expected an exception"
3324-
node_completed = cast("dict[str, Any] | None", expected.get("node_completed_event_carries_error"))
3325-
if node_completed:
3326-
# Source for the assertion: an unfiltered named collector
3327-
# when present, otherwise the failure-path-only extra
3328-
# ``_AllEventsCollector``.
3329-
unfiltered_named = next((c for c in collectors.values() if c.filter_event_type is None), None)
3330-
source = (
3331-
unfiltered_named.events
3332-
if unfiltered_named is not None
3333-
else (extra.events if extra is not None else [])
3334-
)
3335-
_assert_node_completed_event_carries_error(source, node_completed)
3336-
else:
3337-
if final is None:
3338-
raise AssertionError("expected a non-None final state on success path")
3339-
observer_expectations = cast("dict[str, Any]", expected.get("observers") or {})
3340-
for name, expectations in observer_expectations.items():
3341-
collector = collectors.get(name)
3342-
if collector is None:
3343-
raise AssertionError(f"fixture references unknown observer {name!r}")
3344-
_assert_observer_expectations(name, collector, cast("dict[str, Any]", expectations))
3320+
graph, state_cls, provider = _build_simple_llm_graph(
3321+
case, populate_caller_metadata=populate_caller_metadata
3322+
)
3323+
try:
3324+
extra: _AllEventsCollector | None = None
3325+
if expect_failure and not any(c.filter_event_type is None for c in collectors.values()):
3326+
extra = _AllEventsCollector()
3327+
final, exc = await _invoke_typed_fixture(case, collectors, graph, state_cls, extra_observer=extra)
3328+
3329+
expected = cast("dict[str, Any]", case.get("expected") or {})
3330+
if expect_failure:
3331+
assert exc is not None, "failure-path fixture expected an exception"
3332+
node_completed = cast("dict[str, Any] | None", expected.get("node_completed_event_carries_error"))
3333+
if node_completed:
3334+
# Source for the assertion: an unfiltered named collector
3335+
# when present, otherwise the failure-path-only extra
3336+
# ``_AllEventsCollector``.
3337+
unfiltered_named = next((c for c in collectors.values() if c.filter_event_type is None), None)
3338+
source = (
3339+
unfiltered_named.events
3340+
if unfiltered_named is not None
3341+
else (extra.events if extra is not None else [])
3342+
)
3343+
_assert_node_completed_event_carries_error(source, node_completed)
3344+
else:
3345+
if final is None:
3346+
raise AssertionError("expected a non-None final state on success path")
3347+
observer_expectations = cast("dict[str, Any]", expected.get("observers") or {})
3348+
for name, expectations in observer_expectations.items():
3349+
collector = collectors.get(name)
3350+
if collector is None:
3351+
raise AssertionError(f"fixture references unknown observer {name!r}")
3352+
_assert_observer_expectations(name, collector, cast("dict[str, Any]", expectations))
3353+
finally:
3354+
# _build_simple_llm_graph hands ownership of the provider's
3355+
# httpx.AsyncClient to the runner; close it to release the
3356+
# connection pool.
3357+
await provider.aclose()
33453358

33463359

33473360
async def _run_fixture_050(spec: Mapping[str, Any]) -> None:
@@ -3516,6 +3529,8 @@ async def _ask_body(_s: Any) -> dict[str, str]:
35163529
for handle in handles:
35173530
handle.remove()
35183531
await outer_compiled.drain()
3532+
# Release the underlying httpx.AsyncClient connection pool.
3533+
await provider.aclose()
35193534

35203535
expected = cast("dict[str, Any]", case.get("expected") or {})
35213536
observer_expectations = cast("dict[str, Any]", expected.get("observers") or {})
@@ -3618,6 +3633,8 @@ async def _body(_s: Any, _msgs: Any = msgs, _stores: str = stores_in) -> dict[st
36183633
for handle in handles:
36193634
handle.remove()
36203635
await outer_compiled.drain()
3636+
# Release the underlying httpx.AsyncClient connection pool.
3637+
await provider.aclose()
36213638

36223639
expected = cast("dict[str, Any]", case.get("expected") or {})
36233640
observer_expectations = cast("dict[str, Any]", expected.get("observers") or {})

0 commit comments

Comments
 (0)