Add Daytona workspace and examples #20
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds DaytonaWorkspace and supporting code, docs, and examples to provision and manage OpenHands agent-server sandboxes on Daytona Cloud, including sandbox lifecycle, preview link handling, health checks, optional dependency, and example scripts. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant WS as DaytonaWorkspace
participant Daytona as Daytona API
participant Sandbox as Sandbox
participant Server as Agent Server
participant Health as Health Poller
participant Client as HTTP Client
User->>WS: instantiate & enter
activate WS
WS->>Daytona: create sandbox (image, env, timeouts)
Daytona-->>WS: sandbox id
WS->>Sandbox: start agent-server command (port)
Sandbox->>Server: launch on port
WS->>Daytona: request preview link
Daytona-->>WS: preview URL (+ token)
WS->>Health: poll /health (retry loop)
loop until healthy or timeout
Health->>Server: GET /health
alt 200 OK
Server-->>Health: ready
else
Server-->>Health: not ready / error
end
end
WS->>Client: configure client (headers, token, API key)
WS-->>User: ready
deactivate WS
User->>WS: exit context
activate WS
WS->>Daytona: delete sandbox (unless keep_alive)
Daytona-->>WS: deleted
WS-->>User: cleanup done
deactivate WS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @enyst, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the OpenHands ecosystem by integrating Daytona Cloud as a remote workspace option. This allows agents to be deployed and run in isolated, cloud-backed sandboxes, streamlining development and testing workflows. The changes include the core Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Daytona Cloud-backed workspace, which is a significant addition. The implementation is generally solid, leveraging context managers for resource management and providing good examples. My review includes suggestions to improve the installation documentation, enhance robustness in error handling, and refactor for better efficiency by avoiding redundant client initializations. Overall, these are valuable changes.
examples/02_remote_agent_server/05_daytona_agent_server_client.py
Outdated
Show resolved
Hide resolved
| daytona_config = DaytonaConfig( | ||
| api_key=self.daytona_api_key, | ||
| target=self.daytona_target, | ||
| server_url=self.daytona_server_url, | ||
| ) | ||
| daytona = Daytona(config=daytona_config) |
There was a problem hiding this comment.
The Daytona client is initialized here and again in the __exit__ method. To improve efficiency and avoid code duplication, consider initializing the client once and reusing it.
You could, for example:
- Add
_daytona: Daytona | None = PrivateAttr(default=None)to the class attributes. - In
_start_sandbox, initialize it:self._daytona = Daytona(config=daytona_config). - In
__exit__, reuse it:if self._daytona: self._daytona.delete(self._sandbox).
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Greptile SummaryThis PR adds Key additions:
The code quality is high with proper error handling, cleanup on initialization failure, and comprehensive docstrings. No security issues detected. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant DaytonaWorkspace
participant Daytona API
participant Sandbox
participant Agent Server
User->>DaytonaWorkspace: __init__(daytona_api_key, server_image, ...)
DaytonaWorkspace->>DaytonaWorkspace: model_post_init()
DaytonaWorkspace->>DaytonaWorkspace: _start_sandbox()
DaytonaWorkspace->>Daytona API: create(CreateSandboxFromImageParams)
Daytona API->>Sandbox: provision sandbox from image
Sandbox-->>Daytona API: sandbox created
Daytona API-->>DaytonaWorkspace: Sandbox object
DaytonaWorkspace->>Sandbox: process.create_session("agent-server")
DaytonaWorkspace->>Sandbox: process.execute_session_command(runAsync=True)
Sandbox->>Agent Server: start openhands-agent-server
DaytonaWorkspace->>Sandbox: get_preview_link(port)
Sandbox-->>DaytonaWorkspace: preview URL + token
DaytonaWorkspace->>DaytonaWorkspace: set host, api_key, reset_client()
loop Health check retry (up to 120s)
DaytonaWorkspace->>Agent Server: GET /health
Agent Server-->>DaytonaWorkspace: 200 OK
end
DaytonaWorkspace-->>User: workspace ready
User->>DaytonaWorkspace: execute_command(cmd)
DaytonaWorkspace->>Agent Server: POST /api/bash/start_bash_command
Agent Server-->>DaytonaWorkspace: CommandResult
DaytonaWorkspace-->>User: result
User->>DaytonaWorkspace: __exit__()
DaytonaWorkspace->>DaytonaWorkspace: _cleanup_sandbox()
DaytonaWorkspace->>Daytona API: delete(sandbox)
Daytona API->>Sandbox: destroy sandbox
Sandbox-->>Daytona API: deleted
Daytona API-->>DaytonaWorkspace: success
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
examples/02_remote_agent_server/05_daytona_agent_server_client.py (1)
43-43: Useos.getenvwith assertion for better error messaging.As noted in a previous review, accessing
os.environ["DAYTONA_API_KEY"]directly raises a genericKeyErrorif the variable isn't set. Usingos.getenvwith an explicit assertion provides a clearer error message for users.🔎 Proposed fix
-DAYTONA_API_KEY = os.environ["DAYTONA_API_KEY"] +DAYTONA_API_KEY = os.getenv("DAYTONA_API_KEY") +assert DAYTONA_API_KEY, "DAYTONA_API_KEY environment variable must be set"openhands-workspace/openhands/workspace/daytona/workspace.py (1)
144-150: Consider caching the Daytona client to avoid recreation.The
Daytonaclient is instantiated both in_start_sandbox()and_cleanup_sandbox()with identical configuration. While this works, caching the client would be slightly more efficient.This is a minor optimization and the current approach is functionally correct.
Also applies to: 219-225
🧹 Nitpick comments (6)
examples/02_remote_agent_server/05_daytona_agent_server_client.py (2)
62-77: Health check logic is sound.The retry logic with timeout and error tracking is appropriate for this example script. The broad exception catch (line 73) is intentionally used to handle various connection errors during startup.
The static analysis hint about the long exception message (line 77) is a minor style issue. For production code, you might consider defining a custom exception class, but it's acceptable for an example script.
80-154: LGTM! Well-structured example with comprehensive health checks.The main workflow is logical and includes:
- Proper Daytona sandbox provisioning with timeout
- Agent-server startup inside the sandbox
- Preview link retrieval with token handling
- Both internal and external health checks
- Helpful timing instrumentation
Note that the script doesn't explicitly clean up the sandbox on exit, relying on Daytona's auto-delete interval (60 minutes). For a complete example, consider adding explicit cleanup in a
try/finallyblock or using a context manager pattern.Optional: Add explicit cleanup
Consider wrapping the sandbox lifecycle in a try/finally:
sandbox = None try: sandbox = daytona.create(params, timeout=600) # ... rest of main logic ... finally: if sandbox: try: daytona.delete(sandbox) except Exception: print("Warning: Failed to delete sandbox")examples/02_remote_agent_server/08_convo_with_daytona_workspace.py (2)
48-54: Consider using a dataclass or simple object for mutable state.Using a dict
{"ts": time.time()}to work around closure capture is functional but slightly awkward. Adataclasses.dataclassor a simple class would be cleaner for this pattern.That said, this is an example script and the current approach works fine.
77-78: Add a maximum wait time to prevent potential indefinite waiting.The loop waits while events are received within 2 seconds of each other. If events keep arriving continuously, this could wait indefinitely. Consider adding an upper bound:
start_wait = time.time() while time.time() - last_event_time["ts"] < 2.0 and time.time() - start_wait < 30.0: time.sleep(0.1)openhands-workspace/openhands/workspace/daytona/workspace.py (2)
201-208: Remove unusednoqadirective.Static analysis indicates
# noqa: BLE001is unused. The broad exception catch is appropriate here (health checks can fail in many ways during startup), but the noqa directive isn't suppressing anything.🔎 Proposed fix
- except Exception as e: # noqa: BLE001 + except Exception as e: raise RuntimeError(str(e)) from e
213-227: Consider catching a more specific exception type in cleanup.The broad
except Exceptioncatch is intentional for cleanup resilience (cleanup shouldn't raise), but catching a more specific exception type would be cleaner if the Daytona SDK defines one.However, for cleanup code where you want to ensure no exception propagates, this pattern is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
examples/02_remote_agent_server/05_daytona_agent_server_client.pyexamples/02_remote_agent_server/08_convo_with_daytona_workspace.pyopenhands-workspace/README.mdopenhands-workspace/openhands/workspace/__init__.pyopenhands-workspace/openhands/workspace/daytona/__init__.pyopenhands-workspace/openhands/workspace/daytona/workspace.pyopenhands-workspace/pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (2)
openhands-workspace/openhands/workspace/daytona/__init__.py (1)
openhands-workspace/openhands/workspace/daytona/workspace.py (1)
DaytonaWorkspace(40-228)
examples/02_remote_agent_server/08_convo_with_daytona_workspace.py (3)
openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py (1)
RemoteConversation(430-956)openhands-workspace/openhands/workspace/daytona/workspace.py (1)
DaytonaWorkspace(40-228)openhands-sdk/openhands/sdk/conversation/conversation_stats.py (1)
get_combined_metrics(58-62)
🪛 Ruff (0.14.10)
examples/02_remote_agent_server/05_daytona_agent_server_client.py
77-77: Avoid specifying long messages outside the exception class
(TRY003)
openhands-workspace/openhands/workspace/daytona/workspace.py
27-30: Avoid specifying long messages outside the exception class
(TRY003)
204-204: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
208-208: Avoid specifying long messages outside the exception class
(TRY003)
227-227: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (15)
openhands-workspace/README.md (1)
1-27: LGTM! Clear and comprehensive documentation.The README effectively documents all workspace implementations and provides clear installation instructions for the optional Daytona feature. The mention of lazy importing is helpful for users understanding the import behavior.
openhands-workspace/openhands/workspace/daytona/__init__.py (1)
1-6: LGTM! Standard package initialization.The package initialization follows Python conventions and is consistent with other workspace implementations in the codebase.
openhands-workspace/openhands/workspace/__init__.py (2)
14-14: LGTM! Proper TYPE_CHECKING import and public API export.The TYPE_CHECKING conditional import and
__all__export correctly exposeDaytonaWorkspaceas part of the public API while avoiding import-time dependency on the optionaldaytonapackage.Also applies to: 20-20
35-38: LGTM! Consistent lazy loading pattern.The lazy loading implementation for
DaytonaWorkspacefollows the same pattern asDockerDevWorkspace, ensuring the optionaldaytonadependency is only imported when explicitly requested.examples/02_remote_agent_server/05_daytona_agent_server_client.py (1)
50-59: LGTM! Clean helper function.The
_start_agent_serverfunction correctly starts the agent-server in an asynchronous session within the Daytona sandbox.openhands-workspace/pyproject.toml (1)
13-16: The optional dependency specification is correct.The
daytona>=0.128.1version exists on PyPI and is the latest stable release. No known security vulnerabilities were found for this package.examples/02_remote_agent_server/08_convo_with_daytona_workspace.py (4)
1-15: LGTM!Imports are well-organized and the logger setup is appropriate for this example script.
17-25: LGTM!The LLM configuration with environment variable validation and sensible defaults is appropriate for an example script.
32-46: LGTM!Good use of the context manager pattern for
DaytonaWorkspacewhich ensures proper cleanup. The configuration with environment variables provides flexibility.
69-86: Good resource cleanup pattern.The
try/finallyblock ensuringconversation.close()is always called is the correct pattern for managing the conversation lifecycle.openhands-workspace/openhands/workspace/daytona/workspace.py (5)
23-30: LGTM!The import error handling provides a clear message directing users to install the optional dependency.
40-107: LGTM!Field definitions are comprehensive with clear descriptions. The use of
PrivateAttrfor internal state (_sandbox,_daytona_preview_token) follows pydantic best practices.
109-134: LGTM!The client property with lazy initialization and cached storage is a good pattern. Timeout configuration and conditional header construction are well-handled.
136-142: LGTM!The cleanup-on-failure pattern in
model_post_initis excellent defensive programming. This ensures no sandbox resources are leaked if initialization fails partway through.
210-211: LGTM!The
__exit__method correctly delegates to_cleanup_sandbox, ensuring proper resource cleanup when the context manager exits.
Co-authored-by: openhands <openhands@all-hands.dev>
|
Addressed AI review items:
Remaining notes: Daytona client caching is a minor optimization; current approach keeps setup/teardown simple. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
examples/02_remote_agent_server/05_daytona_agent_server_client.py (3)
63-78: LGTM! Health check logic is solid.The polling logic with timeout and error tracking works correctly. Creating a new
httpx.Clienton each iteration is slightly inefficient, but acceptable for an example with a reasonable iteration count.💡 Optional: Reuse httpx.Client across iterations
def _wait_for_health(url: str, headers: dict[str, str]) -> float: deadline = time.time() + 120 start = time.monotonic() last_err: str | None = None - while time.time() < deadline: - try: - with httpx.Client(timeout=5.0, headers=headers) as client: - resp = client.get(f"{url.rstrip('/')}/health") - if resp.status_code == 200: - return time.monotonic() - start - last_err = f"HTTP {resp.status_code}: {resp.text[:200]}" - except Exception as e: # noqa: BLE001 - last_err = str(e) - time.sleep(1) + with httpx.Client(timeout=5.0, headers=headers) as client: + while time.time() < deadline: + try: + resp = client.get(f"{url.rstrip('/')}/health") + if resp.status_code == 200: + return time.monotonic() - start + last_err = f"HTTP {resp.status_code}: {resp.text[:200]}" + except Exception as e: # noqa: BLE001 + last_err = str(e) + time.sleep(1) raise RuntimeError(f"Agent-server did not become healthy: {last_err}")
124-136: Consider simplifying the local health check command.The Python one-liner is functional but complex and hard to read. For better maintainability, consider using a simpler approach.
💡 Simpler alternative using curl
sandbox.process.execute_session_command( "agent-server", SessionExecuteRequest( - command=( - 'python -c "import urllib.request; ' - 'urllib.request.urlopen("http://127.0.0.1:' - f"{AGENT_SERVER_PORT}" - '" + "/health", timeout=1).read(); ' - "print('OK')\"" - ), + command=f"curl -f http://127.0.0.1:{AGENT_SERVER_PORT}/health", ), timeout=120, )Note: This assumes
curlis available in the container. If not, the current approach is fine.
81-151: Consider adding explicit resource cleanup.While the sandbox has auto-delete configured, it's good practice to clean up resources explicitly, especially in example code that users might adapt.
🔍 Add try-finally for cleanup
def main() -> None: t0 = time.monotonic() daytona_config = DaytonaConfig( api_key=DAYTONA_API_KEY, target=DAYTONA_TARGET, api_url=DAYTONA_API_URL, ) daytona = Daytona(config=daytona_config) params = CreateSandboxFromImageParams( name=f"oh-agent-server-{int(time.time())}", image=AGENT_SERVER_IMAGE, public=False, auto_stop_interval=30, auto_delete_interval=60, env_vars={ "OH_ENABLE_VNC": "false", "LOG_JSON": "true", **({"SESSION_API_KEY": SESSION_API_KEY} if SESSION_API_KEY else {}), }, ) sandbox = daytona.create(params, timeout=600) - t1 = time.monotonic() - - _start_agent_server(sandbox, AGENT_SERVER_PORT) - t2 = time.monotonic() - - preview = sandbox.get_preview_link(AGENT_SERVER_PORT) - t3 = time.monotonic() - - daytona_headers: dict[str, str] = {} - if getattr(preview, "token", None): - daytona_headers["x-daytona-preview-token"] = preview.token - - headers = { - **daytona_headers, - **({"X-Session-API-Key": SESSION_API_KEY} if SESSION_API_KEY else {}), - } - - print(f"Agent-server preview URL: {preview.url}") - - sandbox.process.execute_session_command( - "agent-server", - SessionExecuteRequest( - command=( - 'python -c "import urllib.request; ' - 'urllib.request.urlopen("http://127.0.0.1:' - f"{AGENT_SERVER_PORT}" - '" + "/health", timeout=1).read(); ' - "print('OK')\"" - ), - ), - timeout=120, - ) - t4 = time.monotonic() - - external_ready_s = _wait_for_health(preview.url, headers) - t5 = time.monotonic() - - print("Agent-server is healthy") - print( - "Timings (seconds): " - f"provision={t1 - t0:.2f}, " - f"start_cmd={t2 - t1:.2f}, " - f"preview_link={t3 - t2:.2f}, " - f"local_ready={t4 - t3:.2f}, " - f"preview_ready={external_ready_s:.2f}, " - f"end_to_end={t5 - t0:.2f}" - ) + try: + t1 = time.monotonic() + + _start_agent_server(sandbox, AGENT_SERVER_PORT) + t2 = time.monotonic() + + preview = sandbox.get_preview_link(AGENT_SERVER_PORT) + t3 = time.monotonic() + + daytona_headers: dict[str, str] = {} + if getattr(preview, "token", None): + daytona_headers["x-daytona-preview-token"] = preview.token + + headers = { + **daytona_headers, + **({"X-Session-API-Key": SESSION_API_KEY} if SESSION_API_KEY else {}), + } + + print(f"Agent-server preview URL: {preview.url}") + + sandbox.process.execute_session_command( + "agent-server", + SessionExecuteRequest( + command=( + 'python -c "import urllib.request; ' + 'urllib.request.urlopen("http://127.0.0.1:' + f"{AGENT_SERVER_PORT}" + '" + "/health", timeout=1).read(); ' + "print('OK')\"" + ), + ), + timeout=120, + ) + t4 = time.monotonic() + + external_ready_s = _wait_for_health(preview.url, headers) + t5 = time.monotonic() + + print("Agent-server is healthy") + print( + "Timings (seconds): " + f"provision={t1 - t0:.2f}, " + f"start_cmd={t2 - t1:.2f}, " + f"preview_link={t3 - t2:.2f}, " + f"local_ready={t4 - t3:.2f}, " + f"preview_ready={external_ready_s:.2f}, " + f"end_to_end={t5 - t0:.2f}" + ) + finally: + # Clean up the sandbox + print("Cleaning up sandbox...") + sandbox.delete()
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/02_remote_agent_server/05_daytona_agent_server_client.pyopenhands-workspace/openhands/workspace/__init__.py
🚧 Files skipped from review as they are similar to previous changes (1)
- openhands-workspace/openhands/workspace/init.py
🧰 Additional context used
🧬 Code graph analysis (1)
examples/02_remote_agent_server/05_daytona_agent_server_client.py (1)
openhands-workspace/openhands/workspace/daytona/workspace.py (2)
_wait_for_health(201-208)client(110-125)
🪛 GitHub Actions: [Optional] Docs example
examples/02_remote_agent_server/05_daytona_agent_server_client.py
[error] 1-1: Undocumented example detected. Please add documentation for this example in the docs repo.
🪛 Ruff (0.14.10)
examples/02_remote_agent_server/05_daytona_agent_server_client.py
78-78: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-binary-and-test (ubuntu-latest)
- GitHub Check: build-binary-and-test (macos-latest)
🔇 Additional comments (6)
examples/02_remote_agent_server/05_daytona_agent_server_client.py (6)
1-19: LGTM! Clear and comprehensive documentation.The docstring effectively explains the purpose, prerequisites, workflow, and important notes about preview tokens and API keys.
21-32: LGTM! Imports and environment loading are appropriate.The imports cover all necessary functionality, and the dotenv loading with a configurable path is a good practice.
35-48: LGTM! Environment variable handling is robust.The code correctly uses
os.getenv()with clear assertions for required variables and provides sensible defaults. This addresses the previous review feedback about error handling.
51-60: LGTM! Server startup logic is clear and correct.The function appropriately uses async execution for the long-running server process, and the comment about the CLI entrypoint adds helpful context.
81-123: LGTM! Well-structured workflow with good timing instrumentation.The Daytona configuration, sandbox creation, and header setup are all correctly implemented. The timing measurements will be helpful for performance analysis.
154-155: LGTM! Standard entry point.The entry point follows Python conventions correctly.
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
This adds an optional Daytona Cloud-backed workspace (
DaytonaWorkspace) that provisions a Daytona sandbox from the published OpenHands agent-server image, starts the agent-server, and connects via Daytona preview links.Also includes:
examples/02_remote_agent_server/05_daytona_agent_server_client.py: minimal Daytona sandbox + healthcheck clientexamples/02_remote_agent_server/08_convo_with_daytona_workspace.py: runs twoconversation.run()calls in a Daytona workspace (FACTS.txt create + delete)Notes:
daytonaas an optional dependency:openhands-workspace[daytona]openhands-workspace/README.mdwith install instructionsCo-authored-by: openhands openhands@all-hands.dev
@enyst can click here to continue refining the PR
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.