Skip to content

Commit

Permalink
feat(robot server): add a POST method on the analyses endpoint (#14828)
Browse files Browse the repository at this point in the history
Closes AUTH-255

# Overview

Adds a POST method to the existing `/protocols/{protocolId}/analyses`
endpoint in order to post a new analysis for an existing protocol.

This endpoint will take a request body with two optional fields:
- `runTimeParameterValues`
- `forceReAnalyze`

The new method can affect the analyses in three ways:
1. When the request is sent with `forceReAnalyze=True`, the server will
unconditionally start a new analysis for the protocol using any RTP data
sent along with it. It will return a 201 CREATED status and respond with
a list of analysis summaries of all the analyses (ordered oldest first),
including the newly started analysis.
2. When the request is sent without the `forceReAnalyze` field (or with
`forceReAnalyze=False`), then the server will check the last analysis of
the protocol
- if the RTP values used for it were **different** from the RTP values
sent with the current request, then the server will start a new analysis
using the new RTP values. It will return a 201 CREATED status and
respond with a list of analysis summaries of all the analyses, including
the newly started analysis.
- if the RTP values used for it were the **same** as the RTP values sent
with the current request, then the server will **NOT** start a new
analysis. It will return a 200 OK status, and simply return the existing
list of analysis summaries.

This request requires the last analysis of the protocol to have been
completed before handling this request. If the last analysis is pending,
it will return a 503 error.

# Test Plan

Test out the above three cases and anything else you can think might
affect the behavior.
It's pretty well tested in unit & integration tests and it is also the
same logic used for handling analyses from `POST /protocols`, so it is
expected to work well when used as tested in integration tests.

# Review requests

Usual review for code sanity check.

# Risk assessment

Low. New HTTP API not yet used anywhere.
  • Loading branch information
sanni-t authored Apr 8, 2024
1 parent 2a717d7 commit 75acb05
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 34 deletions.
14 changes: 13 additions & 1 deletion robot-server/robot_server/protocols/analysis_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# TODO(mc, 2021-08-25): add modules to simulation result
from enum import Enum

from opentrons.protocol_engine.types import RunTimeParameter
from opentrons.protocol_engine.types import RunTimeParameter, RunTimeParamValuesType
from opentrons_shared_data.robot.dev_types import RobotType
from pydantic import BaseModel, Field
from typing import List, Optional, Union, NamedTuple
Expand Down Expand Up @@ -40,6 +40,18 @@ class AnalysisResult(str, Enum):
NOT_OK = "not-ok"


class AnalysisRequest(BaseModel):
"""Model for analysis request body."""

runTimeParameterValues: RunTimeParamValuesType = Field(
default={},
description="Key-value pairs of run-time parameters defined in a protocol.",
)
forceReAnalyze: bool = Field(
False, description="Whether to force start a new analysis."
)


class AnalysisSummary(BaseModel):
"""Base model for an analysis of a protocol."""

Expand Down
166 changes: 135 additions & 31 deletions robot-server/robot_server/protocols/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
from textwrap import dedent
from datetime import datetime
from pathlib import Path
from typing import List, Optional, Union
from typing import List, Optional, Union, Tuple

from opentrons.protocol_engine.types import RunTimeParamValuesType
from opentrons_shared_data.robot import user_facing_robot_type
from typing_extensions import Literal

Expand All @@ -32,13 +33,14 @@
SimpleEmptyBody,
MultiBodyMeta,
PydanticResponse,
RequestModel,
)

