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

[Enhancement]: Handle GH token refresh inside runtime #6632

Merged
merged 27 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
59f5a4c
fix circular import issues
malhotra5 Feb 6, 2025
d36aa29
get token from gh service
malhotra5 Feb 6, 2025
4191310
plumb user_id and get latest token when needed
malhotra5 Feb 6, 2025
aef8ba9
retry build
malhotra5 Feb 6, 2025
4560f86
fix build err
malhotra5 Feb 6, 2025
006a692
relocate filestore init
malhotra5 Feb 6, 2025
32ffc00
pass user_id to remote runtime init
malhotra5 Feb 6, 2025
639eb03
debug logs + fix async err
malhotra5 Feb 6, 2025
a2e4f5b
remove debug logs
malhotra5 Feb 6, 2025
021e64c
make gh service standalone
malhotra5 Feb 7, 2025
289870d
remove server_config dependency from gh service
malhotra5 Feb 7, 2025
8fefdff
move gh types from server to services
malhotra5 Feb 7, 2025
c4b0955
rm gh service cls route from server
malhotra5 Feb 7, 2025
944dc17
use impl from services
malhotra5 Feb 7, 2025
9345dc6
rm refresh param + fetch token from ghservice
malhotra5 Feb 7, 2025
2762997
revert container changes
malhotra5 Feb 7, 2025
f9e836c
use kwargs
malhotra5 Feb 7, 2025
bb8dfbd
rm config init
malhotra5 Feb 7, 2025
82bb4e2
revert shared.py changes
malhotra5 Feb 7, 2025
dda8ecd
move service types to service folder
malhotra5 Feb 7, 2025
79bc289
cmp user_id first
malhotra5 Feb 8, 2025
96de016
Merge branch 'main' into runtime-refresh
malhotra5 Feb 10, 2025
0aeacdb
merge main
malhotra5 Feb 10, 2025
66ae198
export secret val
malhotra5 Feb 10, 2025
3ee8356
rename folder
malhotra5 Feb 10, 2025
5923fe7
rename gh user id
malhotra5 Feb 10, 2025
63c63a3
fix renamed folder
malhotra5 Feb 10, 2025
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
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import os
from typing import Any

import httpx
from pydantic import SecretStr

