feat: add argument --base-container-image to resolver #7612
Conversation
Tetsu-is
changed the title
Tetsu-is
changed the title
openhands/resolver/resolve_issue.py
Outdated
| if runtime_container_image is None and not my_args.is_experimental: | ||
| runtime_container_image = ( | ||
| f'ghcr.io/all-hands-ai/runtime:{openhands.__version__}-nikolaik' | ||
| ) |
There was a problem hiding this comment.
Thanks @Tetsu-is ! Could you explain why these lines were removed?
There was a problem hiding this comment.
@neubig
Of course.
In the existing implementation, custom sandbox will be build only when runtime_container_image is None & `base_container_image is Not None (openhands/runtime/impl/docker/docker_runtime.py).
if self.runtime_container_image is None:
if self.base_container_image is None:
raise ValueError(
'Neither runtime container image nor base container image is set'
)
self.send_status_message('STATUS$STARTING_CONTAINER')
self.runtime_container_image = build_runtime_image(
self.base_container_image,
...
)When executed likepython -m openhands.resolver.resolve_issue --base-container-image BASE_IMAGE, the runtime_container_image is not provided, and only the base_container_image is given. At that time, if 'ghcr.io/all-hands-ai/runtime:{openhands.version}-nikolaik' is assigned to runtime_container_image, the build of the custom sandbox will not be executed.
My ideal scenario is below.
| runtime-container-image | base-container-image | result |
|---|---|---|
| given | given | error |
| given | not given | build sandbox with runtime-container-image |
| not given | given | build sandbox with base-container-image |
| not given | not given | error or build sandbox with default nikolaik |
When runtime-container-image is not given, do you think ghcr.io/all-hands-ai/runtime:{openhands.__version__}-nikolaik should be set as default?
There was a problem hiding this comment.
Please correct me if I'm misunderstanding, not given + not given is the default case, when the user doesn't have a special requirement for a particular image, so they run the resolver without setting any of those. In that case, the result should be "build sandbox with default nikolaik".
Unless they set is_experimental, in which case it shouldn't set it, so that it builds from source, which will mean build from main. (experimental installs openhands main instead of the latest released version IIRC)
Do you think this behavior should change, or actually, does it change in this PR?
There was a problem hiding this comment.
@enyst
Thank you for your advice. I didn't understand the meaning of is_experimental flag.
I agree with you . Default behavior should be building sandbox with default nikolaik image.
I will change it in this PR.
There was a problem hiding this comment.
when not specifying base-conatiner-image and runtime-container-image, use default nikolaik.
| builder_cmd = ['docker', 'buildx', 'use', 'default'] | ||
| subprocess.Popen( | ||
| builder_cmd, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| universal_newlines=True, | ||
| ) | ||
|
|
There was a problem hiding this comment.
If the default builder instance is not specified, the sandbox build will fail because it cannot reference the base container image.(when I tried in my repository)
End-user friendly description of the problem this fixes or functionality that this introduces.
Enabled custom sandbox for resolver. You can apply base image with
--base-container image YOUR_BASE_CONTAINER_IMAGEexample
Give a summary of what the PR does, explaining any non-trivial design decisions.
I added
base_container_imagesetting for resolver in order to enable github actions to use any programming language.Instead, we have to specify
-runtime-container-imageeven when we don't use custom sandbox(I will fix it so that user dont have to specify when using defaultt runtime image)--base-container-imageto resolver.resolve_issue.py--base-container-imageto resolver.resolve_all_issues.pyLink of any specific issues this addresses.
I haven't created issue about this topic.