refactor(robot-server): move run time params to their own DB table

api/src/opentrons/protocol_api/_parameter_context.py

@@ -15,7 +15,7 @@
 from opentrons.protocols.parameters.exceptions import ParameterDefinitionError
 from opentrons.protocol_engine.types import (
     RunTimeParameter,
-    RunTimeParamValuesType,
+    PrimitiveRunTimeParamValuesType,
 )
 
 from ._parameters import Parameters
@@ -185,7 +185,9 @@ def add_csv_file(
         )
         self._parameters[parameter.variable_name] = parameter
 
-    def set_parameters(self, parameter_overrides: RunTimeParamValuesType) -> None:
+    def set_parameters(
+        self, parameter_overrides: PrimitiveRunTimeParamValuesType
+    ) -> None:
         """Sets parameters to values given by client, validating them as well.
 
         :meta private:

api/src/opentrons/protocol_engine/types.py

@@ -1053,6 +1053,6 @@ class CSVParameter(RTPBase):
 
 RunTimeParameter = Union[NumberParameter, EnumParameter, BooleanParameter, CSVParameter]
 
-RunTimeParamValuesType = Dict[
+PrimitiveRunTimeParamValuesType = Dict[
     StrictStr, Union[StrictInt, StrictFloat, StrictBool, StrictStr]
 ]  # update value types as more RTP types are added

api/src/opentrons/protocol_runner/protocol_runner.py

@@ -43,7 +43,7 @@
     PostRunHardwareState,
     DeckConfigurationType,
     RunTimeParameter,
-    RunTimeParamValuesType,
+    PrimitiveRunTimeParamValuesType,
 )
 from ..protocols.types import PythonProtocol
 
@@ -130,7 +130,7 @@ async def run(
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
-        run_time_param_values: Optional[RunTimeParamValuesType] = None,
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType] = None,
     ) -> RunResult:
         """Run a given protocol to completion."""
 
@@ -184,7 +184,7 @@ async def load(
         self,
         protocol_source: ProtocolSource,
         python_parse_mode: PythonParseMode,
-        run_time_param_values: Optional[RunTimeParamValuesType],
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType],
     ) -> None:
         """Load a Python or JSONv5(& older) ProtocolSource into managed ProtocolEngine."""
         labware_definitions = await protocol_reader.extract_labware_definitions(
@@ -250,7 +250,7 @@ async def run(  # noqa: D102
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
-        run_time_param_values: Optional[RunTimeParamValuesType] = None,
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType] = None,
         python_parse_mode: PythonParseMode = PythonParseMode.NORMAL,
     ) -> RunResult:
         # TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
@@ -361,7 +361,7 @@ async def run(  # noqa: D102
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
-        run_time_param_values: Optional[RunTimeParamValuesType] = None,
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType] = None,
     ) -> RunResult:
         # TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
         # currently `protocol_source` arg is only used by tests
@@ -433,7 +433,7 @@ async def run(  # noqa: D102
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
-        run_time_param_values: Optional[RunTimeParamValuesType] = None,
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType] = None,
     ) -> RunResult:
         assert protocol_source is None
         await self._hardware_api.home()

api/src/opentrons/protocol_runner/python_protocol_wrappers.py

@@ -13,7 +13,7 @@
 from opentrons.hardware_control import HardwareControlAPI
 from opentrons.legacy_broker import LegacyBroker
 from opentrons.protocol_engine import ProtocolEngine
-from opentrons.protocol_engine.types import RunTimeParamValuesType
+from opentrons.protocol_engine.types import PrimitiveRunTimeParamValuesType
 from opentrons.protocol_reader import ProtocolSource, ProtocolFileRole
 from opentrons.util.broker import Broker
 
@@ -161,7 +161,7 @@ async def execute(
     def extract_run_parameters(
         protocol: PythonProtocol,
         parameter_context: ParameterContext,
-        run_time_param_overrides: Optional[RunTimeParamValuesType],
+        run_time_param_overrides: Optional[PrimitiveRunTimeParamValuesType],
     ) -> Optional[Parameters]:
         """Extract the parameters defined in the protocol, overridden with values for the run."""
         return exec_add_parameters(

api/src/opentrons/protocol_runner/run_orchestrator.py

@@ -31,7 +31,7 @@
     LabwareOffset,
     DeckConfigurationType,
     RunTimeParameter,
-    RunTimeParamValuesType,
+    PrimitiveRunTimeParamValuesType,
 )
 from ..protocol_reader import JsonProtocolConfig, PythonProtocolConfig, ProtocolSource
 from ..protocols.parse import PythonParseMode
@@ -168,7 +168,7 @@ async def run(
         self,
         deck_configuration: DeckConfigurationType,
         protocol_source: Optional[ProtocolSource] = None,
-        run_time_param_values: Optional[RunTimeParamValuesType] = None,
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType] = None,
     ) -> RunResult:
         """Start the run."""
         if self._protocol_runner:
@@ -227,7 +227,20 @@ def get_loaded_labware_definitions(self) -> List[LabwareDefinition]:
         return self._protocol_engine.state_view.labware.get_loaded_labware_definitions()
 
     def get_run_time_parameters(self) -> List[RunTimeParameter]:
-        """Parameter definitions defined by protocol, if any. Will always be empty before execution."""
+        """Get the list of run time parameters defined in the protocol, if any.
+
+        This returns a list of all run time parameters with their validated definitions
+        and client-requested values. Will always be empty before loading the runner.
+
+        If there was an error during RTP definition validation, then this list will
+        contain the parameter definitions that were validated before the error occurred.
+        These parameters' values will be default values.
+
+        If all definitions validated successfully but an error occurred while
+        setting the RTP values with those sent by the client, then only the parameters
+        whose values were successfully set will have the client-requested values while
+        the others will contain the default values.
+        """
         return (
             []
             if self._protocol_runner is None
@@ -323,7 +336,7 @@ def get_protocol_runner(self) -> Optional[Union[JsonRunner, PythonAndLegacyRunne
     async def load(
         self,
         protocol_source: ProtocolSource,
-        run_time_param_values: Optional[RunTimeParamValuesType],
+        run_time_param_values: Optional[PrimitiveRunTimeParamValuesType],
         parse_mode: ParseMode,
     ) -> None:
         """Load a json/python protocol."""

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

@@ -10,7 +10,7 @@
 from opentrons.protocol_api._parameters import Parameters
 from opentrons.protocols.execution.errors import ExceptionInProtocolError
 from opentrons.protocols.types import PythonProtocol, MalformedPythonProtocolError
-from opentrons.protocol_engine.types import RunTimeParamValuesType
+from opentrons.protocol_engine.types import PrimitiveRunTimeParamValuesType
 
 
 from opentrons_shared_data.errors.exceptions import ExecutionCancelledError
@@ -67,7 +67,7 @@ def _raise_pretty_protocol_error(exception: Exception, filename: str) -> None:
 
 def _parse_and_set_parameters(
     parameter_context: ParameterContext,
-    run_time_param_overrides: Optional[RunTimeParamValuesType],
+    run_time_param_overrides: Optional[PrimitiveRunTimeParamValuesType],
     new_globs: Dict[Any, Any],
     filename: str,
 ) -> Parameters:
@@ -104,7 +104,7 @@ def _get_filename(
 def exec_add_parameters(
     protocol: PythonProtocol,
     parameter_context: ParameterContext,
-    run_time_param_overrides: Optional[RunTimeParamValuesType],
+    run_time_param_overrides: Optional[PrimitiveRunTimeParamValuesType],
 ) -> Optional[Parameters]:
     """Exec the add_parameters function and get the final run time parameters with overrides."""
     new_globs: Dict[Any, Any] = {}

robot-server/robot_server/persistence/__init__.py

@@ -1 +1,5 @@
 """Support for persisting data across device reboots."""
+
+from ._files_and_directories import LATEST_VERSION_DIRECTORY
+
+__all__ = ["LATEST_VERSION_DIRECTORY"]

robot-server/robot_server/persistence/_files_and_directories.py

@@ -0,0 +1,7 @@
+"""Files and directories in persistent storage."""
+from typing import Final
+
+DECK_CONFIGURATION_FILE: Final = "deck_configuration.json"
+PROTOCOLS_DIRECTORY: Final = "protocols"
+DB_FILE: Final = "robot_server.db"
+LATEST_VERSION_DIRECTORY: Final = "5.1"

robot-server/robot_server/persistence/_migrations/up_to_3.py

@@ -33,33 +33,31 @@
 )
 from ..tables import schema_2, schema_3
 from .._folder_migrator import Migration
+from .._files_and_directories import (
+    DECK_CONFIGURATION_FILE,
+    PROTOCOLS_DIRECTORY,
+    DB_FILE,
+)
 from ._util import copy_rows_unmodified, copy_if_exists, copytree_if_exists
 from . import up_to_2
-
 from . import _up_to_3_worker
 
 
-# TODO: Define a single source of truth somewhere for these paths.
-_DECK_CONFIGURATION_FILE = "deck_configuration.json"
-_PROTOCOLS_DIRECTORY = "protocols"
-_DB_FILE = "robot_server.db"
-
-
 _log = getLogger(__name__)
 
 
 class MigrationUpTo3(Migration):  # noqa: D101
     def migrate(self, source_dir: Path, dest_dir: Path) -> None:
         """Migrate the persistence directory from schema 2 to 3."""
         copy_if_exists(
-            source_dir / _DECK_CONFIGURATION_FILE, dest_dir / _DECK_CONFIGURATION_FILE
+            source_dir / DECK_CONFIGURATION_FILE, dest_dir / DECK_CONFIGURATION_FILE
         )
         copytree_if_exists(
-            source_dir / _PROTOCOLS_DIRECTORY, dest_dir / _PROTOCOLS_DIRECTORY
+            source_dir / PROTOCOLS_DIRECTORY, dest_dir / PROTOCOLS_DIRECTORY
         )
 
-        source_db_file = source_dir / _DB_FILE
-        dest_db_file = dest_dir / _DB_FILE
+        source_db_file = source_dir / DB_FILE
+        dest_db_file = dest_dir / DB_FILE
 
         with ExitStack() as exit_stack:
             source_engine = exit_stack.enter_context(sql_engine_ctx(source_db_file))

robot-server/robot_server/persistence/_migrations/v4_to_v5.py

@@ -4,50 +4,128 @@
 
 - Adds a new "protocol_kind" column to protocols table
 - Adds a new "data_files" table
+- Removes the "run_time_parameter_values_and_defaults" column of analysis_table and
+  creates a separate analysis_primitive_type_rtp_table instead. The migration does not
+  port the data from run_time_parameter_values_and_defaults into analysis_primitive_type_rtp_table.
+  The consequence of which is that any checks for previous matching analysis (for protocols with RTPs only)
+  will fail and a new analysis will be triggered. This new analysis will then save its
+  RTP data to the new table. RTP data belonging to previous analyses will still be available
+  as part of the completed analysis blob.
 """
 
 from pathlib import Path
 from contextlib import ExitStack