from openhands.services.github.github_types import (
from openhands.integrations.github.github_types import (
GhAuthenticationError,
GHUnknownException,
GitHubRepository,
GitHubUser,
)
from openhands.utils.import_utils import get_impl


class GitHubService:
Expand Down Expand Up @@ -133,3 +135,10 @@ async def search_repositories(
]

return repos


github_service_cls = os.environ.get(
'OPENHANDS_GITHUB_SERVICE_CLS',
'openhands.integrations.github.github_service.GitHubService',
)
GithubServiceImpl = get_impl(GitHubService, github_service_cls)
14 changes: 14 additions & 0 deletions openhands/runtime/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
UserRejectObservation,
)
from openhands.events.serialization.action import ACTION_TYPE_TO_CLASS
from openhands.integrations.github.github_service import GithubServiceImpl
from openhands.microagent import (
BaseMicroAgent,
load_microagents_from_dir,
Expand Down Expand Up @@ -94,6 +95,7 @@ def __init__(
status_callback: Callable | None = None,
attach_to_existing: bool = False,
headless_mode: bool = False,
github_user_id: str | None = None,
):
self.sid = sid
self.event_stream = event_stream
Expand Down Expand Up @@ -126,6 +128,8 @@ def __init__(
self, enable_llm_editor=config.get_agent_config().codeact_enable_llm_editor
)

self.github_user_id = github_user_id

def setup_initial_env(self) -> None:
if self.attach_to_existing:
return
Expand Down Expand Up @@ -213,6 +217,16 @@ async def _handle_action(self, event: Action) -> None:
event.set_hard_timeout(self.config.sandbox.timeout, blocking=False)
assert event.timeout is not None
try:
if isinstance(event, CmdRunAction):
malhotra5 marked this conversation as resolved.
Show resolved Hide resolved
if self.github_user_id and '$GITHUB_TOKEN' in event.command:
gh_client = GithubServiceImpl(user_id=self.github_user_id)
token = await gh_client.get_latest_token()
if token:
export_cmd = CmdRunAction(
f"export GITHUB_TOKEN='{token.get_secret_value()}'"
)
await call_sync_from_async(self.run, export_cmd)

observation: Observation = await call_sync_from_async(
self.run_action, event
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def __init__(
status_callback: Any | None = None,
attach_to_existing: bool = False,
headless_mode: bool = True,
github_user_id: str | None = None,
):
self.session = HttpSession()
self.action_semaphore = threading.Semaphore(1) # Ensure one action at a time
Expand All @@ -70,6 +71,7 @@ def __init__(
status_callback,
attach_to_existing,
headless_mode,
github_user_id,
)

@abstractmethod
Expand Down
15 changes: 11 additions & 4 deletions openhands/runtime/impl/remote/remote_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def __init__(
status_callback: Optional[Callable] = None,
attach_to_existing: bool = False,
headless_mode: bool = True,
github_user_id: str | None = None,
):
super().__init__(
config,
Expand All @@ -55,6 +56,7 @@ def __init__(
status_callback,
attach_to_existing,
headless_mode,
github_user_id,
)
if self.config.sandbox.api_key is None:
raise ValueError(
Expand Down Expand Up @@ -291,7 +293,8 @@ def _wait_until_alive(self):
stop=tenacity.stop_after_delay(
self.config.sandbox.remote_runtime_init_timeout
)
| stop_if_should_exit() | self._stop_if_closed,
| stop_if_should_exit()
| self._stop_if_closed,
reraise=True,
retry=tenacity.retry_if_exception_type(AgentRuntimeNotReadyError),
wait=tenacity.wait_fixed(2),
Expand Down Expand Up @@ -394,10 +397,14 @@ def _send_action_server_request(self, method, url, **kwargs):

retry_decorator = tenacity.retry(
retry=tenacity.retry_if_exception_type(ConnectionError),
stop=tenacity.stop_after_attempt(3) | stop_if_should_exit() | self._stop_if_closed,
stop=tenacity.stop_after_attempt(3)
| stop_if_should_exit()
| self._stop_if_closed,
wait=tenacity.wait_exponential(multiplier=1, min=4, max=60),
)
return retry_decorator(self._send_action_server_request_impl)(method, url, **kwargs)
return retry_decorator(self._send_action_server_request_impl)(
method, url, **kwargs
)

def _send_action_server_request_impl(self, method, url, **kwargs):
try:
Expand Down Expand Up @@ -430,6 +437,6 @@ def _send_action_server_request_impl(self, method, url, **kwargs):
) from e
else:
raise e

def _stop_if_closed(self, retry_state: tenacity.RetryCallState) -> bool:
return self._runtime_closed
2 changes: 0 additions & 2 deletions openhands/server/config/server_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ class ServerConfig(ServerConfigInterface):
)
conversation_manager_class: str = 'openhands.server.conversation_manager.standalone_conversation_manager.StandaloneConversationManager'

github_service_class: str = 'openhands.services.github.github_service.GitHubService'

def verify_config(self):
if self.config_cls:
raise ValueError('Unexpected config path provided')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ async def _cleanup_stale(self):
self._close_session(sid) for sid in self._local_agent_loops_by_sid
)
return
except Exception as e:
logger.error(f'error_cleaning_stale')
except Exception:
logger.error('error_cleaning_stale')
await asyncio.sleep(_CLEANUP_INTERVAL)

async def get_running_agent_loops(
Expand Down
13 changes: 5 additions & 8 deletions openhands/server/routes/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,17 @@
from fastapi.responses import JSONResponse
from pydantic import SecretStr

from openhands.server.auth import get_github_token, get_user_id
from openhands.server.shared import server_config
from openhands.services.github.github_service import (
from openhands.integrations.github.github_service import GithubServiceImpl
from openhands.integrations.github.github_types import (
GhAuthenticationError,
GHUnknownException,
GitHubService,
GitHubRepository,
GitHubUser,
)
from openhands.services.github.github_types import GitHubRepository, GitHubUser
from openhands.utils.import_utils import get_impl
from openhands.server.auth import get_github_token, get_user_id

app = APIRouter(prefix='/api/github')

GithubServiceImpl = get_impl(GitHubService, server_config.github_service_class)


@app.get('/repositories')
async def get_github_repositories(
Expand Down
6 changes: 3 additions & 3 deletions openhands/server/routes/manage_conversations.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
from openhands.core.logger import openhands_logger as logger
from openhands.events.action.message import MessageAction
from openhands.events.stream import EventStreamSubscriber
from openhands.integrations.github.github_service import GithubServiceImpl
from openhands.runtime import get_runtime_cls
from openhands.server.auth import get_github_token, get_user_id
from openhands.server.routes.github import GithubServiceImpl
from openhands.server.session.conversation_init_data import ConversationInitData
from openhands.server.shared import (
ConversationStoreImpl,
Expand Down Expand Up @@ -131,8 +131,8 @@ async def new_conversation(request: Request, data: InitSessionRequest):
"""
logger.info('Initializing new conversation')
user_id = get_user_id(request)
github_service = GithubServiceImpl(user_id=user_id, token=get_github_token(request))
github_token = await github_service.get_latest_token()
gh_client = GithubServiceImpl(user_id=user_id, token=get_github_token(request))
github_token = await gh_client.get_latest_token()

selected_repository = data.selected_repository
initial_user_msg = data.initial_user_msg
Expand Down
6 changes: 4 additions & 2 deletions openhands/server/routes/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
from pydantic import SecretStr

from openhands.core.logger import openhands_logger as logger
from openhands.integrations.github.github_service import GithubServiceImpl
from openhands.server.auth import get_github_token, get_user_id
from openhands.server.settings import GETSettingsModel, POSTSettingsModel, Settings
from openhands.server.shared import SettingsStoreImpl, config
from openhands.services.github.github_service import GitHubService

app = APIRouter(prefix='/api')

Expand Down Expand Up @@ -51,7 +51,9 @@ async def store_settings(
try:
# We check if the token is valid by getting the user
# If the token is invalid, this will raise an exception
github = GitHubService(user_id=None, token=SecretStr(settings.github_token))
github = GithubServiceImpl(
user_id=None, token=SecretStr(settings.github_token)
)
await github.get_user()

except Exception as e:
Expand Down
9 changes: 9 additions & 0 deletions openhands/server/session/agent_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from openhands.microagent import BaseMicroAgent
from openhands.runtime import get_runtime_cls
from openhands.runtime.base import Runtime
from openhands.runtime.impl.remote.remote_runtime import RemoteRuntime
from openhands.security import SecurityAnalyzer, options
from openhands.storage.files import FileStore
from openhands.utils.async_utils import call_sync_from_async
Expand Down Expand Up @@ -49,6 +50,7 @@ def __init__(
sid: str,
file_store: FileStore,
status_callback: Optional[Callable] = None,
github_user_id: str | None = None,
):
"""Initializes a new instance of the Session class
Expand All @@ -61,6 +63,7 @@ def __init__(
self.event_stream = EventStream(sid, file_store)
self.file_store = file_store
self._status_callback = status_callback
self.github_user_id = github_user_id

async def start(
self,
Expand Down Expand Up @@ -202,6 +205,11 @@ async def _create_runtime(
if github_token
else None
)

kwargs = {}
if runtime_cls == RemoteRuntime:
kwargs['github_user_id'] = self.github_user_id

self.runtime = runtime_cls(
config=config,
event_stream=self.event_stream,
Expand All @@ -210,6 +218,7 @@ async def _create_runtime(
status_callback=self._status_callback,
headless_mode=False,
env_vars=env_vars,
**kwargs,
)

# FIXME: this sleep is a terrible hack.
Expand Down
5 changes: 4 additions & 1 deletion openhands/server/session/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def __init__(
self.last_active_ts = int(time.time())
self.file_store = file_store
self.agent_session = AgentSession(
sid, file_store, status_callback=self.queue_status_message
sid,
file_store,
status_callback=self.queue_status_message,
github_user_id=user_id,
)
self.agent_session.event_stream.subscribe(
EventStreamSubscriber.SERVER, self.on_event, self.sid
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_github_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import pytest
from pydantic import SecretStr

from openhands.services.github.github_service import GitHubService
from openhands.services.github.github_types import GhAuthenticationError
from openhands.integrations.github.github_service import GitHubService
from openhands.integrations.github.github_types import GhAuthenticationError


@pytest.mark.asyncio
Expand Down