-
Notifications
You must be signed in to change notification settings - Fork 231
Add durable Strands sandbox support #1768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7fcd20e
59f3dfc
2b0f989
343d0d7
ef2c383
da17bd3
37031bb
b8117f7
18d709b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| from collections.abc import AsyncGenerator, Callable | ||
| from collections.abc import AsyncGenerator, Awaitable, Callable | ||
| from contextlib import asynccontextmanager | ||
| from dataclasses import replace | ||
| from datetime import timedelta | ||
|
|
||
| from strands.models import BedrockModel, Model | ||
| from strands.sandbox import Sandbox | ||
| from strands.tools.mcp import MCPClient | ||
|
|
||
| from temporalio.contrib.pydantic import pydantic_data_converter | ||
|
|
@@ -14,6 +15,10 @@ | |
|
|
||
| from ._failure_converter import StrandsFailureConverter | ||
| from ._model_activity import ModelActivity | ||
| from ._sandbox_activity import ( | ||
| SandboxActivities, | ||
| SandboxWorkflowContext, | ||
| ) | ||
| from ._temporal_mcp_client import ( | ||
| _evict_connection, | ||
| build_call_tool_activity, | ||
|
|
@@ -39,16 +44,32 @@ class StrandsPlugin(SimplePlugin): | |
| ``mcp_connection_idle_timeout`` controls how long a worker-process MCP | ||
| connection is kept open between ``call-tool`` activities before it is | ||
| disconnected; the timer resets on every reuse. Defaults to 5 minutes. | ||
|
|
||
| When ``sandboxes`` is supplied, registers one stable set of activities that | ||
| dispatches each operation by sandbox name. Each factory receives the | ||
| requesting Workflow run's context and may return a sandbox directly or | ||
| awaitably. Worker-local adapters are cached by sandbox name and Workflow | ||
| chain until ``sandbox_cache_idle_timeout`` elapses. Use the same name in | ||
| workflow-side ``TemporalSandbox(name)`` instances. | ||
| """ | ||
|
|
||
| def __init__( | ||
| self, | ||
| *, | ||
| models: dict[str, Callable[[], Model]] | None = None, | ||
| mcp_clients: dict[str, Callable[[], MCPClient]] | None = None, | ||
| sandboxes: dict[ | ||
| str, | ||
| Callable[ | ||
| [SandboxWorkflowContext], | ||
| Sandbox | Awaitable[Sandbox], | ||
| ], | ||
| ] | ||
| | None = None, | ||
| mcp_connection_idle_timeout: timedelta | None = None, | ||
| sandbox_cache_idle_timeout: timedelta | None = None, | ||
| ) -> None: | ||
| """Build the plugin from optional model and MCP transport factories. | ||
| """Build the plugin from optional model, MCP, and sandbox factories. | ||
|
|
||
| If ``models`` is omitted, registers a single ``BedrockModel()`` factory | ||
| under the name ``"bedrock"``, matching Strands' own implicit default. | ||
|
|
@@ -62,6 +83,14 @@ def __init__( | |
| ma = ModelActivity(models, default_name=default_name) | ||
| activities.extend([ma.invoke_model, ma.invoke_model_streaming]) | ||
|
|
||
| sandbox_activities = ( | ||
| SandboxActivities(sandboxes, sandbox_cache_idle_timeout) | ||
| if sandboxes | ||
| else None | ||
| ) | ||
| if sandbox_activities is not None: | ||
| activities.extend(sandbox_activities.activities()) | ||
|
|
||
| mcp_clients = mcp_clients or {} | ||
| for server, client_factory in mcp_clients.items(): | ||
| activities.append( | ||
|
|
@@ -80,6 +109,8 @@ async def run_context() -> AsyncGenerator[None, None]: | |
| try: | ||
| yield | ||
| finally: | ||
| if sandbox_activities is not None: | ||
| await sandbox_activities.aclose() | ||
|
Comment on lines
+112
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When one client-level Useful? React with 👍 / 👎. |
||
| for server in mcp_clients: | ||
| await _evict_connection(server) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we are using SandboxWorkflowContext do we need the name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name selects the corresponding sandbox defined in StrandsPlugin(sandboxes=...), whereas the context contains the Workflow ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the factory function handle all sandbox retrievals instead of name map?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name map is good because it's what we already do for MCP, and it allows us to have multiple sandboxes in the same environment.