Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/openai/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Regenerate the module accessor fix from its authoritative source

On the next SDK regeneration, this module-level fix will be discarded because src/openai/__init__.py is generator-owned in scripts/castiron/generated-files.json, and regeneration replaces owned files without replaying handwritten patches. The parent file matches its recorded generated hash, whereas this commit changes it without a corresponding generation update. The ownership guidance in AGENTS.md, via CONTRIBUTING.md#modifyingadding-code, explicitly prohibits hand-editing generated output. Implement this accessor change in the authoritative generator/configuration and regenerate the output and inventory so the falsy-client regression does not return.

AGENTS.md reference: AGENTS.md:L7-L7

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for catching this. I confirmed that src/openai/__init__.py is listed in scripts/castiron/generated-files.json and that the contribution guide requires changes to generated files to come from the authoritative generator. My merge update included a direct edit, so this finding remains unresolved.

I could not find the authored compiler/configuration or an external regeneration workflow for this accessor in the public checkout. Could a maintainer point me to the appropriate source and regeneration procedure, or advise how this generated change should be handled? I will not edit the inventory by hand.

AI-assisted analysis, checked against the current inventory and contribution guide.


@_client.setter
def _client(self, value: _httpx.Client) -> None:
Expand Down
72 changes: 40 additions & 32 deletions src/openai/_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
Hughhhhcoder marked this conversation as resolved.
Resolved
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,
Expand All @@ -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
Comment thread
Hughhhhcoder marked this conversation as resolved.
Resolved
else SyncHttpxClientWrapper(
base_url=base_url,
timeout=timeout,
)
)

def is_closed(self) -> bool:
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions src/openai/lib/_client_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/openai/lib/bedrock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 31 additions & 0 deletions tests/lib/test_bedrock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down
90 changes: 90 additions & 0 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions tests/test_module_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down