mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 16:38:34 +08:00
fix(sandbox-spec): fall back to defaults when runtime-api has no warm runtimes (#15141)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
co-authored by
openhands
parent
af7efea64f
commit
d626f5285c
@@ -9,7 +9,9 @@ import httpx
|
||||
from fastapi import Request
|
||||
from pydantic import Field, PrivateAttr
|
||||
|
||||
from openhands.app_server.errors import SandboxError
|
||||
from openhands.app_server.sandbox.remote_sandbox_spec_service import (
|
||||
get_default_sandbox_specs,
|
||||
)
|
||||
from openhands.app_server.sandbox.sandbox_spec_models import (
|
||||
SandboxSpecInfo,
|
||||
SandboxSpecInfoPage,
|
||||
@@ -68,6 +70,11 @@ class DynamicRemoteSandboxSpecService(SandboxSpecService):
|
||||
specs.append(spec)
|
||||
name_to_spec[config['name']] = spec
|
||||
|
||||
# When runtime-api reports no warm runtimes, fall back to the default
|
||||
# sandbox specs so callers (e.g. conversation startup) can still proceed.
|
||||
if not specs:
|
||||
specs = get_default_sandbox_specs()
|
||||
|
||||
self._cached_specs = specs
|
||||
self._name_to_spec = name_to_spec
|
||||
self._cache_expires_at = now + self.cache_ttl_seconds
|
||||
@@ -90,8 +97,6 @@ class DynamicRemoteSandboxSpecService(SandboxSpecService):
|
||||
|
||||
async def get_default_sandbox_spec(self) -> SandboxSpecInfo:
|
||||
specs = await self._fetch_specs()
|
||||
if not specs:
|
||||
raise SandboxError('No warm runtime configs available from runtime-api.')
|
||||
if self.default_spec_name:
|
||||
spec = self._name_to_spec.get(self.default_spec_name)
|
||||
if spec is not None:
|
||||
|
||||
@@ -6,7 +6,6 @@ import httpx
|
||||
import pytest
|
||||
from starlette.datastructures import State
|
||||
|
||||
from openhands.app_server.errors import SandboxError
|
||||
from openhands.app_server.sandbox.dynamic_remote_sandbox_spec_service import (
|
||||
DynamicRemoteSandboxSpecService,
|
||||
DynamicRemoteSandboxSpecServiceInjector,
|
||||
@@ -165,17 +164,58 @@ class TestFetchSpecs:
|
||||
assert client.get.call_count == 1
|
||||
assert len(specs) == 3 # fresh data, not the single stale entry
|
||||
|
||||
async def test_empty_configs_list(self):
|
||||
"""An empty configs list must yield an empty spec list."""
|
||||
async def test_empty_configs_falls_back_to_default_specs(self):
|
||||
"""An empty configs list must fall back to get_default_sandbox_specs().
|
||||
|
||||
Without a warm runtime we still want the service to be usable, so we
|
||||
hand back the same default specs the static RemoteSandboxSpecService uses.
|
||||
"""
|
||||
from openhands.app_server.sandbox.remote_sandbox_spec_service import (
|
||||
get_default_sandbox_specs,
|
||||
)
|
||||
|
||||
ctx, _ = _make_async_client_mock(_make_http_response({'configs': []}))
|
||||
service = _make_service()
|
||||
|
||||
with patch('httpx.AsyncClient', return_value=ctx):
|
||||
specs = await service._fetch_specs()
|
||||
|
||||
assert specs == []
|
||||
# Compare field-by-field since SandboxSpecInfo has an auto-generated
|
||||
# created_at that differs between calls.
|
||||
expected = get_default_sandbox_specs()
|
||||
assert len(specs) == len(expected)
|
||||
for actual, want in zip(specs, expected, strict=False):
|
||||
assert actual.id == want.id
|
||||
assert actual.command == want.command
|
||||
assert actual.initial_env == want.initial_env
|
||||
assert actual.working_dir == want.working_dir
|
||||
# No warm runtime names means the default-name lookup will miss, which
|
||||
# is the intended signal for get_default_sandbox_spec to fall back too.
|
||||
assert service._name_to_spec == {}
|
||||
|
||||
async def test_fallback_specs_are_cached(self):
|
||||
"""Default fallback specs must be cached like normal results."""
|
||||
from openhands.app_server.sandbox.remote_sandbox_spec_service import (
|
||||
get_default_sandbox_specs,
|
||||
)
|
||||
|
||||
ctx, client = _make_async_client_mock(_make_http_response({'configs': []}))
|
||||
service = _make_service(cache_ttl_seconds=60)
|
||||
|
||||
with patch('httpx.AsyncClient', return_value=ctx):
|
||||
first = await service._fetch_specs()
|
||||
second = await service._fetch_specs()
|
||||
|
||||
assert client.get.call_count == 1
|
||||
assert first is second
|
||||
expected = get_default_sandbox_specs()
|
||||
assert len(first) == len(expected)
|
||||
for actual, want in zip(first, expected, strict=False):
|
||||
assert actual.id == want.id
|
||||
assert actual.command == want.command
|
||||
assert actual.initial_env == want.initial_env
|
||||
assert actual.working_dir == want.working_dir
|
||||
|
||||
async def test_http_error_propagates(self):
|
||||
"""An HTTP 500 from runtime-api must propagate as an exception."""
|
||||
ctx, _ = _make_async_client_mock(_make_http_response({}, status_code=500))
|
||||
@@ -308,14 +348,23 @@ class TestGetDefaultSandboxSpec:
|
||||
|
||||
assert spec.id == 'ghcr.io/openhands/agent-server:1.0.0'
|
||||
|
||||
async def test_raises_sandbox_error_when_no_specs_available(self):
|
||||
"""SandboxError must be raised when the endpoint returns no configs."""
|
||||
async def test_falls_back_to_default_spec_when_no_warm_runtimes(self):
|
||||
"""get_default_sandbox_spec must return a default spec (not raise) when
|
||||
the runtime-api endpoint returns no warm runtimes."""
|
||||
from openhands.app_server.sandbox.remote_sandbox_spec_service import (
|
||||
get_default_sandbox_specs,
|
||||
)
|
||||
|
||||
ctx, _ = _make_async_client_mock(_make_http_response({'configs': []}))
|
||||
service = _make_service(default_spec_name='v1_current')
|
||||
|
||||
with patch('httpx.AsyncClient', return_value=ctx):
|
||||
with pytest.raises(SandboxError, match='No warm runtime configs available'):
|
||||
await service.get_default_sandbox_spec()
|
||||
spec = await service.get_default_sandbox_spec()
|
||||
|
||||
# The configured warm-runtime name doesn't exist, so we get the first
|
||||
# of the fallback default specs rather than raising.
|
||||
expected = get_default_sandbox_specs()
|
||||
assert spec.id == expected[0].id
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user