Skip to content

Commit 5c138f4

Browse files
refactor(client): simplify cleanup (#966)
This removes Client.__del__, but users are not expected to call this directly.
1 parent 3c3ed5e commit 5c138f4

File tree

5 files changed

+23
-59
lines changed

5 files changed

+23
-59
lines changed

pyproject.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ typecheck = { chain = [
8484
]}
8585
"typecheck:pyright" = "pyright"
8686
"typecheck:verify-types" = "pyright --verifytypes openai --ignoreexternal"
87-
"typecheck:mypy" = "mypy --enable-incomplete-feature=Unpack ."
87+
"typecheck:mypy" = "mypy ."
8888

8989
[build-system]
9090
requires = ["hatchling"]

src/openai/__init__.py

-7
Original file line numberDiff line numberDiff line change
@@ -221,13 +221,6 @@ def _client(self, value: _httpx.Client) -> None: # type: ignore
221221

222222
http_client = value
223223

224-
@override
225-
def __del__(self) -> None:
226-
try:
227-
super().__del__()
228-
except Exception:
229-
pass
230-
231224

232225
class _AzureModuleClient(_ModuleClient, AzureOpenAI): # type: ignore
233226
...

src/openai/_base_client.py

+20-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import time
66
import uuid
77
import email
8+
import asyncio
89
import inspect
910
import logging
1011
import platform
@@ -672,9 +673,16 @@ def _idempotency_key(self) -> str:
672673
return f"stainless-python-retry-{uuid.uuid4()}"
673674

674675

676+
class SyncHttpxClientWrapper(httpx.Client):
677+
def __del__(self) -> None:
678+
try:
679+
self.close()
680+
except Exception:
681+
pass
682+
683+
675684
class SyncAPIClient(BaseClient[httpx.Client, Stream[Any]]):
676685
_client: httpx.Client
677-
_has_custom_http_client: bool
678686
_default_stream_cls: type[Stream[Any]] | None = None
679687

680688
def __init__(
@@ -747,15 +755,14 @@ def __init__(
747755
custom_headers=custom_headers,
748756
_strict_response_validation=_strict_response_validation,
749757
)
750-
self._client = http_client or httpx.Client(
758+
self._client = http_client or SyncHttpxClientWrapper(
751759
base_url=base_url,
752760
# cast to a valid type because mypy doesn't understand our type narrowing
753761
timeout=cast(Timeout, timeout),
754762
proxies=proxies,
755763
transport=transport,
756764
limits=limits,
757765
)
758-
self._has_custom_http_client = bool(http_client)
759766

760767
def is_closed(self) -> bool:
761768
return self._client.is_closed
@@ -1135,9 +1142,17 @@ def get_api_list(
11351142
return self._request_api_list(model, page, opts)
11361143

11371144

1145+
class AsyncHttpxClientWrapper(httpx.AsyncClient):
1146+
def __del__(self) -> None:
1147+
try:
1148+
# TODO(someday): support non asyncio runtimes here
1149+
asyncio.get_running_loop().create_task(self.aclose())
1150+
except Exception:
1151+
pass
1152+
1153+
11381154
class AsyncAPIClient(BaseClient[httpx.AsyncClient, AsyncStream[Any]]):
11391155
_client: httpx.AsyncClient
1140-
_has_custom_http_client: bool
11411156
_default_stream_cls: type[AsyncStream[Any]] | None = None
11421157

11431158
def __init__(
@@ -1210,15 +1225,14 @@ def __init__(
12101225
custom_headers=custom_headers,
12111226
_strict_response_validation=_strict_response_validation,
12121227
)
1213-
self._client = http_client or httpx.AsyncClient(
1228+
self._client = http_client or AsyncHttpxClientWrapper(
12141229
base_url=base_url,
12151230
# cast to a valid type because mypy doesn't understand our type narrowing
12161231
timeout=cast(Timeout, timeout),
12171232
proxies=proxies,
12181233
transport=transport,
12191234
limits=limits,
12201235
)
1221-
self._has_custom_http_client = bool(http_client)
12221236

12231237
def is_closed(self) -> bool:
12241238
return self._client.is_closed

src/openai/_client.py

-24
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from __future__ import annotations
44

55
import os
6-
import asyncio
76
from typing import Any, Union, Mapping
87
from typing_extensions import Self, override
98

@@ -205,16 +204,6 @@ def copy(
205204
# client.with_options(timeout=10).foo.create(...)
206205
with_options = copy
207206

208-
def __del__(self) -> None:
209-
if not hasattr(self, "_has_custom_http_client") or not hasattr(self, "close"):
210-
# this can happen if the '__init__' method raised an error
211-
return
212-
213-
if self._has_custom_http_client:
214-
return
215-
216-
self.close()
217-
218207
@override
219208
def _make_status_error(
220209
self,
@@ -415,19 +404,6 @@ def copy(
415404
# client.with_options(timeout=10).foo.create(...)
416405
with_options = copy
417406

418-
def __del__(self) -> None:
419-
if not hasattr(self, "_has_custom_http_client") or not hasattr(self, "close"):
420-
# this can happen if the '__init__' method raised an error
421-
return
422-
423-
if self._has_custom_http_client:
424-
return
425-
426-
try:
427-
asyncio.get_running_loop().create_task(self.close())
428-
except Exception:
429-
pass
430-
431407
@override
432408
def _make_status_error(
433409
self,

tests/test_client.py

+2-21
Original file line numberDiff line numberDiff line change
@@ -591,24 +591,15 @@ def test_absolute_request_url(self, client: OpenAI) -> None:
591591
)
592592
assert request.url == "https://myapi.com/foo"
593593

594-
def test_client_del(self) -> None:
595-
client = OpenAI(base_url=base_url, api_key=api_key, _strict_response_validation=True)
596-
assert not client.is_closed()
597-
598-
client.__del__()
599-
600-
assert client.is_closed()
601-
602594
def test_copied_client_does_not_close_http(self) -> None:
603595
client = OpenAI(base_url=base_url, api_key=api_key, _strict_response_validation=True)
604596
assert not client.is_closed()
605597

606598
copied = client.copy()
607599
assert copied is not client
608600

609-
copied.__del__()
601+
del copied
610602

611-
assert not copied.is_closed()
612603
assert not client.is_closed()
613604

614605
def test_client_context_manager(self) -> None:
@@ -1325,26 +1316,16 @@ def test_absolute_request_url(self, client: AsyncOpenAI) -> None:
13251316
)
13261317
assert request.url == "https://myapi.com/foo"
13271318

1328-
async def test_client_del(self) -> None:
1329-
client = AsyncOpenAI(base_url=base_url, api_key=api_key, _strict_response_validation=True)
1330-
assert not client.is_closed()
1331-
1332-
client.__del__()
1333-
1334-
await asyncio.sleep(0.2)
1335-
assert client.is_closed()
1336-
13371319
async def test_copied_client_does_not_close_http(self) -> None:
13381320
client = AsyncOpenAI(base_url=base_url, api_key=api_key, _strict_response_validation=True)
13391321
assert not client.is_closed()
13401322

13411323
copied = client.copy()
13421324
assert copied is not client
13431325

1344-
copied.__del__()
1326+
del copied
13451327

13461328
await asyncio.sleep(0.2)
1347-
assert not copied.is_closed()
13481329
assert not client.is_closed()
13491330

13501331
async def test_client_context_manager(self) -> None:

0 commit comments

Comments
 (0)