From df050e47860fe5e0afb49b0448f4af3621781961 Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Thu, 16 Jan 2025 11:40:03 -0500 Subject: [PATCH] Separate data extraction and convo creation logic (#6298) --- openhands/server/listen_socket.py | 3 +- .../server/routes/manage_conversations.py | 84 ++++++++++++------- openhands/server/types.py | 12 +++ 3 files changed, 70 insertions(+), 29 deletions(-) diff --git a/openhands/server/listen_socket.py b/openhands/server/listen_socket.py index 81816a1325e1..baa79a4c6e35 100644 --- a/openhands/server/listen_socket.py +++ b/openhands/server/listen_socket.py @@ -44,7 +44,8 @@ async def connect(connection_id: str, environ, auth): conversation_store = await ConversationStoreImpl.get_instance(config, user_id) metadata = await conversation_store.get_metadata(conversation_id) - if metadata.github_user_id != user_id: + + if metadata.github_user_id != str(user_id): logger.error( f'User {user_id} is not allowed to join conversation {conversation_id}' ) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 5cfc0ba82d52..0015cf0e90aa 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -12,6 +12,7 @@ from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager +from openhands.server.types import LLMAuthenticationError, MissingSettingsError from openhands.storage.data_models.conversation_info import ConversationInfo from openhands.storage.data_models.conversation_info_result_set import ( ConversationInfoResultSet, @@ -33,16 +34,12 @@ class InitSessionRequest(BaseModel): selected_repository: str | None = None -@app.post('/conversations') -async def new_conversation(request: Request, data: InitSessionRequest): - """Initialize a new session or join an existing one. - After successful initialization, the client should connect to the WebSocket - using the returned conversation ID - """ - logger.info('Initializing new conversation') - +async def _create_new_conversation( + user_id: str | None, + token: str | None, + selected_repository: str | None, +): logger.info('Loading settings') - user_id = get_user_id(request) settings_store = await SettingsStoreImpl.get_instance(config, user_id) settings = await settings_store.load() logger.info('Settings loaded') @@ -54,25 +51,16 @@ async def new_conversation(request: Request, data: InitSessionRequest): # but that would run a tiny inference. if not settings.llm_api_key or settings.llm_api_key.isspace(): logger.warn(f'Missing api key for model {settings.llm_model}') - return JSONResponse( - content={ - 'status': 'error', - 'message': 'Error authenticating with the LLM provider. Please check your API key', - 'msg_id': 'STATUS$ERROR_LLM_AUTHENTICATION', - } + raise LLMAuthenticationError( + 'Error authenticating with the LLM provider. Please check your API key' ) + else: logger.warn('Settings not present, not starting conversation') - return JSONResponse( - content={ - 'status': 'error', - 'message': 'Settings not found', - 'msg_id': 'CONFIGURATION$SETTINGS_NOT_FOUND', - } - ) - github_token = getattr(request.state, 'github_token', '') - session_init_args['github_token'] = github_token or data.github_token or '' - session_init_args['selected_repository'] = data.selected_repository + raise MissingSettingsError('Settings not found') + + session_init_args['github_token'] = token or '' + session_init_args['selected_repository'] = selected_repository conversation_init_data = ConversationInitData(**session_init_args) logger.info('Loading conversation store') conversation_store = await ConversationStoreImpl.get_instance(config, user_id) @@ -85,7 +73,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): logger.info(f'New conversation ID: {conversation_id}') repository_title = ( - data.selected_repository.split('/')[-1] if data.selected_repository else None + selected_repository.split('/')[-1] if selected_repository else None ) conversation_title = f'{repository_title or "Conversation"} {conversation_id[:5]}' @@ -95,7 +83,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): conversation_id=conversation_id, title=conversation_title, github_user_id=user_id, - selected_repository=data.selected_repository, + selected_repository=selected_repository, ) ) @@ -112,7 +100,47 @@ async def new_conversation(request: Request, data: InitSessionRequest): except ValueError: pass # Already subscribed - take no action logger.info(f'Finished initializing conversation {conversation_id}') - return JSONResponse(content={'status': 'ok', 'conversation_id': conversation_id}) + + return conversation_id + + +@app.post('/conversations') +async def new_conversation(request: Request, data: InitSessionRequest): + """Initialize a new session or join an existing one. + After successful initialization, the client should connect to the WebSocket + using the returned conversation ID + """ + logger.info('Initializing new conversation') + user_id = get_user_id(request) + github_token = getattr(request.state, 'github_token', '') or data.github_token + selected_repository = data.selected_repository + + try: + conversation_id = await _create_new_conversation( + user_id, github_token, selected_repository + ) + + return JSONResponse( + content={'status': 'ok', 'conversation_id': conversation_id} + ) + except MissingSettingsError as e: + return JSONResponse( + content={ + 'status': 'error', + 'message': str(e), + 'msg_id': 'CONFIGURATION$SETTINGS_NOT_FOUND', + }, + status_code=400, + ) + + except LLMAuthenticationError as e: + return JSONResponse( + content={ + 'status': 'error', + 'message': str(e), + 'msg_id': 'STATUS$ERROR_LLM_AUTHENTICATION', + }, + ) @app.get('/conversations') diff --git a/openhands/server/types.py b/openhands/server/types.py index 8ecb898a76cf..cbf9389d2b44 100644 --- a/openhands/server/types.py +++ b/openhands/server/types.py @@ -35,3 +35,15 @@ async def verify_github_repo_list(self, installation_id: int | None) -> None: async def get_config(self) -> dict[str, str]: """Configure attributes for frontend""" raise NotImplementedError + + +class MissingSettingsError(ValueError): + """Raised when settings are missing or not found.""" + + pass + + +class LLMAuthenticationError(ValueError): + """Raised when there is an issue with LLM authentication.""" + + pass