Skip to content
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

docs: update docker run cmd in readme.md #4786

Closed
wants to merge 1 commit into from

Conversation

Tomlili43
Copy link

End-user friendly description of the problem this fixes or functionality that this introduces
file sync between container and local machine

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below
    readme.md

Give a summary of what the PR does, explaining any non-trivial design decisions
update docker run cmd in readme.md


Link of any specific issues this addresses

@@ -42,13 +42,20 @@ docker pull docker.all-hands.dev/all-hands-ai/runtime:0.12-nikolaik

docker run -it --pull=always \
-e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:0.12-nikolaik \
-e SANDBOX_USER_ID=${SANDBOX_USER_ID:-1234} \
-e WORKSPACE_MOUNT_PATH=${WORKSPACE_BASE:-$PWD/workspace} \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were removed on purpose in the latest release. Is there a specific reason you have added them?

Copy link
Author

@Tomlili43 Tomlili43 Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problem: file sync from container to local machine
issue
solution add env variable to sync

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default way to run OpenHands in the README is not providing a local filesystem.
If you wanted to attach a local filesystem, you can use the document here which is also mentioned in this readme a bit below:
https://docs.all-hands.dev/modules/usage/runtimes#connecting-to-your-filesystem

This doc has all the options you show above so I don't think this PR is required in the main README.

if you run from a local machine, you can use the following command:
```
docker compose up
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added by a community member but is not supported officially by the maintainers.

@mamoodi
Copy link
Collaborator

mamoodi commented Nov 14, 2024

I'm going to close this PR if that's okay. The default option removes the local filesystem method on purpose. If you want to add a local filesystem, you have to follow this doc: https://docs.all-hands.dev/modules/usage/runtimes#connecting-to-your-filesystem

Which has the parameters you are adding to this PR. So everything seems as designed. Please let me know if I am mistaken and we can discuss.

@mamoodi mamoodi closed this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants