feat(robot-server, api): accept RTP overrides via /protocols and inje…

…ct them in python executor (#14688)

Closes AUTH-64, AUTH-65

Updates the POST /protocols API to accept an additional runTimeParameterValues form-data field. This field is expected to be a stringified JSON composed of key-value pairs of the parameter variable name and its override value.

These override values are converted into a dictionary and passed into the protocol runner, which makes them available to the python executor.

Risk assessment:

High. Affects the correctness of order and contents of analysis summaries returned by the POST /protocols endpoint when using run time parameters. This behavior will remain broken until AUTH-70 is implemented. Until then, please be cautious that a protocol analysis request that uses custom RTP values will modify an existing protocol analysis database such that the app might pick the wrong analysis for a run. I will recommend deleting such protocols (and runs) from the database after you are done testing, especially if you are testing on a shared robot.

api/src/opentrons/execute.py

@@ -600,7 +600,9 @@ def _run_file_non_pe(
 
     context.home()
     try:
-        execute_apiv2.run_protocol(protocol, context)
+        # TODO (spp, 2024-03-18): use true run-time param overrides once enabled
+        #  for cli protocol simulation/ execution
+        execute_apiv2.run_protocol(protocol, context, run_time_param_overrides=None)
     finally:
         context.cleanup()
 

api/src/opentrons/protocol_engine/types.py

@@ -909,3 +909,7 @@ class EnumParameter(RTPBase):
 
 
 RunTimeParameter = Union[IntParameter, FloatParameter, EnumParameter]
+
+RunTimeParamValuesType = Dict[
+    str, Union[float, bool, str]
+]  # update value types as more RTP types are added

api/src/opentrons/protocol_reader/protocol_reader.py

@@ -53,7 +53,10 @@ def __init__(
         self._file_hasher = file_hasher or FileHasher()
 
     async def save(
-        self, files: Sequence[BufferedFile], directory: Path, content_hash: str
+        self,
+        files: Sequence[BufferedFile],
+        directory: Path,
+        content_hash: str,
     ) -> ProtocolSource:
         """Compute a `ProtocolSource` from buffered files and save them as files.
 

api/src/opentrons/protocol_runner/legacy_wrappers.py

@@ -20,6 +20,7 @@
 )
 from opentrons.legacy_broker import LegacyBroker
 from opentrons.protocol_engine import ProtocolEngine
+from opentrons.protocol_engine.types import RunTimeParamValuesType
 from opentrons.protocol_reader import ProtocolSource, ProtocolFileRole
 from opentrons.util.broker import Broker
 
@@ -168,9 +169,13 @@ class LegacyExecutor:
     """Interface to execute Protocol API v2 protocols in a child thread."""
 
     @staticmethod
-    async def execute(protocol: LegacyProtocol, context: LegacyProtocolContext) -> None:
+    async def execute(
+        protocol: LegacyProtocol,
+        context: LegacyProtocolContext,
+        run_time_param_values: Optional[RunTimeParamValuesType],
+    ) -> None:
         """Execute a PAPIv2 protocol with a given ProtocolContext in a child thread."""
-        await to_thread.run_sync(run_protocol, protocol, context)
+        await to_thread.run_sync(run_protocol, protocol, context, run_time_param_values)
 
 
 __all__ = [

api/src/opentrons/protocol_runner/protocol_runner.py

@@ -35,7 +35,11 @@
     LegacyExecutor,
     LegacyLoadInfo,
 )
-from ..protocol_engine.types import PostRunHardwareState, DeckConfigurationType
+from ..protocol_engine.types import (
+    PostRunHardwareState,
+    DeckConfigurationType,
+    RunTimeParamValuesType,
+)
 
 
 class RunResult(NamedTuple):
@@ -110,6 +114,7 @@ async def run(
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
+        run_time_param_values: Optional[RunTimeParamValuesType] = None,
     ) -> RunResult:
         """Run a given protocol to completion."""
 
@@ -139,17 +144,18 @@ def __init__(
         self._legacy_executor = legacy_executor or LegacyExecutor()
         # TODO(mc, 2022-01-11): replace task queue with specific implementations
         # of runner interface
-        self._task_queue = (
-            task_queue or TaskQueue()
-        )  # cleanup_func=protocol_engine.finish))
+        self._task_queue = task_queue or TaskQueue()
         self._task_queue.set_cleanup_func(
             func=protocol_engine.finish,
             drop_tips_after_run=drop_tips_after_run,
             post_run_hardware_state=post_run_hardware_state,
         )
 
     async def load(
-        self, protocol_source: ProtocolSource, python_parse_mode: PythonParseMode
+        self,
+        protocol_source: ProtocolSource,
+        python_parse_mode: PythonParseMode,
+        run_time_param_values: Optional[RunTimeParamValuesType],
     ) -> None:
         """Load a Python or JSONv5(& older) ProtocolSource into managed ProtocolEngine."""
         labware_definitions = await protocol_reader.extract_labware_definitions(
@@ -186,26 +192,30 @@ async def load(
         initial_home_command = pe_commands.HomeCreate(
             params=pe_commands.HomeParams(axes=None)
         )
-        # this command homes all axes, including pipette plugner and gripper jaw
+        # this command homes all axes, including pipette plunger and gripper jaw
         self._protocol_engine.add_command(request=initial_home_command)
 
         self._task_queue.set_run_func(
             func=self._legacy_executor.execute,
             protocol=protocol,
             context=context,
+            run_time_param_values=run_time_param_values,
         )
 
     async def run(  # noqa: D102
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
+        run_time_param_values: Optional[RunTimeParamValuesType] = None,
         python_parse_mode: PythonParseMode = PythonParseMode.NORMAL,
     ) -> RunResult:
         # TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
-        # currently `protocol_source` arg is only used by tests
+        # currently `protocol_source` arg is only used by tests & protocol analyzer
         if protocol_source:
             await self.load(
-                protocol_source=protocol_source, python_parse_mode=python_parse_mode
+                protocol_source=protocol_source,
+                python_parse_mode=python_parse_mode,
+                run_time_param_values=run_time_param_values,
             )
 
         self.play(deck_configuration=deck_configuration)
@@ -305,6 +315,7 @@ async def run(  # noqa: D102
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
+        run_time_param_values: Optional[RunTimeParamValuesType] = None,
     ) -> RunResult:
         # TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
         # currently `protocol_source` arg is only used by tests
@@ -348,6 +359,7 @@ async def run(  # noqa: D102
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
+        run_time_param_values: Optional[RunTimeParamValuesType] = None,
     ) -> RunResult:
         assert protocol_source is None
         await self._hardware_api.home()

api/src/opentrons/protocols/execution/execute.py

@@ -1,4 +1,5 @@
 import logging
+from typing import Optional, Dict, Union
 
 from opentrons.protocol_api import ProtocolContext
 from opentrons.protocols.execution.execute_python import run_python
@@ -16,7 +17,12 @@
 MODULE_LOG = logging.getLogger(__name__)
 
 
-def run_protocol(protocol: Protocol, context: ProtocolContext) -> None:
+def run_protocol(
+    protocol: Protocol,
+    context: ProtocolContext,
+    # TODO (spp, 2024-03-20): move RunTimeParamValuesType to a top level types and use here
+    run_time_param_overrides: Optional[Dict[str, Union[float, bool, str]]] = None,
+) -> None:
     """Run a protocol.
 
     :param protocol: The :py:class:`.protocols.types.Protocol` to execute

api/src/opentrons/simulate.py

@@ -866,7 +866,9 @@ def _run_file_non_pe(
     context.home()
     with scraper.scrape():
         try:
-            execute.run_protocol(protocol, context)
+            # TODO (spp, 2024-03-18): use true run-time param overrides once enabled
+            #  for cli protocol simulation/ execution
+            execute.run_protocol(protocol, context, run_time_param_overrides=None)
             if (
                 isinstance(protocol, PythonProtocol)
                 and protocol.api_level >= APIVersion(2, 0)

api/tests/opentrons/protocol_runner/smoke_tests/test_protocol_runner.py

@@ -43,7 +43,11 @@ async def test_runner_with_python(
         robot_type="OT-2 Standard",
         protocol_config=protocol_source.config,
     )
-    result = await subject.run(deck_configuration=[], protocol_source=protocol_source)
+    result = await subject.run(
+        deck_configuration=[],
+        protocol_source=protocol_source,
+        run_time_param_values=None,
+    )
     commands_result = result.commands
     pipettes_result = result.state_summary.pipettes
     labware_result = result.state_summary.labware
@@ -176,7 +180,11 @@ async def test_runner_with_legacy_python(legacy_python_protocol_file: Path) -> N
         robot_type="OT-2 Standard",
         protocol_config=protocol_source.config,
     )
-    result = await subject.run(deck_configuration=[], protocol_source=protocol_source)
+    result = await subject.run(
+        deck_configuration=[],
+        protocol_source=protocol_source,
+        run_time_param_values=None,
+    )
 
     commands_result = result.commands
     pipettes_result = result.state_summary.pipettes
@@ -235,7 +243,11 @@ async def test_runner_with_legacy_json(legacy_json_protocol_file: Path) -> None:
     subject = await create_simulating_runner(
         robot_type="OT-2 Standard", protocol_config=protocol_source.config
     )
-    result = await subject.run(deck_configuration=[], protocol_source=protocol_source)
+    result = await subject.run(
+        deck_configuration=[],
+        protocol_source=protocol_source,
+        run_time_param_values=None,
+    )
 
     commands_result = result.commands
     pipettes_result = result.state_summary.pipettes

api/tests/opentrons/protocol_runner/test_protocol_runner.py

@@ -475,6 +475,7 @@ async def test_load_legacy_python(
     await legacy_python_runner_subject.load(
         legacy_protocol_source,
         python_parse_mode=PythonParseMode.ALLOW_LEGACY_METADATA_AND_REQUIREMENTS,
+        run_time_param_values=None,
     )
 
     decoy.verify(
@@ -487,6 +488,7 @@ async def test_load_legacy_python(
             func=legacy_executor.execute,
             protocol=legacy_protocol,
             context=legacy_context,
+            run_time_param_values=None,
         ),
     )
     assert broker_captor.value is legacy_python_runner_subject.broker
@@ -545,6 +547,7 @@ async def test_load_python_with_pe_papi_core(
     await legacy_python_runner_subject.load(
         legacy_protocol_source,
         python_parse_mode=PythonParseMode.ALLOW_LEGACY_METADATA_AND_REQUIREMENTS,
+        run_time_param_values=None,
     )
 
     decoy.verify(protocol_engine.add_plugin(matchers.IsA(LegacyContextPlugin)), times=0)
@@ -606,6 +609,7 @@ async def test_load_legacy_json(
     await legacy_python_runner_subject.load(
         legacy_protocol_source,
         python_parse_mode=PythonParseMode.ALLOW_LEGACY_METADATA_AND_REQUIREMENTS,
+        run_time_param_values=None,
     )
 
     decoy.verify(
@@ -618,6 +622,7 @@ async def test_load_legacy_json(
             func=legacy_executor.execute,
             protocol=legacy_protocol,
             context=legacy_context,
+            run_time_param_values=None,
         ),
     )
 

robot-server/robot_server/protocols/analysis_store.py

@@ -115,6 +115,8 @@ def add_pending(self, protocol_id: str, analysis_id: str) -> AnalysisSummary:
         Returns:
             A summary of the just-added analysis.
         """
+        # TODO (spp, 2024-03-19): cap the number of analyses being stored by
+        #  auto-deleting old ones
         new_pending_analysis = self._pending_store.add(
             protocol_id=protocol_id, analysis_id=analysis_id
         )

robot-server/robot_server/protocols/protocol_analyzer.py

@@ -1,8 +1,10 @@
 """Protocol analysis module."""
 import logging
+from typing import Optional
 
 from opentrons import protocol_runner
 from opentrons.protocol_engine.errors import ErrorOccurrence
+from opentrons.protocol_engine.types import RunTimeParamValuesType
 import opentrons.util.helpers as datetime_helper
 
 import robot_server.errors.error_mappers as em
@@ -27,6 +29,7 @@ async def analyze(
         self,
         protocol_resource: ProtocolResource,
         analysis_id: str,
+        run_time_param_values: Optional[RunTimeParamValuesType],
     ) -> None:
         """Analyze a given protocol, storing the analysis when complete."""
         runner = await protocol_runner.create_simulating_runner(
@@ -35,7 +38,9 @@ async def analyze(
         )
         try:
             result = await runner.run(
-                protocol_source=protocol_resource.source, deck_configuration=[]
+                protocol_source=protocol_resource.source,
+                deck_configuration=[],
+                run_time_param_values=run_time_param_values,
             )
         except BaseException as error:
             internal_error = em.map_unexpected_error(error=error)

robot-server/robot_server/protocols/router.py

@@ -1,4 +1,5 @@
 """Router for /protocols endpoints."""
+import json
 import logging
 from textwrap import dedent
 from datetime import datetime
@@ -165,6 +166,14 @@ async def create_protocol(
             " protocol resources on the robot."
         ),
     ),
+    run_time_parameter_values: Optional[str] = Form(
+        default=None,
+        description="Key-value pairs of run-time parameters defined in a protocol."
+        " Note that this is expected to be a string holding a JSON object."
+        " Also, if this data is included in the request, the server will"
+        " always trigger an analysis (for now).",
+        alias="runTimeParameterValues",
+    ),
     protocol_directory: Path = Depends(get_protocol_directory),
     protocol_store: ProtocolStore = Depends(get_protocol_store),
     analysis_store: AnalysisStore = Depends(get_analysis_store),
@@ -184,6 +193,7 @@ async def create_protocol(
     Arguments:
         files: List of uploaded files, from form-data.
         key: Optional key for client-side tracking
+        run_time_parameter_values: Key value pairs of run-time parameters defined in a protocol.
         protocol_directory: Location to store uploaded files.
         protocol_store: In-memory database of protocol resources.
         analysis_store: In-memory database of protocol analyses.
@@ -205,11 +215,34 @@ async def create_protocol(
         assert file.filename is not None
     buffered_files = await file_reader_writer.read(files=files)  # type: ignore[arg-type]
 
+    if isinstance(run_time_parameter_values, str):
+        # We have to do this isinstance check because if `runTimeParameterValues` is
+        # not specified in the request, then it gets assigned a Form(None) value
+        # instead of just a None. \(O.o)/
+        # TODO: check if we can make our own "RTP multipart-form field" Pydantic type
+        #  so we can validate the data contents and return a better error response.
+        parsed_rtp = json.loads(run_time_parameter_values)
+    else:
+        parsed_rtp = None
     content_hash = await file_hasher.hash(buffered_files)
     cached_protocol_id = protocol_store.get_id_by_hash(content_hash)
 
     if cached_protocol_id is not None:
+        # Protocol exists in database
         resource = protocol_store.get(protocol_id=cached_protocol_id)
+        if parsed_rtp:
+            # This protocol exists in database but needs to be re-analyzed with the
+            # passed-in RTP overrides
+            task_runner.run(
+                protocol_analyzer.analyze,
+                protocol_resource=resource,
+                analysis_id=analysis_id,
+                run_time_param_values=parsed_rtp,
+            )
+            analysis_store.add_pending(
+                protocol_id=cached_protocol_id,
+                analysis_id=analysis_id,
+            )
         analyses = analysis_store.get_summaries_by_protocol(
             protocol_id=cached_protocol_id
         )
@@ -228,7 +261,8 @@ async def create_protocol(
         )
 
         log.info(
-            f'Protocol with id "{cached_protocol_id}" with same contents already exists. returning existing protocol data in response payload'
+            f'Protocol with id "{cached_protocol_id}" with same contents already exists.'
+            f" Returning existing protocol data in response payload."
         )
 
         return await PydanticResponse.create(
@@ -238,7 +272,6 @@ async def create_protocol(
         )
 
     try:
-        # Can make the passed in RTPs as part of protocolSource returned here
         source = await protocol_reader.save(
             files=buffered_files,
             directory=protocol_directory / protocol_id,
@@ -272,6 +305,7 @@ async def create_protocol(
         protocol_analyzer.analyze,
         protocol_resource=protocol_resource,
         analysis_id=analysis_id,
+        run_time_param_values=parsed_rtp,
     )
     pending_analysis = analysis_store.add_pending(
         protocol_id=protocol_id,

robot-server/robot_server/runs/engine_store.py

@@ -214,6 +214,7 @@ async def create(
                 # was uploaded before we added stricter validation, and that
                 # doesn't conform to the new rules.
                 python_parse_mode=PythonParseMode.ALLOW_LEGACY_METADATA_AND_REQUIREMENTS,
+                run_time_param_values=None,
             )
         elif isinstance(runner, JsonRunner):
             assert (