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

Refactor the route POST: /api/tools to FastAPI #17295

Draft
wants to merge 52 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
bd9e30f
Unmark create method as internal
heisner-tillman Jan 19, 2024
4542333
Refactor create operation
heisner-tillman Jan 19, 2024
28d1d6d
Create pydantic models for create operation
heisner-tillman Jan 19, 2024
181f63b
Add json keywoard when posting to
heisner-tillman Jan 20, 2024
a606b66
Rename and rework pydnatic model for create operation return
heisner-tillman Jan 20, 2024
8177005
Regenerate the client schema
heisner-tillman Jan 20, 2024
632eac9
Specify jobs field
heisner-tillman Jan 21, 2024
bfeb888
Add pydantic model to return of _handle_inputs_output_to_api_response…
heisner-tillman Jan 21, 2024
fca3497
Add pydantic model to return of FastAPI endpoints
heisner-tillman Jan 21, 2024
976a917
Split create operation to allow for file upload
heisner-tillman Jan 22, 2024
d6d363e
Regenerate client schema
heisner-tillman Jan 22, 2024
39481cc
Extract tempfile instance
heisner-tillman Jan 22, 2024
a036653
Rename create operation and create service methods to reduce code red…
heisner-tillman Jan 24, 2024
0bc847c
Regenerate client schema
heisner-tillman Jan 24, 2024
a9bbf2f
Remove comment
heisner-tillman Jan 25, 2024
001f533
Add json keyword when posting to /api/tools during testing
heisner-tillman Jan 25, 2024
434b045
Fix naming issue of files, when executing a tool
heisner-tillman Jan 26, 2024
098aa80
Remove legacy mapping of execute operations and add route for index o…
heisner-tillman Jan 26, 2024
9f2da1b
Exclude unset fields of execute payload, before passing it to create …
heisner-tillman Feb 4, 2024
702d955
mark create method as internal again as it is not used outside of the…
heisner-tillman Feb 4, 2024
8838d2f
Mark execute operation as async
heisner-tillman Feb 7, 2024
498f3f5
Add missing field to payload of execute operation for tools API
heisner-tillman Feb 8, 2024
8806f70
Move depend_on_either_json_or_form_data function from tool_shed to ga…
heisner-tillman Feb 8, 2024
b049ff2
Refactor execute operation to receive files in payload and adjust the…
heisner-tillman Feb 8, 2024
f69e11f
Regenrate the client schema
heisner-tillman Feb 8, 2024
f298968
Differentiate between type of files on check to process them
heisner-tillman Feb 8, 2024
00ce35e
Remove json keyword from interactor method, when posting to /api/tools
heisner-tillman Feb 15, 2024
ecd3a48
Regenerate the client schema
heisner-tillman Feb 26, 2024
37e5a41
Regenerate the client schema
heisner-tillman Mar 7, 2024
6826dbf
Add typing to input of service methods used in execute operation
heisner-tillman Apr 2, 2024
ae36dfe
Type collections in return model of execute oerpation
heisner-tillman Apr 3, 2024
b272fd5
Type tool_id with str in payload of execute operation
heisner-tillman Apr 3, 2024
4379464
Remove unused typing
heisner-tillman Apr 4, 2024
9be481d
Do not encode id's in method, as it is handled by the return model
heisner-tillman Apr 4, 2024
e8133c6
Type outputs field in model ToolResponse with extended HDA models
heisner-tillman Apr 4, 2024
95c02d4
Rename ToolResponse model and only use it in execute operation of the…
heisner-tillman Apr 7, 2024
8a44ea0
Rename the class FetchTools to FastAPITools as it covers more endpoin…
heisner-tillman Apr 9, 2024
a3ba4a7
Type field tool_uuid with UUID4 in ExecuteToolPayload model
heisner-tillman Apr 9, 2024
0d0ada3
Cast tool_uuid to string - if present in the payload - after validati…
heisner-tillman Apr 11, 2024
d125e30
Apply code suggestions
heisner-tillman Apr 18, 2024
8d97c50
Remove invalid parameter when creating payload to run a tool
heisner-tillman Apr 18, 2024
60a5bbf
Add an optional tool version field to the JobBaseModel
heisner-tillman Apr 18, 2024
b033aa4
Narrow down the HDA, HDCA and Jobmodels used in the response model of…
heisner-tillman Apr 18, 2024
71f7bfc
Apply code suggestions
heisner-tillman Apr 22, 2024
a10088a
Add missing field state to payload model of the execute operation
heisner-tillman Apr 22, 2024
54448e5
Add validator for extra fields to payload model of the execute operat…
heisner-tillman Apr 22, 2024
3d803d3
Adjust error type that get raised by model validator
heisner-tillman Apr 29, 2024
2d65739
Add missing field in payload model of execute operation
heisner-tillman Apr 29, 2024
dc03471
Add missing field in payload model of execute operation
heisner-tillman May 13, 2024
3d6c325
Regenerate the client schema
heisner-tillman May 13, 2024
34fe594
Add umask-friendly permissions fixing, when creating a temporary file
heisner-tillman May 21, 2024
d7833d3
Regenerate the client schema
heisner-tillman May 30, 2024
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
481 changes: 481 additions & 0 deletions client/src/api/schema/schema.ts

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions lib/galaxy/schema/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,10 @@ class JobBaseModel(Model, WithModelClass):
title="Tool ID",
description="Identifier of the tool that generated this job.",
)
tool_version: Optional[str] = Field(
None,
description="Version of the tool that generated this job.",
)
state: JobState = Field(
...,
title="State",
Expand Down
146 changes: 146 additions & 0 deletions lib/galaxy/schema/tools.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import json
from typing import (
Any,
Dict,
List,
Optional,
)

from pydantic import (
ConfigDict,
Field,
field_validator,
model_validator,
UUID4,
)
from typing_extensions import (
Annotated,
Literal,
Self,
)

from galaxy.schema.fields import DecodedDatabaseIdField
from galaxy.schema.schema import (
HDACustom,
HDCADetailed,
JobBaseModel,
Model,
)

ToolOutputName = Annotated[
str,
Field(
...,
title="Output Name",
description="The name of the tool output",
),
]


class ExecuteToolPayload(Model):
tool_id: Optional[str] = Field(
default=None,
heisner-tillman marked this conversation as resolved.
Show resolved Hide resolved
)
tool_uuid: Optional[UUID4] = Field(
default=None,
description="The UUID of the tool to execute.",
)
action: Optional[str] = Field(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with this, where did you find that ?

Copy link
Member

Choose a reason for hiding this comment

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

This one's deprecated and raises an exception if provided, we can remove that

Suggested change
action: Optional[str] = Field(

default=None,
description="The action to perform",
)
tool_version: Optional[str] = Field(
default=None,
)
history_id: Optional[DecodedDatabaseIdField] = Field(
default=None,
)

@field_validator("inputs", mode="before", check_fields=False)
@classmethod
def inputs_string_to_json(cls, v):
if isinstance(v, str):
return json.loads(v)
return v

inputs: Dict[str, Any] = Field(
default={},
description="The input parameters to use when executing a tool. Keys correspond to parameter names, values are the values set for these parameters. If parameter values are left unset defaults will be used if possible.",
)
use_cached_job: Optional[bool] = Field(
default=False,
description="If set to `true` an attempt is made to find an equivalent job and use its outputs, thereby skipping execution of the job on the compute infrastructure.",
)
preferred_object_store_id: Optional[str] = Field(
default=None,
)
input_format: Optional[Literal["legacy", "21.01"]] = Field(
default=None,
title="Input Format",
description="""If set to `legacy` or parameter is not set, inputs are assumed to be objects with concatenated keys for nested parameter, e.g. `{"conditional|parameter_name": "parameter_value"}`. If set to `"21.01"` inputs can be provided as nested objects, e.g `{"conditional": {"parameter_name": "parameter_value"}}`.""",
)
data_manager_mode: Optional[Literal["populate", "dry_run", "bundle"]] = Field(
default=None,
)
send_email_notification: Optional[bool] = Field(
default=None,
title="Send Email Notification",
description="Flag indicating whether to send email notification",
)
state: Optional[str] = Field(
default=None,
)
type: Optional[str] = Field(
default=None,
)
assert_ok: Optional[bool] = Field(
default=None,
)
model_config = ConfigDict(extra="allow")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you expect any other field to be here that cannot be part of the model? If not we should avoid using extra=allow.

Copy link
Contributor Author

@heisner-tillman heisner-tillman Apr 2, 2024

Choose a reason for hiding this comment

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

Yes, we expect other fields. They fields will have the prefix "files_" and it is not possible to narrow them down further (I think). The execute method (line 131 -152) in the service layer handles them like this:

    # process files, when they come in as multipart file data
    files = {}
    for key in list(create_payload.keys()):
        if key.startswith("files_") and isinstance(create_payload[key], UploadFile):
            files[key] = self.create_temp_file_execute(trans, create_payload.pop(key))
    create_payload.update(files)

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a validator that validates the whole model ? https://docs.pydantic.dev/latest/concepts/validators/#model-validators mode="after" and then check that the only fields not defined in the model are files_* fields ?


@model_validator(mode="after")
def validate_extra_fields(self) -> Self:
if self.model_extra is not None:
for field_name in self.model_extra.keys():
if not field_name.startswith("files_"):
raise ValueError(f"Invalid field name: {field_name}")
return self


# The following models should eventually be removed as we move towards creating jobs asynchronously
# xref: https://github.com/galaxyproject/galaxy/pull/17393
class HDAToolOutput(HDACustom):
output_name: ToolOutputName


class HDCAToolOutput(HDCADetailed):
output_name: ToolOutputName


class ExecuteToolResponse(Model):
outputs: List[HDAToolOutput] = Field(
default=[],
title="Outputs",
description="The outputs of the job.",
)
output_collections: List[HDCAToolOutput] = Field(
default=[],
title="Output Collections",
description="The output dataset collections of the job.",
)
jobs: List[JobBaseModel] = Field(
default=[],
title="Jobs",
description="The jobs of the tool.",
)
implicit_collections: List[HDCAToolOutput] = Field(
default=[],
title="Implicit Collections",
description="The implicit dataset collections of the job.",
)
produces_entry_points: bool = Field(
default=...,
title="Produces Entry Points",
description="Flag indicating whether the creation of the tool produces entry points for interactive tools.",
)
errors: List[Any] = Field(default=[], title="Errors", description="Errors encountered while executing the tool.")
27 changes: 27 additions & 0 deletions lib/galaxy/webapps/galaxy/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import inspect
from enum import Enum
from json import JSONDecodeError
from string import Template
from typing import (
Any,
Expand Down Expand Up @@ -67,6 +68,7 @@
)
from galaxy.exceptions import (
AdminRequiredException,
MessageException,
UserCannotRunAsException,
)
from galaxy.managers.session import GalaxySessionManager
Expand Down Expand Up @@ -109,6 +111,8 @@ async def get_app_with_request_session() -> AsyncGenerator[StructuredApp, None]:

T = TypeVar("T")

B = TypeVar("B", bound=BaseModel)


class GalaxyTypeDepends(Depends):
"""Variant of fastapi Depends that can also work on WSGI Galaxy controllers."""
Expand Down Expand Up @@ -602,3 +606,26 @@ def search_query_param(model_name: str, tags: list, free_text_fields: list) -> O
title="Search query.",
description=description,
)


# TODO: mypy complains that Depends(get_body) is returned B is expected.
# def depend_on_either_json_or_form_data(model: Type[B]) -> B:
def depend_on_either_json_or_form_data(model: Type[B]):
async def get_body(request: Request):
content_type = request.headers.get("Content-Type")
if content_type is None:
raise MessageException("No Content-Type provided!")
elif content_type == "application/json":
try:
return model(**await request.json())
except JSONDecodeError:
raise MessageException("Invalid JSON data")
elif content_type == "application/x-www-form-urlencoded" or content_type.startswith("multipart/form-data"):
try:
return model(**await request.form())
except Exception:
raise MessageException("Invalid Form data")
else:
raise MessageException("Content-Type not supported!")

return Depends(get_body)
44 changes: 17 additions & 27 deletions lib/galaxy/webapps/galaxy/api/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
FetchDataFormPayload,
FetchDataPayload,
)
from galaxy.schema.tools import (
ExecuteToolPayload,
ExecuteToolResponse,
)
from galaxy.tools.evaluation import global_tool_errors
from galaxy.util.zipstream import ZipstreamWrapper
from galaxy.web import (
Expand All @@ -46,16 +50,14 @@
APIContentTypeRoute,
as_form,
BaseGalaxyAPIController,
depend_on_either_json_or_form_data,
depends,
DependsOnTrans,
Router,
)

