Skip to content

Commit

Permalink
Revert "Fix closing sessions (#6114)"
Browse files Browse the repository at this point in the history
This reverts commit 8795ee6.
  • Loading branch information
xingyaoww committed Jan 15, 2025
1 parent 25dc793 commit 0852fed
Show file tree
Hide file tree
Showing 10 changed files with 221 additions and 331 deletions.
4 changes: 2 additions & 2 deletions openhands/core/config/sandbox_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class SandboxConfig:

remote_runtime_api_url: str = 'http://localhost:8000'
local_runtime_url: str = 'http://localhost'
keep_runtime_alive: bool = False
keep_runtime_alive: bool = True
rm_all_containers: bool = False
api_key: str | None = None
base_container_image: str = 'nikolaik/python-nodejs:python3.12-nodejs22' # default to nikolaik/python-nodejs:python3.12-nodejs22 for eventstream runtime
Expand All @@ -60,7 +60,7 @@ class SandboxConfig:
runtime_startup_env_vars: dict[str, str] = field(default_factory=dict)
browsergym_eval_env: str | None = None
platform: str | None = None
close_delay: int = 15
close_delay: int = 900
remote_runtime_resource_factor: int = 1
enable_gpu: bool = False
docker_runtime_kwargs: str | None = None
Expand Down
7 changes: 4 additions & 3 deletions openhands/runtime/builder/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from openhands.core.logger import openhands_logger as logger
from openhands.runtime.builder import RuntimeBuilder
from openhands.runtime.utils.request import send_request
from openhands.utils.http_session import HttpSession
from openhands.utils.shutdown_listener import (
should_continue,
sleep_if_should_continue,
Expand All @@ -19,10 +18,12 @@
class RemoteRuntimeBuilder(RuntimeBuilder):
"""This class interacts with the remote Runtime API for building and managing container images."""

def __init__(self, api_url: str, api_key: str, session: HttpSession | None = None):
def __init__(
self, api_url: str, api_key: str, session: requests.Session | None = None
):
self.api_url = api_url
self.api_key = api_key
self.session = session or HttpSession()
self.session = session or requests.Session()
self.session.headers.update({'X-API-Key': self.api_key})

def build(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from openhands.runtime.base import Runtime
from openhands.runtime.plugins import PluginRequirement
from openhands.runtime.utils.request import send_request
from openhands.utils.http_session import HttpSession


class ActionExecutionClient(Runtime):
Expand All @@ -56,7 +55,7 @@ def __init__(
attach_to_existing: bool = False,
headless_mode: bool = True,
):
self.session = HttpSession()
self.session = requests.Session()
self.action_semaphore = threading.Semaphore(1) # Ensure one action at a time
self._runtime_initialized: bool = False
self._vscode_token: str | None = None # initial dummy value
Expand Down
7 changes: 3 additions & 4 deletions openhands/runtime/utils/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import requests
from tenacity import retry, retry_if_exception, stop_after_attempt, wait_exponential

from openhands.utils.http_session import HttpSession
from openhands.utils.tenacity_stop import stop_if_should_exit


Expand Down Expand Up @@ -35,7 +34,7 @@ def is_retryable_error(exception):
wait=wait_exponential(multiplier=1, min=4, max=60),
)
def send_request(
session: HttpSession,
session: requests.Session,
method: str,
url: str,
timeout: int = 10,
Expand All @@ -49,11 +48,11 @@ def send_request(
_json = response.json()
except (requests.exceptions.JSONDecodeError, json.decoder.JSONDecodeError):
_json = None
finally:
response.close()
raise RequestHTTPError(
e,
response=e.response,
detail=_json.get('detail') if _json is not None else None,
) from e
finally:
response.close()
return response
2 changes: 1 addition & 1 deletion openhands/server/routes/manage_conversations.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ async def search_conversations(
for conversation in conversation_metadata_result_set.results
if hasattr(conversation, 'created_at')
)
running_conversations = await session_manager.get_running_agent_loops(
running_conversations = await session_manager.get_agent_loop_running(
get_user_id(request), set(conversation_ids)
)
result = ConversationInfoResultSet(
Expand Down
33 changes: 13 additions & 20 deletions openhands/server/session/agent_session.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import asyncio
import time
from typing import Callable, Optional

from openhands.controller import AgentController
Expand All @@ -17,10 +16,10 @@
from openhands.runtime.base import Runtime
from openhands.security import SecurityAnalyzer, options
from openhands.storage.files import FileStore
from openhands.utils.async_utils import call_sync_from_async
from openhands.utils.async_utils import call_async_from_sync, call_sync_from_async
from openhands.utils.shutdown_listener import should_continue

WAIT_TIME_BEFORE_CLOSE = 90
WAIT_TIME_BEFORE_CLOSE = 300
WAIT_TIME_BEFORE_CLOSE_INTERVAL = 5


Expand All @@ -37,8 +36,7 @@ class AgentSession:
controller: AgentController | None = None
runtime: Runtime | None = None
security_analyzer: SecurityAnalyzer | None = None
_starting: bool = False
_started_at: float = 0
_initializing: bool = False
_closed: bool = False
loop: asyncio.AbstractEventLoop | None = None

Expand Down Expand Up @@ -90,8 +88,7 @@ async def start(
if self._closed:
logger.warning('Session closed before starting')
return
self._starting = True
self._started_at = time.time()
self._initializing = True
self._create_security_analyzer(config.security.security_analyzer)
await self._create_runtime(
runtime_name=runtime_name,
Expand All @@ -112,19 +109,24 @@ async def start(
self.event_stream.add_event(
ChangeAgentStateAction(AgentState.INIT), EventSource.ENVIRONMENT
)
self._starting = False
self._initializing = False

async def close(self):
def close(self):
"""Closes the Agent session"""
if self._closed:
return
self._closed = True
while self._starting and should_continue():
call_async_from_sync(self._close)

async def _close(self):
seconds_waited = 0
while self._initializing and should_continue():
logger.debug(
f'Waiting for initialization to finish before closing session {self.sid}'
)
await asyncio.sleep(WAIT_TIME_BEFORE_CLOSE_INTERVAL)
if time.time() <= self._started_at + WAIT_TIME_BEFORE_CLOSE:
seconds_waited += WAIT_TIME_BEFORE_CLOSE_INTERVAL
if seconds_waited > WAIT_TIME_BEFORE_CLOSE:
logger.error(
f'Waited too long for initialization to finish before closing session {self.sid}'
)
Expand Down Expand Up @@ -309,12 +311,3 @@ def _maybe_restore_state(self) -> State | None:
else:
logger.debug('No events found, no state to restore')
return restored_state

def get_state(self) -> AgentState | None:
controller = self.controller
if controller:
return controller.state.agent_state
if time.time() > self._started_at + WAIT_TIME_BEFORE_CLOSE:
# If 5 minutes have elapsed and we still don't have a controller, something has gone wrong
return AgentState.ERROR
return None
Loading

0 comments on commit 0852fed

Please sign in to comment.