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

Add support for .openhands/setup.sh script #5985

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rbren
Copy link
Collaborator

@rbren rbren commented Jan 2, 2025

This PR adds support for running .openhands/setup.sh after runtime initialization.

  • Add maybe_run_setup_script method to Runtime class
  • Run setup script after cloning repository
  • Use runtime file operations to read and execute script

The setup script is run in the runtime environment, similar to how other runtime operations work.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:4ae95d0-nikolaik   --name openhands-app-4ae95d0   docker.all-hands.dev/all-hands-ai/openhands:4ae95d0

@mamoodi
Copy link
Collaborator

mamoodi commented Jan 14, 2025

@rbren gentle ping to see if this is on your radar

@rbren
Copy link
Collaborator Author

rbren commented Jan 14, 2025

Yup still want to get this done! Haven't had time to test yet though

@rbren
Copy link
Collaborator Author

rbren commented Jan 14, 2025

@openhands-agent can you fix the merge conflicts? Look at diff vs main and understand the conflicts first

@openhands-agent
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@rbren rbren marked this pull request as ready for review January 23, 2025 16:50
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

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

Just to confirm, if the setup.sh script takes a long time to run, what does it look like in the frontend?

@rbren
Copy link
Collaborator Author

rbren commented Jan 23, 2025

We sit in this state for up to 120s
Screenshot 2025-01-23 at 12 00 28 PM

Copy link
Collaborator

@tofarr tofarr left a comment

Choose a reason for hiding this comment

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

🍰

@@ -223,6 +223,7 @@ async def _create_runtime(
repo_directory = await call_sync_from_async(
self.runtime.clone_repo, github_token, selected_repository
)
await call_sync_from_async(self.runtime.maybe_run_setup_script)
Copy link
Collaborator

Choose a reason for hiding this comment

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

When a session is restored, this will run a second/third time, even though the sandbox was restarted and not created? Not sure, and I didn't try it, it looks that way from the code.

(it's the status quo for clone_repo too, so not an issue for this PR, just seems worth thinking if this is the right behavior)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants