fix(bedrock): route _require_boto3 through lazy_deps.ensure#24972
Closed
ChyuWei wants to merge 2 commits into
Closed
fix(bedrock): route _require_boto3 through lazy_deps.ensure#24972ChyuWei wants to merge 2 commits into
ChyuWei wants to merge 2 commits into
Conversation
The 2026-05-12 supply-chain-hardening pass moved boto3 out of [all]
and into LAZY_DEPS["provider.bedrock"], but _require_boto3 in
agent/bedrock_adapter.py still did a bare try/except ImportError and
told the user to run `pip install boto3` manually. Fresh installs
hitting a Bedrock chat therefore saw the manual-install message on
every first call even with lazy installs enabled.
Mirror the pattern already used by agent/anthropic_adapter.py's
_get_anthropic_sdk: call tools.lazy_deps.ensure("provider.bedrock",
prompt=False) before the import, tolerating ImportError (lazy_deps
module absent) and arbitrary exceptions (FeatureUnavailable when
lazy installs are opted out or offline). The existing manual-install
error survives as the final fallback.
Adds a regression test that patches tools.lazy_deps.ensure and asserts
_require_boto3 routes through it with the correct feature key, plus a
sibling test confirming the import path still succeeds when ensure()
itself raises.
…t it The 2026-05-12 policy removed boto3 from [all], so CI's default `pip install -e .[all,dev]` no longer pulls boto3 (see NousResearch#24601). The regression tests added for _require_boto3 must therefore be runnable without the real boto3 to actually guard the lazy-install path on every CI run, not just on dev machines that happen to have it. Stub boto3 via `patch.dict(sys.modules, {"boto3": <fake module>})` in both tests. Verified locally that they pass in both states — boto3 installed and fully uninstalled.
This was referenced May 13, 2026
Contributor
Author
|
Closing stale PR — will revisit if needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Restores the lazy-install path for
boto3. The 2026-05-12 supply-chain-hardening pass (#24515) relocatedboto3from the[all]extra intoLAZY_DEPS["provider.bedrock"], butagent/bedrock_adapter.py::_require_boto3was not updated alongside the other adapters and still does a baretry: import boto3 / except ImportError: raise. Fresh installs therefore hit the manual-install error on the very first Bedrock call, even withsecurity.allow_lazy_installsleft at the defaulttrue.This PR mirrors the pattern already used by
agent/anthropic_adapter.py::_get_anthropic_sdk— calltools.lazy_deps.ensure("provider.bedrock", prompt=False)before the import, swallowingImportError(thetools.lazy_depsmodule itself unavailable — e.g. minimal packaging) and genericException(FeatureUnavailablewhen the user has opted out of lazy installs or is offline). The existing manual-installImportErrorsurvives as the final fallback when lazy install is unavailable or fails.Related Issue
Fixes #24967
Type of Change
Changes Made
agent/bedrock_adapter.py—_require_boto3()now callstools.lazy_deps.ensure("provider.bedrock", prompt=False)before theimport boto3attempt, matching_get_anthropic_sdkinanthropic_adapter.py.tests/agent/test_bedrock_adapter.py— newTestRequireBoto3LazyInstallclass:test_require_boto3_calls_ensure_provider_bedrockpatchestools.lazy_deps.ensureand asserts it is called with("provider.bedrock", prompt=False).test_require_boto3_tolerates_ensure_failureconfirms the import still succeeds whenensure()itself raises (FeatureUnavailable fallback).boto3viapatch.dict(sys.modules, {"boto3": <fake>})so they run in the minimal CI environment that fix(install): use--extra allnot--all-extras; drop lazy-covered extras from [all] #24515 / test(bedrock,tts): skip baseline tests for optional deps removed from [all] #24601 established (i.e. without the realboto3installed).How to Test
boto3installed andsecurity.allow_lazy_installsat the defaulttrue, configure the Bedrock provider and start a chat.tools.lazy_deps.ensure→ pip-installboto3==1.42.89→ normal Converse call, not the manual-installImportError.Checklist
Code
fix(scope):,test(scope):)_require_boto3)pytest tests/agent/test_bedrock_adapter.py tests/tools/test_lazy_deps.py→ 163 passed, both with and withoutboto3installed)Documentation & Housekeeping
cli-config.yaml.example— N/A (no config keys changed)CONTRIBUTING.mdorAGENTS.md— N/A (no architecture change)Screenshots / Logs
Before this PR, on a fresh install with
security.allow_lazy_installs: true(default):The retry loop exhausts without ever attempting a lazy install. Also reproduced earlier in the session from the model-discovery path:
Both paths go through
_require_boto3, so the single-point fix in this PR resolves discovery and chat together.