Skip to content

Commit

Permalink
refactor(client): simplify cleanup (#278)
Browse files Browse the repository at this point in the history
This removes Client.__del__, but users are not expected to call this directly.
  • Loading branch information
stainless-bot committed Dec 13, 2023
1 parent d952e64 commit d6221bf
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 54 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ typecheck = { chain = [
]}
"typecheck:pyright" = "pyright"
"typecheck:verify-types" = "pyright --verifytypes anthropic --ignoreexternal"
"typecheck:mypy" = "mypy --enable-incomplete-feature=Unpack ."
"typecheck:mypy" = "mypy ."

[build-system]
requires = ["hatchling"]
Expand Down
26 changes: 20 additions & 6 deletions src/anthropic/_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import time
import uuid
import email
import asyncio
import inspect
import logging
import platform
Expand Down Expand Up @@ -672,9 +673,16 @@ def _idempotency_key(self) -> str:
return f"stainless-python-retry-{uuid.uuid4()}"


class SyncHttpxClientWrapper(httpx.Client):
def __del__(self) -> None:
try:
self.close()
except Exception:
pass


class SyncAPIClient(BaseClient[httpx.Client, Stream[Any]]):
_client: httpx.Client
_has_custom_http_client: bool
_default_stream_cls: type[Stream[Any]] | None = None

def __init__(
Expand Down Expand Up @@ -747,15 +755,14 @@ def __init__(
custom_headers=custom_headers,
_strict_response_validation=_strict_response_validation,
)
self._client = http_client or httpx.Client(
self._client = http_client or SyncHttpxClientWrapper(
base_url=base_url,
# cast to a valid type because mypy doesn't understand our type narrowing
timeout=cast(Timeout, timeout),
proxies=proxies,
transport=transport,
limits=limits,
)
self._has_custom_http_client = bool(http_client)

def is_closed(self) -> bool:
return self._client.is_closed
Expand Down Expand Up @@ -1135,9 +1142,17 @@ def get_api_list(
return self._request_api_list(model, page, opts)


class AsyncHttpxClientWrapper(httpx.AsyncClient):
def __del__(self) -> None:
try:
# TODO(someday): support non asyncio runtimes here
asyncio.get_running_loop().create_task(self.aclose())
except Exception:
pass


class AsyncAPIClient(BaseClient[httpx.AsyncClient, AsyncStream[Any]]):
_client: httpx.AsyncClient
_has_custom_http_client: bool
_default_stream_cls: type[AsyncStream[Any]] | None = None

def __init__(
Expand Down Expand Up @@ -1210,15 +1225,14 @@ def __init__(
custom_headers=custom_headers,
_strict_response_validation=_strict_response_validation,
)
self._client = http_client or httpx.AsyncClient(
self._client = http_client or AsyncHttpxClientWrapper(
base_url=base_url,
# cast to a valid type because mypy doesn't understand our type narrowing
timeout=cast(Timeout, timeout),
proxies=proxies,
transport=transport,
limits=limits,
)
self._has_custom_http_client = bool(http_client)

def is_closed(self) -> bool:
return self._client.is_closed
Expand Down
30 changes: 4 additions & 26 deletions src/anthropic/_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from __future__ import annotations

import os
import asyncio
from typing import Any, Union, Mapping
from typing_extensions import Self, override

Expand Down Expand Up @@ -34,6 +33,8 @@
DEFAULT_MAX_RETRIES,
SyncAPIClient,
AsyncAPIClient,
SyncHttpxClientWrapper,
AsyncHttpxClientWrapper,
)

__all__ = [
Expand Down Expand Up @@ -222,7 +223,7 @@ def copy(
if http_client is not None:
raise ValueError("The 'http_client' argument is mutually exclusive with 'connection_pool_limits'")

if self._has_custom_http_client:
if not isinstance(self._client, SyncHttpxClientWrapper):
raise ValueError(
"A custom HTTP client has been set and is mutually exclusive with the 'connection_pool_limits' argument"
)
Expand Down Expand Up @@ -253,16 +254,6 @@ def copy(
# client.with_options(timeout=10).foo.create(...)
with_options = copy

def __del__(self) -> None:
if not hasattr(self, "_has_custom_http_client") or not hasattr(self, "close"):
# this can happen if the '__init__' method raised an error
return

if self._has_custom_http_client:
return

self.close()

def count_tokens(
self,
text: str,
Expand Down Expand Up @@ -483,7 +474,7 @@ def copy(
if http_client is not None:
raise ValueError("The 'http_client' argument is mutually exclusive with 'connection_pool_limits'")

if self._has_custom_http_client:
if not isinstance(self._client, AsyncHttpxClientWrapper):
raise ValueError(
"A custom HTTP client has been set and is mutually exclusive with the 'connection_pool_limits' argument"
)
Expand Down Expand Up @@ -514,19 +505,6 @@ def copy(
# client.with_options(timeout=10).foo.create(...)
with_options = copy

def __del__(self) -> None:
if not hasattr(self, "_has_custom_http_client") or not hasattr(self, "close"):
# this can happen if the '__init__' method raised an error
return

if self._has_custom_http_client:
return

try:
asyncio.get_running_loop().create_task(self.close())
except Exception:
pass

async def count_tokens(
self,
text: str,
Expand Down
23 changes: 2 additions & 21 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,24 +692,15 @@ def test_proxies_option_mutually_exclusive_with_http_client(self) -> None:
http_client=http_client,
)

def test_client_del(self) -> None:
client = Anthropic(base_url=base_url, api_key=api_key, _strict_response_validation=True)
assert not client.is_closed()

client.__del__()

assert client.is_closed()

def test_copied_client_does_not_close_http(self) -> None:
client = Anthropic(base_url=base_url, api_key=api_key, _strict_response_validation=True)
assert not client.is_closed()

copied = client.copy()
assert copied is not client

copied.__del__()
del copied

assert not copied.is_closed()
assert not client.is_closed()

def test_client_context_manager(self) -> None:
Expand Down Expand Up @@ -1515,26 +1506,16 @@ async def test_proxies_option_mutually_exclusive_with_http_client(self) -> None:
http_client=http_client,
)

async def test_client_del(self) -> None:
client = AsyncAnthropic(base_url=base_url, api_key=api_key, _strict_response_validation=True)
assert not client.is_closed()

client.__del__()

await asyncio.sleep(0.2)
assert client.is_closed()

async def test_copied_client_does_not_close_http(self) -> None:
client = AsyncAnthropic(base_url=base_url, api_key=api_key, _strict_response_validation=True)
assert not client.is_closed()

copied = client.copy()
assert copied is not client

copied.__del__()
del copied

await asyncio.sleep(0.2)
assert not copied.is_closed()
assert not client.is_closed()

async def test_client_context_manager(self) -> None:
Expand Down

0 comments on commit d6221bf

Please sign in to comment.