-import shutil
-from typing import Any
 
 import sqlalchemy
 
-from ..database import sql_engine_ctx
-from ..tables import schema_5
+from ..database import sql_engine_ctx, sqlite_rowid
+from ..tables import schema_5, schema_4
 from .._folder_migrator import Migration
 
-_DB_FILE = "robot_server.db"
+from ._util import copy_rows_unmodified, copy_if_exists, copytree_if_exists
+from .._files_and_directories import (
+    DECK_CONFIGURATION_FILE,
+    PROTOCOLS_DIRECTORY,
+    DB_FILE,
+)
 
 
 class Migration4to5(Migration):  # noqa: D101
     def migrate(self, source_dir: Path, dest_dir: Path) -> None:
         """Migrate the persistence directory from schema 4 to 5."""
-        # Copy over all existing directories and files to new version
-        for item in source_dir.iterdir():
-            if item.is_dir():
-                shutil.copytree(src=item, dst=dest_dir / item.name)
-            else:
-                shutil.copy(src=item, dst=dest_dir / item.name)
-        dest_db_file = dest_dir / _DB_FILE
+        # Copy over unmodified directories and files to new version
+        copy_if_exists(
+            source_dir / DECK_CONFIGURATION_FILE, dest_dir / DECK_CONFIGURATION_FILE
+        )
+        copytree_if_exists(
+            source_dir / PROTOCOLS_DIRECTORY, dest_dir / PROTOCOLS_DIRECTORY
+        )
+
+        source_db_file = source_dir / DB_FILE
+        dest_db_file = dest_dir / DB_FILE
 
         # Append the new column to existing protocols in v4 database
         with ExitStack() as exit_stack:
