Skip to content
Merged
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
33 changes: 28 additions & 5 deletions src/app/endpoints/conversations.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ async def get_conversation_endpoint_handler(
client = AsyncLlamaStackClientHolder().get_client()

agent_sessions = (await client.agents.session.list(agent_id=agent_id)).data
if not agent_sessions:
logger.error("No sessions found for conversation %s", conversation_id)
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail={
"response": "Conversation not found",
"cause": f"Conversation {conversation_id} could not be retrieved.",
},
)
session_id = str(agent_sessions[0].get("session_id"))

session_response = await client.agents.session.retrieve(
Expand Down Expand Up @@ -283,6 +292,8 @@ async def get_conversation_endpoint_handler(
"cause": f"Conversation {conversation_id} could not be retrieved: {str(e)}",
},
) from e
except HTTPException:
raise
except Exception as e:
# Handle case where session doesn't exist or other errors
logger.exception("Error retrieving conversation %s: %s", conversation_id, e)
Expand Down Expand Up @@ -339,11 +350,21 @@ async def delete_conversation_endpoint_handler(
try:
# Get Llama Stack client
client = AsyncLlamaStackClientHolder().get_client()
# Delete session using the conversation_id as session_id
# In this implementation, conversation_id and session_id are the same
await client.agents.session.delete(
agent_id=agent_id, session_id=conversation_id
)

agent_sessions = (await client.agents.session.list(agent_id=agent_id)).data

if not agent_sessions:
# If no sessions are found, do not raise an error, just return a success response
logger.info("No sessions found for conversation %s", conversation_id)
return ConversationDeleteResponse(
conversation_id=conversation_id,
success=True,
response="Conversation deleted successfully",
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be returning 404 on DELETE, for REST APIs I think the approach needs to be idempotent.

If the user asks "delete this" and the end state (resource gone) is already true, the operation is logically successful.


session_id = str(agent_sessions[0].get("session_id"))

await client.agents.session.delete(agent_id=agent_id, session_id=session_id)

logger.info("Successfully deleted conversation %s", conversation_id)

Expand Down Expand Up @@ -371,6 +392,8 @@ async def delete_conversation_endpoint_handler(
"cause": f"Conversation {conversation_id} could not be deleted: {str(e)}",
},
) from e
except HTTPException:
raise
except Exception as e:
# Handle case where session doesn't exist or other errors
logger.exception("Error deleting conversation %s: %s", conversation_id, e)
Expand Down
12 changes: 11 additions & 1 deletion src/utils/endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,17 @@ async def get_agent(
await client.agents.delete(agent_id=orphan_agent_id)
sessions_response = await client.agents.session.list(agent_id=conversation_id)
logger.info("session response: %s", sessions_response)
session_id = str(sessions_response.data[0]["session_id"])
try:
session_id = str(sessions_response.data[0]["session_id"])
except IndexError as e:
logger.error("No sessions found for conversation %s", conversation_id)
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail={
"response": "Conversation not found",
"cause": f"Conversation {conversation_id} could not be retrieved.",
},
) from e
else:
conversation_id = agent.agent_id
session_id = await agent.create_session(get_suid())
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/app/endpoints/test_conversations.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,10 @@ async def test_successful_conversation_deletion(self, mocker, setup_configuratio

# Mock AsyncLlamaStackClientHolder
mock_client = mocker.AsyncMock()
# Ensure the endpoint sees an existing session so it proceeds to delete
mock_client.agents.session.list.return_value = mocker.Mock(
data=[{"session_id": VALID_CONVERSATION_ID}]
)
mock_client.agents.session.delete.return_value = None # Successful deletion
mock_client_holder = mocker.patch(
"app.endpoints.conversations.AsyncLlamaStackClientHolder"
Expand Down