fix: resolve blocking boto3 calls in spreadsheet tools async context#287
Closed
SHIVANSH-ux-ys wants to merge 6 commits into
Closed
Conversation
…ate-Development#260) - Convert list_spreadsheets @tool to async def so Strands can await it - Replace _get_session_files asyncio-in-asyncio ThreadPoolExecutor dance with a direct await on repo.list_session_files() (async repo method) - Wrap blocking boto3 DynamoDB query in _get_kb_files with asyncio.to_thread() to avoid blocking the event loop - Cascade async upward to _find_file and analyze_spreadsheet in analyze_tool.py (both import the now-async helper functions) No behavioural changes; only the concurrency model is corrected.
…ool (Boise-State-Development#260) - TestGetSessionFiles: verifies repo is directly awaited (no ThreadPoolExecutor), non-tabular files are filtered, and exceptions return [] - TestGetKbFiles: verifies asyncio.to_thread is called for the DynamoDB query, incomplete documents are skipped, and missing env var returns [] - TestMakeListSpreadsheetsTool: verifies returned tool is async (iscoroutinefunction), empty-files message, and combined KB + session file listing Also sets asyncio_mode = auto in pytest.ini so pytest-asyncio picks up async test functions automatically.
…lopment#260 - Fix stdlib import order: asyncio before logging (PEP 8 / isort) - Hoist DYNAMODB_ASSISTANTS_TABLE_NAME guard out of _fetch() so we skip asyncio.to_thread entirely when env var is absent (avoids needless thread-pool allocation for a no-op path) - Expand _get_kb_files docstring to note asyncio.to_thread usage - test: remove unused imports (field, Optional) from test file - test: remove importlib.reload() from test_uses_asyncio_to_thread — reload invalidated the is_tabular_file mock by refreshing the module namespace after patching, causing the real function to run unchecked - test: narrow asyncio.to_thread patch to module scope (list_spreadsheets_tool.asyncio.to_thread) instead of global asyncio namespace to prevent patch leakage across tests
…se-State-Development#260) Adds scripts/verify_spreadsheet_async.py — a zero-dependency script reviewers can run instantly to confirm the async fix is working: python scripts/verify_spreadsheet_async.py Checks (all green, no AWS credentials required): 1. @tool wrapper is a coroutine function 2. _get_session_files awaits the repository directly (no thread tricks) 3. _get_kb_files routes the boto3 call through asyncio.to_thread 4. End-to-end listing combines KB + session files correctly 5. 5 concurrent tool calls complete without deadlock
Required by the Version Check CI gate. Synced across: - VERSION - backend/pyproject.toml - frontend/ai.client/package.json - infrastructure/package.json - README.md - backend/uv.lock
Author
|
Hi @DerrickF — thanks for raising #260! This PR resolves it by converting the blocking boto3 and ThreadPoolExecutor patterns to proper async/await. All 5 checks pass locally (run Could you please approve the workflow runs so CI can execute? The checks are currently showing |
…State-Development#260) _get_session_files does a lazy import inside the function body: from apis.shared.files.repository import get_file_upload_repository Patching it as a list_spreadsheets_tool attribute raised AttributeError in CI ('module has no attribute get_file_upload_repository'). Fix: patch at apis.shared.files.repository.get_file_upload_repository (the actual source) so the lazy import picks up the mock.
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
2 participants
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 & Why
The
list_spreadsheetsandanalyze_spreadsheettools were makingblocking boto3 / DynamoDB calls directly on the async event loop,
which could stall or deadlock the Strands agent runtime under concurrent
tool use. A secondary issue was a fragile
ThreadPoolExecutor + asyncio.run()workaround that broke under Python 3.11+ asyncio policy changes.
Closes #260
Changes
list_spreadsheets_tool.py— converted tool and helpers toasync def;replaced the ThreadPoolExecutor dance with
await repo.list_session_files();wrapped blocking
boto3.Table.query()inasyncio.to_thread()analyze_tool.py— cascaded async to_find_fileandanalyze_spreadsheet(both import the now-async helpers)
tests/agents/builtin_tools/test_list_spreadsheets_tool.py— new file;9 unit tests covering async behaviour, file filtering, and error handling
pytest.ini— addedasyncio_mode = autofor pytest-asyncioType of change