Skip to content

Commit 4e5837a

Browse files
committed
fix: address bot's comments
1 parent 6281721 commit 4e5837a

5 files changed

Lines changed: 21 additions & 47 deletions

File tree

src/strands/models/_openai_bedrock.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22
33
Converts an ``aws_config`` dict into the ``base_url`` and ``api_key`` that the
44
OpenAI Python SDK consumes. Tokens are minted on demand via
5-
``aws_bedrock_token_generator.provide_token`` so long-running agents keep working
6-
past the bearer token's maximum lifetime.
5+
``aws_bedrock_token_generator.provide_token`` so long-running agents survive the
6+
bearer token's maximum lifetime.
77
8-
``aws_bedrock_token_generator`` is imported lazily inside
9-
:func:`resolve_bedrock_client_args` so that users of OpenAI-compatible extras
10-
(``sagemaker``, ``litellm``, bare ``openai`` without Mantle) don't pay an
11-
``ImportError`` just for importing the model class.
8+
``aws_bedrock_token_generator`` is imported lazily so that extras which reuse
9+
the OpenAI package without pulling the Mantle dependency do not hit an
10+
``ImportError`` at module load.
1211
"""
1312

1413
from __future__ import annotations
@@ -25,7 +24,7 @@ class AwsConfig(TypedDict, total=False):
2524
"""AWS-side config for reaching Bedrock Mantle via an OpenAI-compatible client.
2625
2726
Attributes:
28-
region: AWS region hosting the Bedrock Mantle endpoint (required).
27+
region: AWS region hosting the Bedrock Mantle endpoint.
2928
credentials_provider: Optional botocore ``CredentialProvider`` forwarded to
3029
``provide_token``. Defaults to the AWS credential chain.
3130
expiry: Optional ``timedelta`` for the bearer token's lifetime, forwarded
@@ -61,8 +60,7 @@ def resolve_bedrock_client_args(aws_config: AwsConfig, client_args: dict[str, An
6160
"Install it with: pip install strands-agents[openai]"
6261
) from e
6362

64-
# Only forward optional kwargs when explicitly set, so provide_token's own
65-
# defaults apply. Passing expiry=None in particular crashes the library.
63+
# Only forward kwargs the user set; provide_token rejects expiry=None.
6664
token_kwargs: dict[str, Any] = {"region": region}
6765
if "credentials_provider" in aws_config:
6866
token_kwargs["aws_credentials_provider"] = aws_config["credentials_provider"]

src/strands/models/openai.py

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -89,27 +89,20 @@ def __init__(
8989
Note: The client should not be shared across different asyncio event loops.
9090
client_args: Arguments for the OpenAI client (legacy approach).
9191
For a complete list of supported arguments, see https://pypi.org/project/openai/.
92-
May be combined with ``aws_config``; transport-level options like ``http_client``,
93-
``timeout``, or ``default_headers`` are preserved, while ``base_url`` and
94-
``api_key`` are always overridden by ``aws_config`` when both are set.
92+
May be combined with ``aws_config``; when both are set, ``aws_config`` overrides
93+
``base_url`` and ``api_key`` only.
9594
aws_config: Route requests through Amazon Bedrock's Mantle (OpenAI-compatible)
96-
endpoint. Provide ``{"region": "us-east-1"}`` at minimum. Accepts optional
97-
``credentials_provider`` (a botocore ``CredentialProvider``) and ``expiry``
98-
(a ``datetime.timedelta`` up to 12h). When set, a fresh bearer token is minted
99-
on every request via ``aws-bedrock-token-generator`` and the OpenAI client is
100-
pointed at ``https://bedrock-mantle.<region>.api.aws/v1``. Cannot be combined
101-
with a pre-built ``client``.
95+
endpoint. See :class:`AwsConfig` for accepted keys. When set, a fresh bearer
96+
token is minted on every request. Cannot be combined with a pre-built ``client``.
10297
**model_config: Configuration options for the OpenAI model.
10398
10499
Raises:
105-
ValueError: If ``client`` is combined with ``client_args`` or ``aws_config``,
106-
or if ``aws_config`` is missing a region.
100+
ValueError: If ``client`` is combined with ``client_args`` or ``aws_config``.
107101
"""
108102
validate_config_keys(model_config, self.OpenAIConfig)
109103
self.config = dict(model_config)
110104

111-
# Validate that client configuration methods are mutually exclusive where they conflict.
112-
# client_args + aws_config is allowed — aws_config will override base_url / api_key only.
105+
# client_args + aws_config is allowed; aws_config overrides base_url / api_key only.
113106
client_args_provided = client_args is not None and len(client_args) > 0
114107
if client is not None and client_args_provided:
115108
raise ValueError("Only one of 'client' or 'client_args' should be provided, not both.")
@@ -125,9 +118,7 @@ def __init__(
125118
def _resolve_client_args(self) -> dict[str, Any]:
126119
"""Return the kwargs to pass to ``openai.AsyncOpenAI`` for the current request.
127120
128-
When ``aws_config`` is set, a fresh Bedrock Mantle bearer token is minted on every
129-
call and ``base_url`` / ``api_key`` are overridden. Any other entries from
130-
``client_args`` (e.g. ``http_client``, ``timeout``) are preserved.
121+
Delegates to :func:`resolve_bedrock_client_args` when ``aws_config`` is set.
131122
"""
132123
if self._aws_config is not None:
133124
return resolve_bedrock_client_args(self._aws_config, self.client_args)
@@ -619,7 +610,6 @@ async def _get_client(self) -> AsyncIterator[Any]:
619610
# Use the injected client (caller manages lifecycle)
620611
yield self._custom_client
621612
else:
622-
# Create a new client from resolved args (static client_args or freshly-minted Bedrock creds).
623613
# We initialize an OpenAI context on every request so as to avoid connection sharing in the underlying
624614
# httpx client. The asyncio event loop does not allow connections to be shared. For more details, please
625615
# refer to https://github.com/encode/httpx/discussions/2959.

src/strands/models/openai_responses.py

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,19 +152,12 @@ def __init__(
152152
Args:
153153
client_args: Arguments for the OpenAI client.
154154
For a complete list of supported arguments, see https://pypi.org/project/openai/.
155-
May be combined with ``aws_config``; transport-level options like ``http_client``,
156-
``timeout``, or ``default_headers`` are preserved, while ``base_url`` and
157-
``api_key`` are always overridden by ``aws_config`` when both are set.
155+
May be combined with ``aws_config``; when both are set, ``aws_config`` overrides
156+
``base_url`` and ``api_key`` only.
158157
aws_config: Route requests through Amazon Bedrock's Mantle (OpenAI-compatible)
159-
endpoint. Provide ``{"region": "us-east-1"}`` at minimum. Accepts optional
160-
``credentials_provider`` (a botocore ``CredentialProvider``) and ``expiry``
161-
(a ``datetime.timedelta`` up to 12h). When set, a fresh bearer token is minted
162-
on every request via ``aws-bedrock-token-generator`` and the OpenAI client is
163-
pointed at ``https://bedrock-mantle.<region>.api.aws/v1``.
158+
endpoint. See :class:`AwsConfig` for accepted keys. When set, a fresh bearer
159+
token is minted on every request.
164160
**model_config: Configuration options for the OpenAI Responses API model.
165-
166-
Raises:
167-
ValueError: If ``aws_config`` is missing a region.
168161
"""
169162
validate_config_keys(model_config, self.OpenAIResponsesConfig)
170163
self.config = dict(model_config)
@@ -177,9 +170,7 @@ def __init__(
177170
def _resolve_client_args(self) -> dict[str, Any]:
178171
"""Return the kwargs to pass to ``openai.AsyncOpenAI`` for the current request.
179172
180-
When ``aws_config`` is set, a fresh Bedrock Mantle bearer token is minted on every
181-
call and ``base_url`` / ``api_key`` are overridden. Any other entries from
182-
``client_args`` (e.g. ``http_client``, ``timeout``) are preserved.
173+
Delegates to :func:`resolve_bedrock_client_args` when ``aws_config`` is set.
183174
"""
184175
if self._aws_config is not None:
185176
return resolve_bedrock_client_args(self._aws_config, self.client_args)

tests/strands/models/test_openai.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,8 +1718,6 @@ def test_format_request_messages_multiple_tool_calls_with_images():
17181718

17191719

17201720
class TestOpenAIModelAwsConfig:
1721-
"""Tests for the Bedrock Mantle pathway via the aws_config kwarg."""
1722-
17231721
@pytest.fixture
17241722
def mock_provide_token(self):
17251723
with unittest.mock.patch("aws_bedrock_token_generator.provide_token") as mock:
@@ -1731,12 +1729,11 @@ def test_aws_config_sets_base_url_and_api_key(self, openai_client, mock_provide_
17311729
_ = openai_client
17321730
model = OpenAIModel(model_id="openai.gpt-oss-120b", aws_config={"region": "us-east-1"})
17331731

1734-
# api_key is resolved per-request (lazy), so check via the resolved client_args at call time
1732+
# Token is minted lazily per request, so inspect the resolved kwargs.
17351733
resolved = model._resolve_client_args()
17361734
assert resolved["base_url"] == "https://bedrock-mantle.us-east-1.api.aws/v1"
17371735
assert resolved["api_key"] == "bedrock-api-key-deadbeef&Version=1"
1738-
# Only region is forwarded when the user did not set optional kwargs,
1739-
# so provide_token's own defaults (e.g. 12h expiry) apply.
1736+
# Optional kwargs aren't forwarded so provide_token's own defaults apply.
17401737
mock_provide_token.assert_called_once_with(region="us-east-1")
17411738

17421739
def test_aws_config_forwards_credentials_provider_and_expiry(self, openai_client, mock_provide_token):

tests/strands/models/test_openai_responses.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,8 +1306,6 @@ async def test_fallback_logs_debug(self, model, openai_client, messages, caplog)
13061306

13071307

13081308
class TestOpenAIResponsesModelAwsConfig:
1309-
"""Tests for the Bedrock Mantle pathway via the aws_config kwarg."""
1310-
13111309
@pytest.fixture
13121310
def mock_provide_token(self):
13131311
with unittest.mock.patch("aws_bedrock_token_generator.provide_token") as mock:

0 commit comments

Comments
 (0)