Skip to content

Commit

Permalink
Ensure accessibility of job, scope get to user. (e.g. -- admins could…
Browse files Browse the repository at this point in the history
… query for shared/errored datasets, etc)
  • Loading branch information
dannon committed Nov 19, 2024
1 parent c98216d commit 8aadf1a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
11 changes: 8 additions & 3 deletions lib/galaxy/managers/chat.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from typing import Optional

from fastapi import Path
from sqlalchemy import select
from sqlalchemy import (
and_,
select,
)
from sqlalchemy.exc import (
MultipleResultsFound,
NoResultFound,
Expand Down Expand Up @@ -66,10 +69,12 @@ def get(self, trans: ProvidesUserContext, job_id: JobIdPathParam) -> ChatExchang
:raises: InconsistentDatabase, InternalServerError
"""
try:
stmt = select(ChatExchange).where(ChatExchange.job_id == job_id)
stmt = select(ChatExchange).where(
and_(ChatExchange.job_id == job_id, ChatExchange.user_id == trans.user.id)
)
chat_response = trans.sa_session.execute(stmt).scalar_one()
except MultipleResultsFound:
# TODO: Unsure about this, isn't this more applicable when we're getting the response for response.id instead of response.job_id?
# TODO: Unsure about this, isn't this more applicable when we're getting the response for response.id instead of response.
raise InconsistentDatabase("Multiple chat responses found with the same job id.")
except NoResultFound:
# TODO: Would there be cases where we raise an exception here? Or, is there a better way to return None?
Expand Down
13 changes: 8 additions & 5 deletions lib/galaxy/webapps/galaxy/api/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
JobIdPathParam,
)
from galaxy.managers.context import ProvidesUserContext
from galaxy.managers.jobs import JobManager
from galaxy.schema.schema import ChatPayload
from galaxy.webapps.galaxy.api import (
depends,
Expand All @@ -36,6 +37,7 @@
class ChatAPI:
config: GalaxyAppConfiguration = depends(GalaxyAppConfiguration)
chat_manager: ChatManager = depends(ChatManager)
job_manager: JobManager = depends(JobManager)

@router.post("/api/chat")
def query(
Expand All @@ -47,8 +49,8 @@ def query(
"""We're off to ask the wizard"""
# Currently job-based chat exchanges are the only ones supported,
# and will only have the one message.

if job_id:
job = self.job_manager.get_accessible_job(trans, job_id)
if job:
# If there's an existing response for this job, just return that one for now.
# TODO: Support regenerating the response as a new message, and
# asking follow-up questions.
Expand All @@ -62,8 +64,8 @@ def query(
response = self._call_openai(messages)
answer = response.choices[0].message.content

if job_id:
self.chat_manager.create(trans, job_id, answer)
if job:
self.chat_manager.create(trans, job.id, answer)

return answer

Expand All @@ -75,7 +77,8 @@ def feedback(
trans: ProvidesUserContext = DependsOnTrans,
) -> int | None:
"""Provide feedback on the chatbot response."""
chat_response = self.chat_manager.set_feedback_for_job(trans, job_id, feedback)
job = self.job_manager.get_accessible_job(trans, job_id)
chat_response = self.chat_manager.set_feedback_for_job(trans, job.id, feedback)
return chat_response.messages[0].feedback

def _ensure_openai_configured(self):
Expand Down

0 comments on commit 8aadf1a

Please sign in to comment.