+            source_engine = exit_stack.enter_context(sql_engine_ctx(source_db_file))
+
             dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file))
             schema_5.metadata.create_all(dest_engine)
 
-            def add_column(
-                engine: sqlalchemy.engine.Engine,
-                table_name: str,
-                column: Any,
-            ) -> None:
-                column_type = column.type.compile(engine.dialect)
-                engine.execute(
-                    f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
-                )
-
-            add_column(
-                dest_engine,
-                schema_5.protocol_table.name,
-                schema_5.protocol_table.c.protocol_kind,
-            )
+            source_transaction = exit_stack.enter_context(source_engine.begin())
+            dest_transaction = exit_stack.enter_context(dest_engine.begin())
+
+            _migrate_db_with_changes(source_transaction, dest_transaction)
+
+
+def _migrate_db_with_changes(
+    source_transaction: sqlalchemy.engine.Connection,
+    dest_transaction: sqlalchemy.engine.Connection,
+) -> None:
+    _migrate_protocol_table_with_new_protocol_kind_col(
+        source_transaction,
+        dest_transaction,
+    )
+    _migrate_analysis_table_excluding_rtp_defaults_and_vals(
+        source_transaction,
+        dest_transaction,
+    )
+    copy_rows_unmodified(
+        schema_4.run_table,
+        schema_5.run_table,
+        source_transaction,
+        dest_transaction,
+        order_by_rowid=True,
+    )
+    copy_rows_unmodified(
+        schema_4.action_table,
+        schema_5.action_table,
+        source_transaction,
+        dest_transaction,
+        order_by_rowid=True,
+    )
+    copy_rows_unmodified(
+        schema_4.run_command_table,
+        schema_5.run_command_table,
+        source_transaction,
+        dest_transaction,
+        order_by_rowid=True,
+    )
+
+
+def _migrate_protocol_table_with_new_protocol_kind_col(
+    source_transaction: sqlalchemy.engine.Connection,
+    dest_transaction: sqlalchemy.engine.Connection,
+) -> None:
+    """Add a new 'protocol_kind' column to protocols table."""
+    select_old_protocols = sqlalchemy.select(schema_4.protocol_table).order_by(
+        sqlite_rowid
+    )
+    insert_new_protocols = sqlalchemy.insert(schema_5.protocol_table)
+    for old_row in source_transaction.execute(select_old_protocols).all():
+        dest_transaction.execute(
+            insert_new_protocols,
+            id=old_row.id,
+            created_at=old_row.created_at,
+            protocol_key=old_row.protocol_key,
+            protocol_kind=None,
+        )
+
+
+def _migrate_analysis_table_excluding_rtp_defaults_and_vals(
+    source_transaction: sqlalchemy.engine.Connection,
+    dest_transaction: sqlalchemy.engine.Connection,
+) -> None:
+    """Remove run_time_parameter_values_and_defaults column from analysis_table."""
+    select_old_analyses = sqlalchemy.select(schema_4.analysis_table).order_by(
+        sqlite_rowid
+    )
+    insert_new_analyses = sqlalchemy.insert(schema_5.analysis_table)
+    for old_row in source_transaction.execute(select_old_analyses).all():
+        dest_transaction.execute(
+            insert_new_analyses,
+            id=old_row.id,
+            protocol_id=old_row.protocol_id,
+            analyzer_version=old_row.analyzer_version,
+            completed_analysis=old_row.completed_analysis,
+            # run_time_parameter_values_and_defaults column is omitted
+        )