from .protocol_auto_deleter import ProtocolAutoDeleter
from .protocol_models import Protocol, ProtocolFile, Metadata
from .protocol_analyzer import ProtocolAnalyzer
from .analysis_store import AnalysisStore, AnalysisNotFoundError, AnalysisIsPendingError
from .analysis_models import ProtocolAnalysis
from .analysis_models import ProtocolAnalysis, AnalysisRequest, AnalysisSummary
from .protocol_store import (
ProtocolStore,
ProtocolResource,
Expand Down Expand Up @@ -162,7 +164,7 @@ class ProtocolLinks(BaseModel):
status.HTTP_503_SERVICE_UNAVAILABLE: {"model": ErrorBody[LastAnalysisPending]},
},
)
async def create_protocol( # noqa: C901
async def create_protocol(
files: List[UploadFile] = File(...),
# use Form because request is multipart/form-data
# https://fastapi.tiangolo.com/tutorial/request-forms-and-files/
Expand Down Expand Up @@ -238,35 +240,18 @@ async def create_protocol( # noqa: C901

if cached_protocol_id is not None:
resource = protocol_store.get(protocol_id=cached_protocol_id)
analyses = analysis_store.get_summaries_by_protocol(
protocol_id=cached_protocol_id
)

try:
if (
# Unexpected situations, like powering off the robot after a protocol upload
# but before the analysis is complete, can leave the protocol resource
# without an associated analysis.
len(analyses) == 0
or
# The most recent analysis was done using different RTP values
not await analysis_store.matching_rtp_values_in_analysis(
analysis_summary=analyses[-1], new_rtp_values=parsed_rtp
)
):
# This protocol exists in database but needs to be (re)analyzed
task_runner.run(
protocol_analyzer.analyze,
protocol_resource=resource,
analysis_id=analysis_id,
run_time_param_values=parsed_rtp,
)
analyses.append(
analysis_store.add_pending(
protocol_id=cached_protocol_id,
analysis_id=analysis_id,
)
)
analysis_summaries, _ = await _start_new_analysis_if_necessary(
protocol_id=cached_protocol_id,
analysis_id=analysis_id,
rtp_values=parsed_rtp,
force_reanalyze=False,
protocol_store=protocol_store,
analysis_store=analysis_store,
protocol_analyzer=protocol_analyzer,
task_runner=task_runner,
)
except AnalysisIsPendingError as error:
raise LastAnalysisPending(detail=str(error)).as_error(
status.HTTP_503_SERVICE_UNAVAILABLE
Expand All @@ -278,7 +263,7 @@ async def create_protocol( # noqa: C901
protocolType=resource.source.config.protocol_type,
robotType=resource.source.robot_type,
metadata=Metadata.parse_obj(resource.source.metadata),
analysisSummaries=analyses,
analysisSummaries=analysis_summaries,
key=resource.protocol_key,
files=[
ProtocolFile(name=f.path.name, role=f.role)
Expand Down Expand Up @@ -357,6 +342,53 @@ async def create_protocol( # noqa: C901
)


async def _start_new_analysis_if_necessary(
protocol_id: str,
analysis_id: str,
force_reanalyze: bool,
rtp_values: RunTimeParamValuesType,
protocol_store: ProtocolStore,
analysis_store: AnalysisStore,
protocol_analyzer: ProtocolAnalyzer,
task_runner: TaskRunner,
) -> Tuple[List[AnalysisSummary], bool]:
"""Check RTP values and start a new analysis if necessary.
Returns a tuple of the latest list of analysis summaries (including any newly
started analysis) and whether a new analysis was started.
"""
resource = protocol_store.get(protocol_id=protocol_id)
analyses = analysis_store.get_summaries_by_protocol(protocol_id=protocol_id)
started_new_analysis = False
if (
force_reanalyze
or
# Unexpected situations, like powering off the robot after a protocol upload
# but before the analysis is complete, can leave the protocol resource
# without an associated analysis.
len(analyses) == 0
or
# The most recent analysis was done using different RTP values
not await analysis_store.matching_rtp_values_in_analysis(
analysis_summary=analyses[-1], new_rtp_values=rtp_values
)
):
task_runner.run(
protocol_analyzer.analyze,
protocol_resource=resource,
analysis_id=analysis_id,
run_time_param_values=rtp_values,
)
started_new_analysis = True
analyses.append(
analysis_store.add_pending(
protocol_id=protocol_id,
analysis_id=analysis_id,
)
)
return analyses, started_new_analysis


@PydanticResponse.wrap_route(
protocols_router.get,
path="/protocols",
Expand Down Expand Up @@ -519,6 +551,78 @@ async def delete_protocol_by_id(
)


@PydanticResponse.wrap_route(
protocols_router.post,
path="/protocols/{protocolId}/analyses",
summary="Analyze the protocol",
description=dedent(
"""
Generate an analysis for the protocol, based on last analysis and current request data.
"""
),
status_code=status.HTTP_201_CREATED,
responses={
status.HTTP_200_OK: {"model": SimpleMultiBody[AnalysisSummary]},
status.HTTP_201_CREATED: {"model": SimpleMultiBody[AnalysisSummary]},
status.HTTP_404_NOT_FOUND: {"model": ErrorBody[ProtocolNotFound]},
status.HTTP_503_SERVICE_UNAVAILABLE: {"model": ErrorBody[LastAnalysisPending]},
},
)
async def create_protocol_analysis(
protocolId: str,
request_body: Optional[RequestModel[AnalysisRequest]] = None,
protocol_store: ProtocolStore = Depends(get_protocol_store),
analysis_store: AnalysisStore = Depends(get_analysis_store),
protocol_analyzer: ProtocolAnalyzer = Depends(get_protocol_analyzer),
task_runner: TaskRunner = Depends(get_task_runner),
analysis_id: str = Depends(get_unique_id, use_cache=False),
) -> PydanticResponse[SimpleMultiBody[AnalysisSummary]]:
"""Start a new analysis for the given existing protocol.
Starts a new analysis for the protocol along with the provided run-time parameter
values (if any), and appends it to the existing analyses.
If the last analysis in the existing analyses used the same RTP values, then a new
analysis is not created.
If `forceAnalyze` is True, this will always start a new analysis.
Returns: List of analysis summaries available for the protocol, ordered as
most recently started analysis last.
"""
if not protocol_store.has(protocolId):
raise ProtocolNotFound(detail=f"Protocol {protocolId} not found").as_error(
status.HTTP_404_NOT_FOUND
)
try:
(
analysis_summaries,
started_new_analysis,
) = await _start_new_analysis_if_necessary(
protocol_id=protocolId,
analysis_id=analysis_id,
rtp_values=request_body.data.runTimeParameterValues if request_body else {},
force_reanalyze=request_body.data.forceReAnalyze if request_body else False,
protocol_store=protocol_store,
analysis_store=analysis_store,
protocol_analyzer=protocol_analyzer,
task_runner=task_runner,
)
except AnalysisIsPendingError as error:
raise LastAnalysisPending(detail=str(error)).as_error(
status.HTTP_503_SERVICE_UNAVAILABLE
) from error
return await PydanticResponse.create(
content=SimpleMultiBody.construct(
data=analysis_summaries,
meta=MultiBodyMeta(cursor=0, totalLength=len(analysis_summaries)),
),
status_code=status.HTTP_201_CREATED
if started_new_analysis
else status.HTTP_200_OK,
)


@PydanticResponse.wrap_route(
protocols_router.get,
path="/protocols/{protocolId}/analyses",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,22 @@ stages:
# We need to make sure we get the Content-Type right because FastAPI won't do it for us.
Content-Type: application/json
json: !force_format_include '{analysis_data}'


- name: Check that a new analysis is started with forceReAnalyze
request:
url: '{ot2_server_base_url}/protocols/{protocol_id}/analyses'
method: POST
json:
data:
forceReAnalyze: true
response:
strict:
- json:off
status_code: 201
json:
data:
- id: '{analysis_id}'
status: completed
- id: !anystr
status: pending
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,25 @@ stages:
description: What pipette to use during the protocol.
commands:
# Check for this command's presence as a smoke test that the analysis isn't empty.
- commandType: loadPipette
- commandType: loadPipette

- name: Check that a new analysis is started for the protocol because of new RTP values
request:
url: '{ot2_server_base_url}/protocols/{protocol_id}/analyses'
method: POST
json:
data:
runTimeParameterValues:
sample_count: 2
response:
strict:
- json:off
status_code: 201
json:
data:
- id: '{analysis_id}'
status: completed
- id: '{analysis_id2}'
status: completed
- id: !anystr
status: pending
Loading

0 comments on commit 75acb05

Please sign in to comment.