-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Simplify workspace mounting with SANDBOX_VOLUMES #8242
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
Conversation
Great! docker run -it -d \
-e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:c4a347b-nikolaik \
-e LOG_ALL_EVENTS=true \
-e CUSTOM_VOLUMES="/opt/openhands/workspace:/workspace:rw,/mnt/data1:/data:ro" \
-v /var/run/docker.sock:/var/run/docker.sock \
-v ~/.openhands-state:/.openhands-state \
-p 3000:3000 \
--add-host host.docker.internal:host-gateway \
--name openhands-app \
docker.all-hands.dev/all-hands-ai/openhands:c4a347b
|
@OpenHands pls run "make lint" to fix linter |
I'm on it! xingyaoww can track my progress at all-hands.dev |
I successfully addressed the PR comment request to run "make lint" to fix linter issues. Here's a summary of what I did:
The request has been addressed successfully, and the PR should now pass linting checks. The changes were minimal and only involved code formatting adjustments to meet the project's style guidelines. |
…cated WORKSPACE_BASE
Thanks. I was using those in my scripts. |
What if we just called them "volumes"? Ah, scratch that. It's not in |
hmm good idea. @OpenHands, pls move RUNTIME_VOLUMES to SandboxConfig and rename it to "volumes". And update all the documentation to use "SANDBOX_VOLUMES" instead of "RUNTIME_VOLUMES" |
I'm on it! xingyaoww can track my progress at all-hands.dev |
…l references to use SANDBOX_VOLUMES
Work SummaryI've successfully addressed the PR comment by moving the volume configuration from AppConfig to SandboxConfig and updating all references throughout the codebase. Changes Made:
All tests are now passing, and the changes have been committed and pushed to the Note: The PR comment mentioned "RUNTIME_VOLUMES", but the actual field in the code was "custom_volumes", so I updated all references accordingly. |
Once this merges, there will be a period where the docs are wrong because the feature will only be available on main. |
@OpenHands please improve the documentation to tell the user that "/workspace" is the path where the agent will work on by default and they should move folder there if they want the agent to modify the file there. If they want to mount some read-only data for agent to use, they can mount to a different folder than /workspace and then explicit instruct agent to use it, for example, |
I'm on it! xingyaoww can track my progress at all-hands.dev |
I've improved the documentation to address the PR comment that requested clarification about the Changes Made:
All changes have been committed and pushed to the |
@OpenHands can you read the git diff against main, and then use github API to update the PR description here. Make sure you follow the pr template in this repo. |
I'm on it! xingyaoww can track my progress at all-hands.dev |
I've successfully updated the PR description for PR #8242 "Simplify workspace mounting with SANDBOX_VOLUMES" following the repository's PR template. Here's what I did:
The updated PR description now properly follows the repository's template and clearly explains the new The PR description update was successfully confirmed via the GitHub API. |
I noticed that while this PR does a great job updating many documentation files to use the new
How to fix this:
export ANTHROPIC_API_KEY=<your_api_key>
poetry run python docs/translation_updater.py This will detect changes in the English documentation and automatically update all translations using Claude. According to the script, it supports French, Simplified Chinese, Brazilian Portuguese, and Japanese translations. This approach would ensure consistency across all documentation and make sure users are properly informed about the deprecated parameters regardless of which language they are using. |
@OpenHands please review @neubig's lastest comment and work on that |
I'm on it! xingyaoww can track my progress at all-hands.dev |
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.
Looks great to me other than some stale documentation, commented above!
I've successfully addressed @neubig's comment by updating all translation files to mark the workspace_* parameters as deprecated and add the new SANDBOX_VOLUMES parameter. Here's what I did:
All the translation files now consistently mark the workspace_* parameters as deprecated and reference the new SANDBOX_VOLUMES parameter, which completes the work requested in @neubig's comment. The PR #8242 has been updated with these changes. |
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.
Thank you!
End-user friendly description of the problem this fixes or functionality this introduces.
This PR introduces a simpler and more flexible way to mount directories into the OpenHands Docker runtime. Users can now mount multiple directories with different access permissions (read-write or read-only) using the new
SANDBOX_VOLUMES
environment variable. This is particularly useful for accessing large datasets, pre-trained models, or reference code that should be available to the agent but kept separate from the main workspace.Summarize what the PR does, explaining any non-trivial design decisions.
This PR simplifies the workspace mounting process in OpenHands Docker runtime by:
SANDBOX_VOLUMES
environment variable that uses a standard Docker-like format:host_path:container_path[:mode]
/path1:/workspace:rw,/path2:/data:ro
:ro
suffixWORKSPACE_*
variables (marked as deprecated)Design decisions:
SANDBOX_VOLUMES
is considered the primary workspace for backward compatibility/workspace
by default, so users should mount their project directory there/data
)Link of any specific issues this addresses:
Closes #8239
To run this PR locally, use the following command: