mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
Make SlackTeamStore fully async (#13160)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -181,7 +181,7 @@ class SlackManager(Manager):
|
|||||||
)
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
slack_view = SlackFactory.create_slack_view_from_payload(
|
slack_view = await SlackFactory.create_slack_view_from_payload(
|
||||||
message, slack_user, saas_user_auth
|
message, slack_user, saas_user_auth
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -88,9 +88,9 @@ class SlackV1CallbackProcessor(EventCallbackProcessor):
|
|||||||
# Slack helpers
|
# Slack helpers
|
||||||
# -------------------------------------------------------------------------
|
# -------------------------------------------------------------------------
|
||||||
|
|
||||||
def _get_bot_access_token(self):
|
async def _get_bot_access_token(self) -> str | None:
|
||||||
slack_team_store = SlackTeamStore.get_instance()
|
slack_team_store = SlackTeamStore.get_instance()
|
||||||
bot_access_token = slack_team_store.get_team_bot_token(
|
bot_access_token = await slack_team_store.get_team_bot_token(
|
||||||
self.slack_view_data['team_id']
|
self.slack_view_data['team_id']
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -98,7 +98,7 @@ class SlackV1CallbackProcessor(EventCallbackProcessor):
|
|||||||
|
|
||||||
async def _post_summary_to_slack(self, summary: str) -> None:
|
async def _post_summary_to_slack(self, summary: str) -> None:
|
||||||
"""Post a summary message to the configured Slack channel."""
|
"""Post a summary message to the configured Slack channel."""
|
||||||
bot_access_token = self._get_bot_access_token()
|
bot_access_token = await self._get_bot_access_token()
|
||||||
if not bot_access_token:
|
if not bot_access_token:
|
||||||
raise RuntimeError('Missing Slack bot access token')
|
raise RuntimeError('Missing Slack bot access token')
|
||||||
|
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import asyncio
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from uuid import UUID, uuid4
|
from uuid import UUID, uuid4
|
||||||
|
|
||||||
@@ -42,7 +43,7 @@ from openhands.server.user_auth.user_auth import UserAuth
|
|||||||
from openhands.storage.data_models.conversation_metadata import (
|
from openhands.storage.data_models.conversation_metadata import (
|
||||||
ConversationTrigger,
|
ConversationTrigger,
|
||||||
)
|
)
|
||||||
from openhands.utils.async_utils import GENERAL_TIMEOUT, call_async_from_sync
|
from openhands.utils.async_utils import GENERAL_TIMEOUT
|
||||||
|
|
||||||
# =================================================
|
# =================================================
|
||||||
# SECTION: Slack view types
|
# SECTION: Slack view types
|
||||||
@@ -553,7 +554,8 @@ class SlackFactory:
|
|||||||
channel_id, thread_ts
|
channel_id, thread_ts
|
||||||
)
|
)
|
||||||
|
|
||||||
def create_slack_view_from_payload(
|
@staticmethod
|
||||||
|
async def create_slack_view_from_payload(
|
||||||
message: Message, slack_user: SlackUser | None, saas_user_auth: UserAuth | None
|
message: Message, slack_user: SlackUser | None, saas_user_auth: UserAuth | None
|
||||||
):
|
):
|
||||||
payload = message.message
|
payload = message.message
|
||||||
@@ -564,7 +566,7 @@ class SlackFactory:
|
|||||||
team_id = payload['team_id']
|
team_id = payload['team_id']
|
||||||
user_msg = payload.get('user_msg')
|
user_msg = payload.get('user_msg')
|
||||||
|
|
||||||
bot_access_token = slack_team_store.get_team_bot_token(team_id)
|
bot_access_token = await slack_team_store.get_team_bot_token(team_id)
|
||||||
if not bot_access_token:
|
if not bot_access_token:
|
||||||
logger.error(
|
logger.error(
|
||||||
'Did not find slack team',
|
'Did not find slack team',
|
||||||
@@ -594,10 +596,9 @@ class SlackFactory:
|
|||||||
v1_enabled=False,
|
v1_enabled=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
conversation: SlackConversation | None = call_async_from_sync(
|
conversation = await asyncio.wait_for(
|
||||||
SlackFactory.determine_if_updating_existing_conversation,
|
SlackFactory.determine_if_updating_existing_conversation(message),
|
||||||
GENERAL_TIMEOUT,
|
timeout=GENERAL_TIMEOUT,
|
||||||
message,
|
|
||||||
)
|
)
|
||||||
if conversation:
|
if conversation:
|
||||||
logger.info(
|
logger.info(
|
||||||
|
|||||||
@@ -62,7 +62,7 @@ class SlackCallbackProcessor(ConversationCallbackProcessor):
|
|||||||
slack_user, saas_user_auth = await slack_manager.authenticate_user(
|
slack_user, saas_user_auth = await slack_manager.authenticate_user(
|
||||||
self.slack_user_id
|
self.slack_user_id
|
||||||
)
|
)
|
||||||
slack_view = SlackFactory.create_slack_view_from_payload(
|
slack_view = await SlackFactory.create_slack_view_from_payload(
|
||||||
message_obj, slack_user, saas_user_auth
|
message_obj, slack_user, saas_user_auth
|
||||||
)
|
)
|
||||||
# Send the message directly as a string
|
# Send the message directly as a string
|
||||||
|
|||||||
@@ -219,9 +219,9 @@ async def keycloak_callback(
|
|||||||
|
|
||||||
# Retrieve bot token
|
# Retrieve bot token
|
||||||
if team_id and bot_access_token:
|
if team_id and bot_access_token:
|
||||||
slack_team_store.create_team(team_id, bot_access_token)
|
await slack_team_store.create_team(team_id, bot_access_token)
|
||||||
else:
|
else:
|
||||||
bot_access_token = slack_team_store.get_team_bot_token(team_id)
|
bot_access_token = await slack_team_store.get_team_bot_token(team_id)
|
||||||
|
|
||||||
if not bot_access_token:
|
if not bot_access_token:
|
||||||
logger.error(
|
logger.error(
|
||||||
|
|||||||
@@ -1,23 +1,26 @@
|
|||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
from sqlalchemy.orm import sessionmaker
|
from sqlalchemy import delete, select
|
||||||
from storage.database import session_maker
|
from storage.database import a_session_maker
|
||||||
from storage.slack_team import SlackTeam
|
from storage.slack_team import SlackTeam
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
class SlackTeamStore:
|
class SlackTeamStore:
|
||||||
session_maker: sessionmaker
|
async def get_team_bot_token(self, team_id: str) -> str | None:
|
||||||
|
|
||||||
def get_team_bot_token(self, team_id: str) -> str | None:
|
|
||||||
"""
|
"""
|
||||||
Get a team's bot access token by team_id
|
Get a team's bot access token by team_id
|
||||||
"""
|
"""
|
||||||
with session_maker() as session:
|
async with a_session_maker() as session:
|
||||||
team = session.query(SlackTeam).filter(SlackTeam.team_id == team_id).first()
|
result = await session.execute(
|
||||||
|
select(SlackTeam).where(SlackTeam.team_id == team_id)
|
||||||
|
)
|
||||||
|
team = result.scalar_one_or_none()
|
||||||
return team.bot_access_token if team else None
|
return team.bot_access_token if team else None
|
||||||
|
|
||||||
def create_team(
|
async def create_team(
|
||||||
self,
|
self,
|
||||||
team_id: str,
|
team_id: str,
|
||||||
bot_access_token: str,
|
bot_access_token: str,
|
||||||
@@ -26,14 +29,12 @@ class SlackTeamStore:
|
|||||||
Create a new SlackTeam
|
Create a new SlackTeam
|
||||||
"""
|
"""
|
||||||
slack_team = SlackTeam(team_id=team_id, bot_access_token=bot_access_token)
|
slack_team = SlackTeam(team_id=team_id, bot_access_token=bot_access_token)
|
||||||
with session_maker() as session:
|
async with a_session_maker() as session:
|
||||||
session.query(SlackTeam).filter(SlackTeam.team_id == team_id).delete()
|
await session.execute(delete(SlackTeam).where(SlackTeam.team_id == team_id))
|
||||||
|
|
||||||
# Store the token
|
|
||||||
session.add(slack_team)
|
session.add(slack_team)
|
||||||
session.commit()
|
await session.commit()
|
||||||
return slack_team
|
return slack_team
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def get_instance(cls):
|
def get_instance(cls) -> SlackTeamStore:
|
||||||
return SlackTeamStore(session_maker)
|
return SlackTeamStore()
|
||||||
|
|||||||
@@ -145,9 +145,9 @@ class TestSlackV1CallbackProcessor:
|
|||||||
"""Test that processor handles double callback correctly and processes both times."""
|
"""Test that processor handles double callback correctly and processes both times."""
|
||||||
conversation_id = uuid4()
|
conversation_id = uuid4()
|
||||||
|
|
||||||
# Mock SlackTeamStore
|
# Mock SlackTeamStore (async method)
|
||||||
mock_store = MagicMock()
|
mock_store = MagicMock()
|
||||||
mock_store.get_team_bot_token.return_value = 'xoxb-test-token'
|
mock_store.get_team_bot_token = AsyncMock(return_value='xoxb-test-token')
|
||||||
mock_slack_team_store.return_value = mock_store
|
mock_slack_team_store.return_value = mock_store
|
||||||
|
|
||||||
# Mock successful summary generation
|
# Mock successful summary generation
|
||||||
@@ -208,9 +208,9 @@ class TestSlackV1CallbackProcessor:
|
|||||||
"""Test successful end-to-end callback execution."""
|
"""Test successful end-to-end callback execution."""
|
||||||
conversation_id = uuid4()
|
conversation_id = uuid4()
|
||||||
|
|
||||||
# Mock SlackTeamStore
|
# Mock SlackTeamStore (async method)
|
||||||
mock_store = MagicMock()
|
mock_store = MagicMock()
|
||||||
mock_store.get_team_bot_token.return_value = 'xoxb-test-token'
|
mock_store.get_team_bot_token = AsyncMock(return_value='xoxb-test-token')
|
||||||
mock_slack_team_store.return_value = mock_store
|
mock_slack_team_store.return_value = mock_store
|
||||||
|
|
||||||
# Mock summary instruction
|
# Mock summary instruction
|
||||||
@@ -287,9 +287,9 @@ class TestSlackV1CallbackProcessor:
|
|||||||
expected_error,
|
expected_error,
|
||||||
):
|
):
|
||||||
"""Test error handling when bot access token is missing or empty."""
|
"""Test error handling when bot access token is missing or empty."""
|
||||||
# Mock SlackTeamStore to return the test token
|
# Mock SlackTeamStore to return the test token (async method)
|
||||||
mock_store = MagicMock()
|
mock_store = MagicMock()
|
||||||
mock_store.get_team_bot_token.return_value = bot_token
|
mock_store.get_team_bot_token = AsyncMock(return_value=bot_token)
|
||||||
mock_slack_team_store.return_value = mock_store
|
mock_slack_team_store.return_value = mock_store
|
||||||
|
|
||||||
# Mock successful summary generation
|
# Mock successful summary generation
|
||||||
@@ -327,9 +327,9 @@ class TestSlackV1CallbackProcessor:
|
|||||||
expected_error,
|
expected_error,
|
||||||
):
|
):
|
||||||
"""Test error handling for various Slack API errors."""
|
"""Test error handling for various Slack API errors."""
|
||||||
# Mock SlackTeamStore
|
# Mock SlackTeamStore (async method)
|
||||||
mock_store = MagicMock()
|
mock_store = MagicMock()
|
||||||
mock_store.get_team_bot_token.return_value = 'xoxb-test-token'
|
mock_store.get_team_bot_token = AsyncMock(return_value='xoxb-test-token')
|
||||||
mock_slack_team_store.return_value = mock_store
|
mock_slack_team_store.return_value = mock_store
|
||||||
|
|
||||||
# Mock successful summary generation
|
# Mock successful summary generation
|
||||||
@@ -392,9 +392,9 @@ class TestSlackV1CallbackProcessor:
|
|||||||
"""Test error handling for various agent server errors."""
|
"""Test error handling for various agent server errors."""
|
||||||
conversation_id = uuid4()
|
conversation_id = uuid4()
|
||||||
|
|
||||||
# Mock SlackTeamStore
|
# Mock SlackTeamStore (async method)
|
||||||
mock_store = MagicMock()
|
mock_store = MagicMock()
|
||||||
mock_store.get_team_bot_token.return_value = 'xoxb-test-token'
|
mock_store.get_team_bot_token = AsyncMock(return_value='xoxb-test-token')
|
||||||
mock_slack_team_store.return_value = mock_store
|
mock_slack_team_store.return_value = mock_store
|
||||||
|
|
||||||
# Mock summary instruction
|
# Mock summary instruction
|
||||||
|
|||||||
@@ -240,12 +240,12 @@ class TestSlackCallbackProcessor:
|
|||||||
return_value=(mock_slack_user, mock_saas_user_auth)
|
return_value=(mock_slack_user, mock_saas_user_auth)
|
||||||
)
|
)
|
||||||
|
|
||||||
# Mock the SlackFactory
|
# Mock the SlackFactory (async method)
|
||||||
with patch(
|
with patch(
|
||||||
'server.conversation_callback_processor.slack_callback_processor.SlackFactory'
|
'server.conversation_callback_processor.slack_callback_processor.SlackFactory'
|
||||||
) as mock_slack_factory:
|
) as mock_slack_factory:
|
||||||
mock_slack_factory.create_slack_view_from_payload.return_value = (
|
mock_slack_factory.create_slack_view_from_payload = AsyncMock(
|
||||||
mock_slack_view
|
return_value=mock_slack_view
|
||||||
)
|
)
|
||||||
mock_slack_manager.send_message = AsyncMock()
|
mock_slack_manager.send_message = AsyncMock()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user