-
Notifications
You must be signed in to change notification settings - Fork 76
Separate DockerWorkspace and DockerDevWorkspace to fix import issue #1198
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
f1669ea
Separate DockerWorkspace and DockerDevWorkspace to fix import issue
openhands-agent f04577e
Refactor DockerWorkspace initialization to address code review feedback
openhands-agent d21b7b7
Fix pre-commit issues: Update examples to use DockerDevWorkspace and …
openhands-agent cb3d2e0
Merge branch 'main' into openhands-workspace-8fh70x68
xingyaoww 7420d7c
Move TargetType and PlatformType to SDK workspace models
openhands-agent 57bc104
Merge branch 'main' into openhands-workspace-8fh70x68
ryanhoangt 4ddce8c
improve DockerWorkspace server img validator
ryanhoangt 2eca86f
clean up
ryanhoangt 0ab6109
add deprecation warning
ryanhoangt 07c49da
update version in deprecation warning
ryanhoangt 251acdb
Merge branch 'main' into openhands-workspace-8fh70x68
ryanhoangt 5d06e86
update examples to use DockerWorkspace
ryanhoangt 5903632
update examples
ryanhoangt 2fd2483
update vscode example
ryanhoangt 0eb4e69
update examples & use inline import
ryanhoangt e027b23
update test
ryanhoangt 6d85df4
Make _get_image public as get_image
openhands-agent 906c795
use main-python
xingyaoww 4327901
Revert "use main-python"
xingyaoww 202945c
Merge branch 'main' into openhands-workspace-8fh70x68
xingyaoww File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,25 @@ | ||
| """OpenHands Workspace - Docker and container-based workspace implementations.""" | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| from .docker import DockerWorkspace | ||
| from .remote_api import APIRemoteWorkspace | ||
|
|
||
|
|
||
| if TYPE_CHECKING: | ||
| from .docker import DockerDevWorkspace | ||
|
|
||
| __all__ = [ | ||
| "DockerWorkspace", | ||
| "DockerDevWorkspace", | ||
| "APIRemoteWorkspace", | ||
| ] | ||
|
|
||
|
|
||
| def __getattr__(name: str): | ||
| """Lazy import DockerDevWorkspace to avoid build module imports.""" | ||
| if name == "DockerDevWorkspace": | ||
| from .docker import DockerDevWorkspace | ||
|
|
||
| return DockerDevWorkspace | ||
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,20 @@ | ||
| """Docker workspace implementation.""" | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| from .workspace import DockerWorkspace | ||
|
|
||
|
|
||
| __all__ = ["DockerWorkspace"] | ||
| if TYPE_CHECKING: | ||
| from .dev_workspace import DockerDevWorkspace | ||
|
|
||
| __all__ = ["DockerWorkspace", "DockerDevWorkspace"] | ||
|
|
||
|
|
||
| def __getattr__(name: str): | ||
| """Lazy import DockerDevWorkspace to avoid build module imports.""" | ||
| if name == "DockerDevWorkspace": | ||
| from .dev_workspace import DockerDevWorkspace | ||
|
|
||
| return DockerDevWorkspace | ||
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
99 changes: 99 additions & 0 deletions
99
openhands-workspace/openhands/workspace/docker/dev_workspace.py
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| """Docker development workspace with on-the-fly image building capability.""" | ||
|
|
||
| from typing import Literal | ||
|
|
||
| from pydantic import Field, model_validator | ||
|
|
||
| from openhands.agent_server.docker.build import ( | ||
| BuildOptions, | ||
| TargetType, | ||
| build, | ||
| ) | ||
|
|
||
| from .workspace import DockerWorkspace | ||
|
|
||
|
|
||
| class DockerDevWorkspace(DockerWorkspace): | ||
| """Docker workspace with on-the-fly image building capability. | ||
|
|
||
| This workspace extends DockerWorkspace to support building Docker images | ||
| on-the-fly from a base image. This is useful for development and testing | ||
| scenarios where you need to customize the agent server environment. | ||
|
|
||
| Note: This class requires the OpenHands SDK workspace structure and should | ||
| only be used within the OpenHands development environment or when you have | ||
| the full SDK source code available. | ||
|
|
||
| For production use cases with pre-built images, use DockerWorkspace instead. | ||
|
|
||
| Example: | ||
| with DockerDevWorkspace( | ||
| base_image="python:3.12", | ||
| target="source" | ||
| ) as workspace: | ||
| result = workspace.execute_command("ls -la") | ||
| """ | ||
|
|
||
| # Add base_image support | ||
| base_image: str | None = Field( | ||
| default=None, | ||
| description=( | ||
| "Base Docker image to build the agent server from. " | ||
| "Mutually exclusive with server_image." | ||
| ), | ||
| ) | ||
|
|
||
| # Add build-specific options | ||
| target: TargetType = Field( | ||
| default="source", description="Build target for the Docker image." | ||
| ) | ||
|
|
||
| # Override platform with stricter type for building | ||
| platform: Literal["linux/amd64", "linux/arm64"] = Field( # type: ignore[assignment] | ||
| default="linux/amd64", | ||
| description=( | ||
| "Platform for the Docker image. " | ||
| "Only linux/amd64 and linux/arm64 are supported for building." | ||
| ), | ||
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_images(self): | ||
| """Ensure exactly one of base_image or server_image is provided.""" | ||
| if (self.base_image is None) == (self.server_image is None): | ||
| raise ValueError( | ||
| "Exactly one of 'base_image' or 'server_image' must be set." | ||
| ) | ||
| if self.base_image and "ghcr.io/openhands/agent-server" in self.base_image: | ||
| raise ValueError( | ||
| "base_image cannot be a pre-built agent-server image. " | ||
| "Use server_image=... instead." | ||
| ) | ||
| return self | ||
|
|
||
| def _get_image(self) -> str: | ||
| """Build the image if base_image is provided, otherwise use server_image. | ||
|
|
||
| This overrides the parent method to add on-the-fly image building | ||
| capability. | ||
|
|
||
| Returns: | ||
| The Docker image tag to use. | ||
| """ | ||
| if self.base_image: | ||
| # Build the image from base_image | ||
| build_opts = BuildOptions( | ||
| base_image=self.base_image, | ||
| target=self.target, | ||
| platforms=[self.platform], | ||
| push=False, | ||
| ) | ||
| tags = build(opts=build_opts) | ||
| if not tags or len(tags) == 0: | ||
| raise RuntimeError("Build failed, no image tags returned") | ||
| return tags[0] | ||
| elif self.server_image: | ||
| # Use pre-built image | ||
| return self.server_image | ||
| else: | ||
| raise ValueError("Either base_image or server_image must be set") |
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
@ryanhoangt We should use
DockerWorkspaceas the default example (more straight forward) and leaveDockerDevWorkspaceas commented. Could you also add a few comments in the code explaining the difference betweenDockerDevWorkspaceandDockerWorkspaceand update corresponding docs in the docs repo?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.
Here's the PR in the
docsrepo: OpenHands/docs#143One downside of using
DockerWorkspaceIMO is that when testing all example scripts on a PR, it won't reflect the latest changes since the image is not newly built.