Skip to content

Commit c676a8c

Browse files
committed
fix: address comments
1 parent ff39434 commit c676a8c

5 files changed

Lines changed: 31 additions & 26 deletions

File tree

src/strands/models/_openai_bedrock.py

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,23 +83,15 @@ def resolve_bedrock_client_args(
8383
) -> dict[str, Any]:
8484
"""Resolve a ``BedrockMantleConfig`` (plus optional ``client_args``) into OpenAI client kwargs.
8585
86-
Mints a fresh bearer token on every call. When ``client_args`` is provided, it must
87-
not contain ``base_url`` or ``api_key`` — those are always derived from the config.
86+
Mints a fresh bearer token on every call. Callers are expected to validate that
87+
``client_args`` does not contain ``base_url`` or ``api_key`` before calling this
88+
function (typically at ``__init__`` time for fail-fast behavior).
8889
8990
Raises:
90-
ValueError: If no region can be resolved, or if ``client_args`` contains
91-
``base_url`` or ``api_key``.
91+
ValueError: If no region can be resolved.
9292
ImportError: If ``aws-bedrock-token-generator`` is not installed.
9393
RuntimeError: If token minting fails (e.g. missing AWS credentials).
9494
"""
95-
if client_args:
96-
conflicting = [k for k in ("api_key", "base_url") if k in client_args]
97-
if conflicting:
98-
raise ValueError(
99-
f"client_args must not contain {conflicting} when bedrock_mantle_config is set; "
100-
"these are derived from the Mantle config automatically."
101-
)
102-
10395
region = _resolve_region(config)
10496

10597
# ``aws-bedrock-token-generator`` is included in the ``openai`` extras group but not in

src/strands/models/openai.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,13 @@ def __init__(
110110
raise ValueError("Only one of 'client' or 'client_args' should be provided, not both.")
111111
if bedrock_mantle_config is not None and client is not None:
112112
raise ValueError("'bedrock_mantle_config' cannot be combined with a pre-built 'client'.")
113+
if bedrock_mantle_config is not None and client_args:
114+
conflicting = [k for k in ("api_key", "base_url") if k in client_args]
115+
if conflicting:
116+
raise ValueError(
117+
f"client_args must not contain {conflicting} when bedrock_mantle_config is set; "
118+
"these are derived from the Mantle config automatically."
119+
)
113120

114121
self._custom_client = client
115122
self.client_args = client_args or {}

src/strands/models/openai_responses.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,14 @@ def __init__(
165165
self.client_args = client_args or {}
166166
self._bedrock_mantle_config = bedrock_mantle_config
167167

168+
if bedrock_mantle_config is not None and client_args:
169+
conflicting = [k for k in ("api_key", "base_url") if k in client_args]
170+
if conflicting:
171+
raise ValueError(
172+
f"client_args must not contain {conflicting} when bedrock_mantle_config is set; "
173+
"these are derived from the Mantle config automatically."
174+
)
175+
168176
logger.debug("config=<%s> | initializing", self.config)
169177

170178
def _resolve_client_args(self) -> dict[str, Any]:

tests/strands/models/test_openai.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1798,16 +1798,15 @@ def test_bedrock_mantle_config_merges_with_client_args(self, openai_client, mock
17981798
assert resolved["http_client"] is sentinel_http_client
17991799
assert resolved["default_headers"] == {"X-Trace-Id": "abc"}
18001800

1801-
def test_bedrock_mantle_config_rejects_base_url_in_client_args(self, openai_client, mock_provide_token):
1801+
def test_bedrock_mantle_config_rejects_base_url_in_client_args(self, openai_client):
18021802
"""client_args must not contain base_url or api_key when bedrock_mantle_config is set."""
18031803
_ = openai_client
1804-
model = OpenAIModel(
1805-
model_id="openai.gpt-oss-120b",
1806-
client_args={"base_url": "https://custom.example.com"},
1807-
bedrock_mantle_config={"region": "us-east-1"},
1808-
)
18091804
with pytest.raises(ValueError, match="client_args must not contain"):
1810-
model._resolve_client_args()
1805+
OpenAIModel(
1806+
model_id="openai.gpt-oss-120b",
1807+
client_args={"base_url": "https://custom.example.com"},
1808+
bedrock_mantle_config={"region": "us-east-1"},
1809+
)
18111810

18121811
def test_bedrock_mantle_config_requires_region(self, openai_client):
18131812
"""bedrock_mantle_config raises when no region can be resolved from config, session, or env."""

tests/strands/models/test_openai_responses.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,16 +1366,15 @@ def test_bedrock_mantle_config_merges_with_client_args(self, openai_client, mock
13661366
assert resolved["timeout"] == 42
13671367
assert resolved["http_client"] is sentinel_http_client
13681368

1369-
def test_bedrock_mantle_config_rejects_base_url_in_client_args(self, openai_client, mock_provide_token):
1369+
def test_bedrock_mantle_config_rejects_base_url_in_client_args(self, openai_client):
13701370
"""client_args must not contain base_url or api_key when bedrock_mantle_config is set."""
13711371
_ = openai_client
1372-
model = OpenAIResponsesModel(
1373-
model_id="openai.gpt-oss-120b",
1374-
client_args={"api_key": "should-not-be-here"},
1375-
bedrock_mantle_config={"region": "us-east-1"},
1376-
)
13771372
with pytest.raises(ValueError, match="client_args must not contain"):
1378-
model._resolve_client_args()
1373+
OpenAIResponsesModel(
1374+
model_id="openai.gpt-oss-120b",
1375+
client_args={"api_key": "should-not-be-here"},
1376+
bedrock_mantle_config={"region": "us-east-1"},
1377+
)
13791378

13801379
def test_bedrock_mantle_config_requires_region(self, openai_client):
13811380
"""bedrock_mantle_config raises when no region can be resolved from config, session, or env."""

0 commit comments

Comments
 (0)