diff --git a/src/openai/__init__.py b/src/openai/__init__.py index 3d6c4a262f..a0f352a42a 100644 --- a/src/openai/__init__.py +++ b/src/openai/__init__.py @@ -340,7 +340,7 @@ def _custom_query(self, value: _t.Mapping[str, object] | None) -> None: @property @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 def _client(self, value: _httpx.Client) -> None: diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index 1e5645a297..cabc96dfc1 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -1014,30 +1014,30 @@ 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. - + # # note: there is an edge case here where the user passes in a client # 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, timeout=timeout, @@ -1047,9 +1047,13 @@ def __init__( custom_headers=custom_headers, _strict_response_validation=_strict_response_validation, ) - self._client = http_client or SyncHttpxClientWrapper( - base_url=base_url, - timeout=timeout, + self._client = ( + http_client + if http_client is not None + else SyncHttpxClientWrapper( + base_url=base_url, + timeout=timeout, + ) ) def is_closed(self) -> bool: @@ -1635,30 +1639,30 @@ 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. - + # # note: there is an edge case here where the user passes in a client # 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, @@ -1668,9 +1672,13 @@ def __init__( custom_headers=custom_headers, _strict_response_validation=_strict_response_validation, ) - self._client = http_client or AsyncHttpxClientWrapper( - base_url=base_url, - timeout=timeout, + self._client = ( + http_client + if http_client is not None + else AsyncHttpxClientWrapper( + base_url=base_url, + timeout=timeout, + ) ) def is_closed(self) -> bool: diff --git a/src/openai/lib/_client_auth.py b/src/openai/lib/_client_auth.py index 982d82ccac..879cd2d27c 100644 --- a/src/openai/lib/_client_auth.py +++ b/src/openai/lib/_client_auth.py @@ -466,7 +466,7 @@ def synccopy( params = {**params, **default_query} 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, _sdk_client.NotGiven) else provider explicit_base_url = base_url is not None and (not isinstance(base_url, _sdk_client.NotGiven)) next_workload_identity = workload_identity if workload_identity is not None else self.workload_identity @@ -939,7 +939,7 @@ def asynccopy( params = {**params, **default_query} 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, _sdk_client.NotGiven) else provider explicit_base_url = base_url is not None and (not isinstance(base_url, _sdk_client.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 64747749d0..fcecedae01 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 8ba681c7ff..53c6ceda57 100644 --- a/tests/lib/test_bedrock.py +++ b/tests/lib/test_bedrock.py @@ -657,6 +657,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 8626b7b54f..226c34efad 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -261,6 +261,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) @@ -508,6 +519,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"]) + 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: @@ -519,6 +551,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, @@ -1811,6 +1856,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) @@ -2060,6 +2116,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"]) + 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: @@ -2071,6 +2148,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 950ce62421..97b5aabeb4 100644 --- a/tests/test_module_client.py +++ b/tests/test_module_client.py @@ -101,6 +101,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