log = logging.getLogger(__name__)

# Do not allow these tools to be called directly - they (it) enforces extra security and
# provides access via a different API endpoint.
PROTECTED_TOOLS = ["__DATA_FETCH__"]
# Tool search bypasses the fulltext for the following list of terms
SEARCH_RESERVED_TERMS_FAVORITES = ["#favs", "#favorites", "#favourites"]

Expand All @@ -74,7 +76,7 @@ class JsonApiRoute(APIContentTypeRoute):


@router.cbv
class FetchTools:
class FastAPITools:
service: ToolsService = depends(ToolsService)

@router.post("/api/tools/fetch", summary="Upload files to Galaxy", route_class_override=JsonApiRoute)
Expand Down Expand Up @@ -103,6 +105,17 @@ async def fetch_form(
files2.append(value)
return self.service.create_fetch(trans, payload, files2)

@router.post(
"/api/tools",
summary="Execute tool with a given parameter payload",
)
def execute(
self,
payload: ExecuteToolPayload = depend_on_either_json_or_form_data(ExecuteToolPayload),
trans: ProvidesHistoryContext = DependsOnTrans,
) -> ExecuteToolResponse:
return self.service.execute(trans, payload)


class ToolsController(BaseGalaxyAPIController, UsesVisualizationMixin):
"""
Expand Down Expand Up @@ -569,29 +582,6 @@ def error_stack(self, trans: GalaxyWebTransaction, **kwd):
"""
return global_tool_errors.error_stack

@expose_api_anonymous
def create(self, trans: GalaxyWebTransaction, payload, **kwd):
"""
POST /api/tools
Execute tool with a given parameter payload

:param input_format: input format for the payload. Possible values are
the default 'legacy' (where inputs nested inside conditionals or
repeats are identified with e.g. '<conditional_name>|<input_name>') or
'21.01' (where inputs inside conditionals or repeats are nested
elements).
:type input_format: str
"""
tool_id = payload.get("tool_id")
tool_uuid = payload.get("tool_uuid")
if tool_id in PROTECTED_TOOLS:
raise exceptions.RequestParameterInvalidException(
f"Cannot execute tool [{tool_id}] directly, must use alternative endpoint."
)
if tool_id is None and tool_uuid is None:
raise exceptions.RequestParameterInvalidException("Must specify a valid tool_id to use this endpoint.")
return self.service._create(trans, payload, **kwd)


def _kwd_or_payload(kwd: Dict[str, Any]) -> Dict[str, Any]:
if "payload" in kwd:
Expand Down
7 changes: 6 additions & 1 deletion lib/galaxy/webapps/galaxy/buildapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,13 @@ def populate_api_routes(webapp, app):
controller="tools",
conditions=dict(method=["POST"]),
)
webapp.mapper.connect(
"api/tools",
action="index",
controller="tools",
conditions=dict(method=["GET"]),
)
webapp.mapper.connect("/api/tools/{id:.+?}", action="show", controller="tools")
webapp.mapper.resource("tool", "tools", path_prefix="/api")
webapp.mapper.resource("dynamic_tools", "dynamic_tools", path_prefix="/api")

webapp.mapper.connect(
Expand Down
Loading
Loading