From dee89462c233965c63dbe78c618e085ddaa11086 Mon Sep 17 00:00:00 2001 From: Ray Myers Date: Thu, 5 Jun 2025 11:19:53 -0500 Subject: [PATCH] Improve type coverage for nested runtime (#8921) --- dev_config/python/.pre-commit-config.yaml | 3 ++- .../runtime/impl/docker/docker_runtime.py | 9 +++++--- openhands/server/config/server_config.py | 4 ++-- .../docker_nested_conversation_manager.py | 22 ++++++++++--------- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/dev_config/python/.pre-commit-config.yaml b/dev_config/python/.pre-commit-config.yaml index 8cb0c24359..fbb425eec0 100644 --- a/dev_config/python/.pre-commit-config.yaml +++ b/dev_config/python/.pre-commit-config.yaml @@ -37,7 +37,8 @@ repos: hooks: - id: mypy additional_dependencies: - [types-requests, types-setuptools, types-pyyaml, types-toml] + [types-requests, types-setuptools, types-pyyaml, types-toml, types-docker, lxml] + # To see gaps add `--html-report mypy-report/` entry: mypy --config-file dev_config/python/mypy.ini openhands/ always_run: true pass_filenames: false diff --git a/openhands/runtime/impl/docker/docker_runtime.py b/openhands/runtime/impl/docker/docker_runtime.py index 74ac002096..3f0e1c72c9 100644 --- a/openhands/runtime/impl/docker/docker_runtime.py +++ b/openhands/runtime/impl/docker/docker_runtime.py @@ -1,6 +1,7 @@ import os from functools import lru_cache from typing import Callable +import typing from uuid import UUID import docker @@ -164,7 +165,7 @@ class DockerRuntime(ActionExecutionClient): f'Container started: {self.container_name}. VSCode URL: {self.vscode_url}', ) - if DEBUG_RUNTIME: + if DEBUG_RUNTIME and self.container: self.log_streamer = LogStreamer(self.container, self.log) else: self.log_streamer = None @@ -281,7 +282,7 @@ class DockerRuntime(ActionExecutionClient): self.api_url = f'{self.config.sandbox.local_runtime_url}:{self._container_port}' use_host_network = self.config.sandbox.use_host_network - network_mode: str | None = 'host' if use_host_network else None + network_mode: typing.Literal['host'] | None = 'host' if use_host_network else None # Initialize port mappings port_mapping: dict[str, list[dict[str, str]]] | None = None @@ -353,6 +354,8 @@ class DockerRuntime(ActionExecutionClient): command = self.get_action_execution_server_startup_command() try: + if self.runtime_container_image is None: + raise ValueError("Runtime container image is not set") self.container = self.docker_client.containers.run( self.runtime_container_image, command=command, @@ -364,7 +367,7 @@ class DockerRuntime(ActionExecutionClient): name=self.container_name, detach=True, environment=environment, - volumes=volumes, + volumes=volumes, # type: ignore device_requests=( [docker.types.DeviceRequest(capabilities=[['gpu']], count=-1)] if self.config.sandbox.enable_gpu diff --git a/openhands/server/config/server_config.py b/openhands/server/config/server_config.py index bfdb62404b..f020d2c1da 100644 --- a/openhands/server/config/server_config.py +++ b/openhands/server/config/server_config.py @@ -48,12 +48,12 @@ class ServerConfig(ServerConfigInterface): return config -def load_server_config(): +def load_server_config() -> ServerConfig: config_cls = os.environ.get('OPENHANDS_CONFIG_CLS', None) logger.info(f'Using config class {config_cls}') server_config_cls = get_impl(ServerConfig, config_cls) - server_config = server_config_cls() + server_config : ServerConfig = server_config_cls() server_config.verify_config() return server_config diff --git a/openhands/server/conversation_manager/docker_nested_conversation_manager.py b/openhands/server/conversation_manager/docker_nested_conversation_manager.py index 6349edd7d9..671bebe2d7 100644 --- a/openhands/server/conversation_manager/docker_nested_conversation_manager.py +++ b/openhands/server/conversation_manager/docker_nested_conversation_manager.py @@ -90,7 +90,8 @@ class DockerNestedConversationManager(ConversationManager): """ Get the running agent loops directly from docker. """ - names = (container.name for container in self.docker_client.containers.list()) + containers : list[Container] = self.docker_client.containers.list() + names = (container.name or '' for container in containers) conversation_ids = { name[len('openhands-runtime-') :] for name in names @@ -282,11 +283,11 @@ class DockerNestedConversationManager(ConversationManager): async def close_session(self, sid: str): stop_all_containers(f'openhands-runtime-{sid}') - async def get_agent_loop_info(self, user_id=None, filter_to_sids=None): + async def get_agent_loop_info(self, user_id: str | None = None, filter_to_sids: set[str] | None = None) -> list[AgentLoopInfo]: results = [] - containers = self.docker_client.containers.list() + containers : list[Container] = self.docker_client.containers.list() for container in containers: - if not container.name.startswith('openhands-runtime-'): + if not container.name or not container.name.startswith('openhands-runtime-'): continue conversation_id = container.name[len('openhands-runtime-') :] if filter_to_sids is not None and conversation_id not in filter_to_sids: @@ -349,11 +350,12 @@ class DockerNestedConversationManager(ConversationManager): def get_nested_url_for_container(self, container: Container) -> str: env = container.attrs['Config']['Env'] container_port = int(next(e[5:] for e in env if e.startswith('port='))) - conversation_id = container.name[len('openhands-runtime-') :] + container_name = container.name or '' + conversation_id = container_name[len('openhands-runtime-') :] nested_url = f'{self.config.sandbox.local_runtime_url}:{container_port}/api/conversations/{conversation_id}' return nested_url - def _get_session_api_key_for_conversation(self, conversation_id: str): + def _get_session_api_key_for_conversation(self, conversation_id: str) -> str: jwt_secret = self.config.jwt_secret.get_secret_value() # type:ignore conversation_key = f'{jwt_secret}:{conversation_id}'.encode() session_api_key = ( @@ -363,7 +365,7 @@ class DockerNestedConversationManager(ConversationManager): ) return session_api_key - async def ensure_num_conversations_below_limit(self, sid: str, user_id: str | None): + async def ensure_num_conversations_below_limit(self, sid: str, user_id: str | None) -> None: response_ids = await self.get_running_agent_loops(user_id) if len(response_ids) >= self.config.max_concurrent_conversations: logger.info( @@ -395,7 +397,7 @@ class DockerNestedConversationManager(ConversationManager): ) await self.close_session(oldest_conversation_id) - def _get_provider_handler(self, settings: Settings): + def _get_provider_handler(self, settings: Settings) -> ProviderHandler: provider_tokens = None if isinstance(settings, ConversationInitData): provider_tokens = settings.git_provider_tokens @@ -405,7 +407,7 @@ class DockerNestedConversationManager(ConversationManager): ) return provider_handler - async def _create_runtime(self, sid: str, user_id: str | None, settings: Settings): + async def _create_runtime(self, sid: str, user_id: str | None, settings: Settings) -> DockerRuntime: # This session is created here only because it is the easiest way to get a runtime, which # is the easiest way to create the needed docker container session = Session( @@ -480,7 +482,7 @@ class DockerNestedConversationManager(ConversationManager): if container: status = container.status if status == 'exited': - await call_sync_from_async(container.start()) + await call_sync_from_async(container.start) return True return False except docker.errors.NotFound as e: