Skip to content
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

Revert "Fix closing sessions" #6300

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading