feat: Add LocalRuntime and rename EventStreamRuntime to DockerRuntime #5284
+748
−551
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ntime - Add new LocalRuntime implementation that runs action_execution_server directly on host - Rename EventStreamRuntime to LocalDockerRuntime for clarity - Move runtime implementations to dedicated directories - Update documentation to reflect runtime changes
- Update imports to use LocalDockerRuntime and LocalRuntime - Add LocalRuntime to get_runtime_classes() - Update _close_test_runtime to handle LocalDockerRuntime
2 tasks
- Add proper type hints for server_process - Fix stdout access safety - Fix async/await type hints - Improve error handling
enyst
reviewed
Nov 26, 2024
… LocalDockerRuntime
|
New OpenHands update |
|
@openhands-agent This PR tests fail with
Fix this error. Keep your changes minimal. |
enyst
reviewed
Nov 29, 2024
enyst
reviewed
Nov 29, 2024
enyst
reviewed
Nov 29, 2024
enyst
reviewed
Nov 29, 2024
enyst
reviewed
Nov 29, 2024
|
@openhands-agent We have done a lot to fix the original. This PR is doing these:
The PR is mostly finished now. It has only one detail that went wrong: instead of the name LocalDockerRuntime, we now want the name DockerRuntime. Find all occurrences and fix them. Additionally, look also for the name EventStreamRuntime, and if you find any, update it to the new name, DockerRuntime. |
… LocalDockerRuntime
|
@xingyaoww I've been continuing to test out this branch, and it's generally been working great. I just hit an issue though, and I'm not sure if it's related to this branch or not (note that I hadn't pulled your latest changes when this happened): Excerpt Strangely, that grep command should return instantly, and does when I run it. Full logs |
|
I just ran the same headless command again, I see the same timeout. I'll next update to your latest changes. |
|
Update: I pulled the latest changes from this PR, and I'm no longer seeing the timeouts 🙌 |
|
Thanks @scosenza! Super happy to see that this PR has been working well for you! Yeah those timeouts are existing runtime-related bugs that we are trying to fix & improve, but yeah, we do have some recent changes that improve the stability there that might help! I think this PR is probably ready for merge (as long as I figure out correct way to run these in CI 🥹 ) |
enyst
reviewed
Jan 30, 2025
| async def execute_action(self, action: Action) -> Observation: | ||
| """Execute an action by sending it to the server.""" | ||
| if not self._runtime_initialized: | ||
| return ErrorObservation('Runtime not initialized') |
There was a problem hiding this comment.
Should we raise an exception, rather than send an ErrorObs? It's not the kind of thing that an agent can fix, and it would end up in its context otherwise
enyst
reviewed
Jan 30, 2025
| return ErrorObservation('Runtime not initialized') | ||
|
|
||
| if self.server_process is None or self.server_process.poll() is not None: | ||
| return ErrorObservation('Server process died') |
enyst
reviewed
Jan 30, 2025
Comment on lines
+312
to
+313
| except requests.exceptions.RequestException as e: | ||
| return ErrorObservation(f'Failed to execute action: {e}') |
There was a problem hiding this comment.
Suggested change
| except requests.exceptions.RequestException as e: | |
| return ErrorObservation(f'Failed to execute action: {e}') |
I wonder if maybe we don't need this? I think if there's an exception, it will fall to the controller, and there the reset should do it if it's necessary 🤔
enyst
reviewed
Jan 30, 2025
|
|
||
| This is the default runtime used within OpenHands. | ||
|
|
||
| ### Local Runtime | ||
|
|
||
| The Local Runtime is designed for direct execution on the local machine. Currently only supports running as `root`: |
There was a problem hiding this comment.
Suggested change
| The Local Runtime is designed for direct execution on the local machine. Currently only supports running as `root`: | |
| The Local Runtime is designed for direct execution on the local machine. Currently only supports running as the local user: |
enyst
reviewed
Jan 30, 2025
| from typing import Callable | ||
|
|
||
|
|
||
| class LogBuffer: |
There was a problem hiding this comment.
I think maybe we don't need this class, it's actually become LogStreamer now in main, and this seems to be the older implementation - not in use, and it doesn't seem used by LocalRuntime either 🤔
enyst
reviewed
Jan 30, 2025
| async def execute_action(self, action: Action) -> Observation: | ||
| """Execute an action by sending it to the server.""" | ||
| if not self._runtime_initialized: | ||
| return ErrorObservation('Runtime not initialized') |
There was a problem hiding this comment.
Suggested change
| return ErrorObservation('Runtime not initialized') | |
| raise AgentRuntimeDisconnectedError('Runtime not initialized') |
|
"Smart Manoj" on Kaggle has made a version of OpenHands that works under the K-prize conditions (which includes no Docker container execution available) and it has actually solved one of the problems using a local LLM (Qwen 2.5 Instruct 32b). https://www.kaggle.com/code/smartmanoj/openhands-fork-offline-version Don't know if it is based on this issue's PR or not. |
|
On Mac, seems like it also needs: Edited to add: |
|
I had some fun today with this branch! For some reason, of course Haiku decided to do on local what it usually doesn't do on docker, because of course! 😂 It works great. I didn't actually have errors yet, so I didn't get to see issues with those ErrorObs, but I think there would be, because they're also missing tool_id, so I still think we should raise exceptions instead. But I had fun with the experiment, this is a really cool runtime and I love it! Note: we could add to documentation a note about Prompt Confirmation, because hey it's a feature that will prove incredibly useful, even for adventurous souls. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new LocalRuntime implementation and renames EventStreamRuntime to LocalDockerRuntime for better clarity.
Changes
Benefits
Issue fixed: https://github.com/orgs/All-Hands-AI/projects/1?pane=issue&itemId=82616729&issue=All-Hands-AI%7COpenHands%7C3903
To run this PR locally, use the following command: