diff --git a/src/openai/__init__.py b/src/openai/__init__.py index 9b0b7badcc..7af2f71425 100644 --- a/src/openai/__init__.py +++ b/src/openai/__init__.py @@ -294,7 +294,7 @@ def _custom_query(self, value: _t.Mapping[str, object] | None) -> None: # type: @property # type: ignore @override def _client(self) -> _httpx.Client: - return http_client or super()._client + return http_client if http_client is not None else super()._client @_client.setter # type: ignore def _client(self, value: _httpx.Client) -> None: # type: ignore diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index f195d04816..f2020d48a1 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -907,6 +907,16 @@ def __init__( custom_query: Mapping[str, object] | None = None, _strict_response_validation: bool, ) -> None: + if ( + http_client is not None + and not is_httpx2_sync_client(http_client) + and not is_legacy_httpx_sync_client(http_client) + ): + raise TypeError( + "Invalid `http_client` argument; Expected an instance of `httpx.Client` or `httpx2.Client` " + f"but got {type(http_client)}" + ) + if not is_given(timeout): # if the user passed in a custom http client with a non-default # timeout set then we use that timeout. @@ -915,22 +925,12 @@ def __init__( # where they've explicitly set the timeout to match the default timeout # as this check is structural, meaning that we'll think they didn't # pass in a timeout and will ignore it - client_timeout = normalize_httpx_timeout(http_client.timeout) if http_client else None - if http_client and client_timeout != HTTPX_DEFAULT_TIMEOUT: + client_timeout = normalize_httpx_timeout(http_client.timeout) if http_client is not None else None + if http_client is not None and client_timeout != HTTPX_DEFAULT_TIMEOUT: timeout = client_timeout else: timeout = DEFAULT_TIMEOUT - if ( - http_client is not None - and not is_httpx2_sync_client(http_client) - and not is_legacy_httpx_sync_client(http_client) - ): - raise TypeError( - "Invalid `http_client` argument; Expected an instance of `httpx.Client` or `httpx2.Client` " - f"but got {type(http_client)}" - ) - super().__init__( version=version, # cast to a valid type because mypy doesn't understand our type narrowing @@ -941,10 +941,14 @@ def __init__( custom_headers=custom_headers, _strict_response_validation=_strict_response_validation, ) - 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), + self._client = ( + http_client + if http_client is not None + else SyncHttpxClientWrapper( + base_url=base_url, + # cast to a valid type because mypy doesn't understand our type narrowing + timeout=cast(Timeout, timeout), + ) ) def is_closed(self) -> bool: @@ -1529,6 +1533,16 @@ def __init__( custom_headers: Mapping[str, str] | None = None, custom_query: Mapping[str, object] | None = None, ) -> None: + if ( + http_client is not None + and not is_httpx2_async_client(http_client) + and not is_legacy_httpx_async_client(http_client) + ): + raise TypeError( + "Invalid `http_client` argument; Expected an instance of `httpx.AsyncClient` or " + f"`httpx2.AsyncClient` but got {type(http_client)}" + ) + if not is_given(timeout): # if the user passed in a custom http client with a non-default # timeout set then we use that timeout. @@ -1537,22 +1551,12 @@ def __init__( # where they've explicitly set the timeout to match the default timeout # as this check is structural, meaning that we'll think they didn't # pass in a timeout and will ignore it - client_timeout = normalize_httpx_timeout(http_client.timeout) if http_client else None - if http_client and client_timeout != HTTPX_DEFAULT_TIMEOUT: + client_timeout = normalize_httpx_timeout(http_client.timeout) if http_client is not None else None + if http_client is not None and client_timeout != HTTPX_DEFAULT_TIMEOUT: timeout = client_timeout else: timeout = DEFAULT_TIMEOUT - if ( - http_client is not None - and not is_httpx2_async_client(http_client) - and not is_legacy_httpx_async_client(http_client) - ): - raise TypeError( - "Invalid `http_client` argument; Expected an instance of `httpx.AsyncClient` or " - f"`httpx2.AsyncClient` but got {type(http_client)}" - ) - super().__init__( version=version, base_url=base_url, @@ -1563,10 +1567,14 @@ def __init__( custom_headers=custom_headers, _strict_response_validation=_strict_response_validation, ) - 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), + self._client = ( + http_client + if http_client is not None + else AsyncHttpxClientWrapper( + base_url=base_url, + # cast to a valid type because mypy doesn't understand our type narrowing + timeout=cast(Timeout, timeout), + ) ) def is_closed(self) -> bool: diff --git a/src/openai/_client.py b/src/openai/_client.py index 7e1daecae8..d549107fd7 100644 --- a/src/openai/_client.py +++ b/src/openai/_client.py @@ -736,7 +736,7 @@ def copy( elif set_default_query is not None: params = set_default_query - http_client = http_client or self._client + http_client = http_client if http_client is not None else self._client next_provider = self._provider if isinstance(provider, NotGiven) else provider explicit_base_url = base_url is not None and not isinstance(base_url, NotGiven) @@ -1491,7 +1491,7 @@ def copy( elif set_default_query is not None: params = set_default_query - http_client = http_client or self._client + http_client = http_client if http_client is not None else self._client next_provider = self._provider if isinstance(provider, NotGiven) else provider explicit_base_url = base_url is not None and not isinstance(base_url, NotGiven) next_workload_identity = workload_identity if workload_identity is not None else self.workload_identity diff --git a/src/openai/lib/bedrock.py b/src/openai/lib/bedrock.py index 2779a419dd..07024360d1 100644 --- a/src/openai/lib/bedrock.py +++ b/src/openai/lib/bedrock.py @@ -606,7 +606,7 @@ def copy( "webhook_secret": webhook_secret if webhook_secret is not None else self.webhook_secret, "websocket_base_url": websocket_base_url if websocket_base_url is not None else self.websocket_base_url, "timeout": self.timeout if isinstance(timeout, NotGiven) else timeout, - "http_client": http_client or self._client, + "http_client": http_client if http_client is not None else self._client, "max_retries": max_retries if is_given(max_retries) else self.max_retries, "default_headers": headers, "default_query": params, @@ -844,7 +844,7 @@ def copy( "webhook_secret": webhook_secret if webhook_secret is not None else self.webhook_secret, "websocket_base_url": websocket_base_url if websocket_base_url is not None else self.websocket_base_url, "timeout": self.timeout if isinstance(timeout, NotGiven) else timeout, - "http_client": http_client or self._client, + "http_client": http_client if http_client is not None else self._client, "max_retries": max_retries if is_given(max_retries) else self.max_retries, "default_headers": headers, "default_query": params, diff --git a/tests/lib/test_bedrock.py b/tests/lib/test_bedrock.py index 903d53d63a..75ff74a7ad 100644 --- a/tests/lib/test_bedrock.py +++ b/tests/lib/test_bedrock.py @@ -656,6 +656,37 @@ def test_preserves_aws_credentials_across_with_options() -> None: assert copied_client._bedrock_state.aws_access_key_id == "access key" +def test_with_options_preserves_falsy_http_client() -> None: + class FalsyHttpClient(httpx2.Client): + def __bool__(self) -> bool: + return False + + with ( + make_sync_client(base_url="https://example.com/openai/v1", api_key="token") as client, + FalsyHttpClient() as http_client, + ): + copied_client = client.with_options(http_client=http_client) + + assert copied_client._client is http_client + copied_client.close() + + +@pytest.mark.asyncio +async def test_async_with_options_preserves_falsy_http_client() -> None: + class FalsyHttpClient(httpx2.AsyncClient): + def __bool__(self) -> bool: + return False + + async with ( + make_async_client(base_url="https://example.com/openai/v1", api_key="token") as client, + FalsyHttpClient() as http_client, + ): + copied_client = client.with_options(http_client=http_client) + + assert copied_client._client is http_client + await copied_client.close() + + @pytest.mark.parametrize("client_cls", [BedrockOpenAI, AsyncBedrockOpenAI]) def test_preserves_default_chain_mode_across_with_options(client_cls: type[Client]) -> None: with update_env(AWS_BEARER_TOKEN_BEDROCK=Omit(), AWS_REGION="us-east-1"): diff --git a/tests/test_client.py b/tests/test_client.py index d82c39e616..57db5bb8df 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -155,6 +155,17 @@ def test_copy(self, client: OpenAI) -> None: assert copied.admin_api_key == "another My Admin API Key" assert client.admin_api_key == "My Admin API Key" + def test_copy_falsy_http_client(self, client: OpenAI) -> None: + class FalsyHttpClient(httpx2.Client): + def __bool__(self) -> bool: + return False + + with FalsyHttpClient() as http_client: + copied = client.copy(http_client=http_client) + + assert copied._client is http_client + copied.close() + def test_copy_default_options(self, client: OpenAI) -> None: # options that have a default are overridden correctly copied = client.copy(max_retries=7) @@ -404,6 +415,27 @@ def test_http_client_timeout_option(self) -> None: client.close() + def test_falsy_http_client_option(self) -> None: + class FalsyHttpClient(httpx2.Client): + def __bool__(self) -> bool: + return False + + with FalsyHttpClient(timeout=None) as http_client: + client = OpenAI( + base_url=base_url, + api_key=api_key, + admin_api_key=admin_api_key, + _strict_response_validation=True, + http_client=http_client, + ) + + assert client._client is http_client + request = client._build_request(FinalRequestOptions(method="get", url="/foo")) + timeout = httpx2.Timeout(**request.extensions["timeout"]) # type: ignore + assert timeout == httpx2.Timeout(None) + + client.close() + async def test_invalid_http_client(self) -> None: with pytest.raises(TypeError, match="Invalid `http_client` arg"): async with httpx2.AsyncClient() as http_client: @@ -415,6 +447,19 @@ async def test_invalid_http_client(self) -> None: http_client=cast(Any, http_client), ) + class FalsyInvalidClient: + def __bool__(self) -> bool: + return False + + with pytest.raises(TypeError, match="Invalid `http_client` arg"): + OpenAI( + base_url=base_url, + api_key=api_key, + admin_api_key=admin_api_key, + _strict_response_validation=True, + http_client=cast(Any, FalsyInvalidClient()), + ) + def test_default_headers_option(self) -> None: test_client = OpenAI( base_url=base_url, @@ -1469,6 +1514,17 @@ def test_copy(self, async_client: AsyncOpenAI) -> None: assert copied.admin_api_key == "another My Admin API Key" assert async_client.admin_api_key == "My Admin API Key" + async def test_copy_falsy_http_client(self, async_client: AsyncOpenAI) -> None: + class FalsyHttpClient(httpx2.AsyncClient): + def __bool__(self) -> bool: + return False + + async with FalsyHttpClient() as http_client: + copied = async_client.copy(http_client=http_client) + + assert copied._client is http_client + await copied.close() + def test_copy_default_options(self, async_client: AsyncOpenAI) -> None: # options that have a default are overridden correctly copied = async_client.copy(max_retries=7) @@ -1720,6 +1776,27 @@ async def test_http_client_timeout_option(self) -> None: await client.close() + async def test_falsy_http_client_option(self) -> None: + class FalsyHttpClient(httpx2.AsyncClient): + def __bool__(self) -> bool: + return False + + async with FalsyHttpClient(timeout=None) as http_client: + client = AsyncOpenAI( + base_url=base_url, + api_key=api_key, + admin_api_key=admin_api_key, + _strict_response_validation=True, + http_client=http_client, + ) + + assert client._client is http_client + request = client._build_request(FinalRequestOptions(method="get", url="/foo")) + timeout = httpx2.Timeout(**request.extensions["timeout"]) # type: ignore + assert timeout == httpx2.Timeout(None) + + await client.close() + def test_invalid_http_client(self) -> None: with pytest.raises(TypeError, match="Invalid `http_client` arg"): with httpx2.Client() as http_client: @@ -1731,6 +1808,19 @@ def test_invalid_http_client(self) -> None: http_client=cast(Any, http_client), ) + class FalsyInvalidClient: + def __bool__(self) -> bool: + return False + + with pytest.raises(TypeError, match="Invalid `http_client` arg"): + AsyncOpenAI( + base_url=base_url, + api_key=api_key, + admin_api_key=admin_api_key, + _strict_response_validation=True, + http_client=cast(Any, FalsyInvalidClient()), + ) + async def test_default_headers_option(self) -> None: test_client = AsyncOpenAI( base_url=base_url, diff --git a/tests/test_module_client.py b/tests/test_module_client.py index 6b8076c6f6..ed9469abca 100644 --- a/tests/test_module_client.py +++ b/tests/test_module_client.py @@ -100,6 +100,17 @@ def test_http_client_option() -> None: assert openai.completions._client._client is new_client +def test_falsy_http_client_option() -> None: + class FalsyHttpClient(httpx2.Client): + def __bool__(self) -> bool: + return False + + with FalsyHttpClient() as new_client: + openai.http_client = new_client + + assert openai.completions._client._client is new_client + + import contextlib from typing import Generator