From 282c99ccf39c2156e01aad4fe44d53a137c990e4 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 1 Nov 2024 12:45:15 -0400 Subject: [PATCH 01/53] WIP intial schema and orchestrator changes --- .../protocol_runner/protocol_runner.py | 2 + .../persistence/tables/schema_8.py | 328 ++++++++++++++++++ .../robot_server/runs/run_data_manager.py | 3 + .../runs/run_orchestrator_store.py | 6 +- robot-server/robot_server/runs/run_store.py | 9 +- 5 files changed, 346 insertions(+), 2 deletions(-) create mode 100644 robot-server/robot_server/persistence/tables/schema_8.py diff --git a/api/src/opentrons/protocol_runner/protocol_runner.py b/api/src/opentrons/protocol_runner/protocol_runner.py index aec2aae80df..20e4d8ec005 100644 --- a/api/src/opentrons/protocol_runner/protocol_runner.py +++ b/api/src/opentrons/protocol_runner/protocol_runner.py @@ -6,6 +6,7 @@ import anyio +from api.src.opentrons.protocol_engine.errors.error_occurrence import ErrorOccurrence from opentrons.hardware_control import HardwareControlAPI from opentrons import protocol_reader from opentrons.legacy_broker import LegacyBroker @@ -56,6 +57,7 @@ class RunResult(NamedTuple): commands: List[Command] state_summary: StateSummary parameters: List[RunTimeParameter] + command_errors: Optional[List[ErrorOccurrence]] = None class AbstractRunner(ABC): diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py new file mode 100644 index 00000000000..5d306a69d2e --- /dev/null +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -0,0 +1,328 @@ +"""v7 of our SQLite schema.""" +import enum +import sqlalchemy + +from robot_server.persistence._utc_datetime import UTCDateTime + +metadata = sqlalchemy.MetaData() + + +class PrimitiveParamSQLEnum(enum.Enum): + """Enum type to store primitive param type.""" + + INT = "int" + FLOAT = "float" + BOOL = "bool" + STR = "str" + + +class ProtocolKindSQLEnum(enum.Enum): + """What kind a stored protocol is.""" + + STANDARD = "standard" + QUICK_TRANSFER = "quick-transfer" + + +class DataFileSourceSQLEnum(enum.Enum): + """The source this data file is from.""" + + UPLOADED = "uploaded" + GENERATED = "generated" + + +protocol_table = sqlalchemy.Table( + "protocol", + metadata, + sqlalchemy.Column( + "id", + sqlalchemy.String, + primary_key=True, + ), + sqlalchemy.Column( + "created_at", + UTCDateTime, + nullable=False, + ), + sqlalchemy.Column("protocol_key", sqlalchemy.String, nullable=True), + sqlalchemy.Column( + "protocol_kind", + sqlalchemy.Enum( + ProtocolKindSQLEnum, + values_callable=lambda obj: [e.value for e in obj], + validate_strings=True, + create_constraint=True, + ), + index=True, + nullable=False, + ), +) + +analysis_table = sqlalchemy.Table( + "analysis", + metadata, + sqlalchemy.Column( + "id", + sqlalchemy.String, + primary_key=True, + ), + sqlalchemy.Column( + "protocol_id", + sqlalchemy.String, + sqlalchemy.ForeignKey("protocol.id"), + index=True, + nullable=False, + ), + sqlalchemy.Column( + "analyzer_version", + sqlalchemy.String, + nullable=False, + ), + sqlalchemy.Column( + "completed_analysis", + # Stores a JSON string. See CompletedAnalysisStore. + sqlalchemy.String, + nullable=False, + ), +) + +analysis_primitive_type_rtp_table = sqlalchemy.Table( + "analysis_primitive_rtp_table", + metadata, + sqlalchemy.Column( + "row_id", + sqlalchemy.Integer, + primary_key=True, + ), + sqlalchemy.Column( + "analysis_id", + sqlalchemy.ForeignKey("analysis.id"), + nullable=False, + ), + sqlalchemy.Column( + "parameter_variable_name", + sqlalchemy.String, + nullable=False, + ), + sqlalchemy.Column( + "parameter_type", + sqlalchemy.Enum( + PrimitiveParamSQLEnum, + values_callable=lambda obj: [e.value for e in obj], + create_constraint=True, + # todo(mm, 2024-09-24): Can we add validate_strings=True here? + ), + nullable=False, + ), + sqlalchemy.Column( + "parameter_value", + sqlalchemy.String, + nullable=False, + ), +) + +analysis_csv_rtp_table = sqlalchemy.Table( + "analysis_csv_rtp_table", + metadata, + sqlalchemy.Column( + "row_id", + sqlalchemy.Integer, + primary_key=True, + ), + sqlalchemy.Column( + "analysis_id", + sqlalchemy.ForeignKey("analysis.id"), + nullable=False, + ), + sqlalchemy.Column( + "parameter_variable_name", + sqlalchemy.String, + nullable=False, + ), + sqlalchemy.Column( + "file_id", + sqlalchemy.ForeignKey("data_files.id"), + nullable=True, + ), +) + +run_table = sqlalchemy.Table( + "run", + metadata, + sqlalchemy.Column( + "id", + sqlalchemy.String, + primary_key=True, + ), + sqlalchemy.Column( + "created_at", + UTCDateTime, + nullable=False, + ), + sqlalchemy.Column( + "protocol_id", + sqlalchemy.String, + sqlalchemy.ForeignKey("protocol.id"), + nullable=True, + ), + sqlalchemy.Column( + "state_summary", + sqlalchemy.String, + nullable=True, + ), + sqlalchemy.Column("engine_status", sqlalchemy.String, nullable=True), + sqlalchemy.Column("_updated_at", UTCDateTime, nullable=True), + sqlalchemy.Column( + "run_time_parameters", + # Stores a JSON string. See RunStore. + sqlalchemy.String, + nullable=True, + ), +) + +action_table = sqlalchemy.Table( + "action", + metadata, + sqlalchemy.Column( + "id", + sqlalchemy.String, + primary_key=True, + ), + sqlalchemy.Column("created_at", UTCDateTime, nullable=False), + sqlalchemy.Column("action_type", sqlalchemy.String, nullable=False), + sqlalchemy.Column( + "run_id", + sqlalchemy.String, + sqlalchemy.ForeignKey("run.id"), + nullable=False, + ), +) + +run_command_table = sqlalchemy.Table( + "run_command", + metadata, + sqlalchemy.Column("row_id", sqlalchemy.Integer, primary_key=True), + sqlalchemy.Column( + "run_id", sqlalchemy.String, sqlalchemy.ForeignKey("run.id"), nullable=False + ), + sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), + sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), + sqlalchemy.Column("command", sqlalchemy.String, nullable=False), + sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=False, index=True), + sqlalchemy.Index( + "ix_run_run_id_command_id", # An arbitrary name for the index. + "run_id", + "command_id", + unique=True, + ), + sqlalchemy.Index( + "ix_run_run_id_index_in_run", # An arbitrary name for the index. + "run_id", + "index_in_run", + unique=True, + ), +) + +run_command_table = sqlalchemy.Table( + "run_command_errors", + metadata, + sqlalchemy.Column("row_id", sqlalchemy.Integer, primary_key=True), + sqlalchemy.Column( + "run_id", sqlalchemy.String, sqlalchemy.ForeignKey("run.id"), nullable=False + ), + sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), + sqlalchemy.Column("command_error", sqlalchemy.String, nullable=False), + sqlalchemy.Index( + "ix_run_run_id_index_in_run", # An arbitrary name for the index. + "run_id", + "index_in_run", + unique=True, + ), +) + +data_files_table = sqlalchemy.Table( + "data_files", + metadata, + sqlalchemy.Column( + "id", + sqlalchemy.String, + primary_key=True, + ), + sqlalchemy.Column( + "name", + sqlalchemy.String, + nullable=False, + ), + sqlalchemy.Column( + "file_hash", + sqlalchemy.String, + nullable=False, + ), + sqlalchemy.Column( + "created_at", + UTCDateTime, + nullable=False, + ), + sqlalchemy.Column( + "source", + sqlalchemy.Enum( + DataFileSourceSQLEnum, + values_callable=lambda obj: [e.value for e in obj], + validate_strings=True, + create_constraint=True, + ), + index=True, + nullable=False, + ), +) + +run_csv_rtp_table = sqlalchemy.Table( + "run_csv_rtp_table", + metadata, + sqlalchemy.Column( + "row_id", + sqlalchemy.Integer, + primary_key=True, + ), + sqlalchemy.Column( + "run_id", + sqlalchemy.ForeignKey("run.id"), + nullable=False, + ), + sqlalchemy.Column( + "parameter_variable_name", + sqlalchemy.String, + nullable=False, + ), + sqlalchemy.Column( + "file_id", + sqlalchemy.ForeignKey("data_files.id"), + nullable=True, + ), +) + + +class BooleanSettingKey(enum.Enum): + """Keys for boolean settings.""" + + ENABLE_ERROR_RECOVERY = "enable_error_recovery" + + +boolean_setting_table = sqlalchemy.Table( + "boolean_setting", + metadata, + sqlalchemy.Column( + "key", + sqlalchemy.Enum( + BooleanSettingKey, + values_callable=lambda obj: [e.value for e in obj], + validate_strings=True, + create_constraint=True, + ), + primary_key=True, + ), + sqlalchemy.Column( + "value", + sqlalchemy.Boolean, + nullable=False, + ), +) diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index d30f5c33979..fb5c78425fe 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -207,6 +207,7 @@ async def create( run_id=prev_run_id, summary=prev_run_result.state_summary, commands=prev_run_result.commands, + command_errors=prev_run_result.command_errors, run_time_parameters=prev_run_result.parameters, ) @@ -367,6 +368,7 @@ async def update(self, run_id: str, current: Optional[bool]) -> Union[Run, BadRu if next_current is False: ( commands, + command_errors, state_summary, parameters, ) = await self._run_orchestrator_store.clear() @@ -376,6 +378,7 @@ async def update(self, run_id: str, current: Optional[bool]) -> Union[Run, BadRu run_id=run_id, summary=state_summary, commands=commands, + command_errors=command_errors, run_time_parameters=parameters, ) self._runs_publisher.publish_pre_serialized_commands_notification(run_id) diff --git a/robot-server/robot_server/runs/run_orchestrator_store.py b/robot-server/robot_server/runs/run_orchestrator_store.py index ee82ea034ac..d8c0b1ca1bf 100644 --- a/robot-server/robot_server/runs/run_orchestrator_store.py +++ b/robot-server/robot_server/runs/run_orchestrator_store.py @@ -287,12 +287,16 @@ async def clear(self) -> RunResult: run_data = self.run_orchestrator.get_state_summary() commands = self.run_orchestrator.get_all_commands() + command_errors = self.run_orchestrator.get_command_errors() run_time_parameters = self.run_orchestrator.get_run_time_parameters() self._run_orchestrator = None return RunResult( - state_summary=run_data, commands=commands, parameters=run_time_parameters + state_summary=run_data, + commands=commands, + command_errors=command_errors, + parameters=run_time_parameters, ) def play(self, deck_configuration: Optional[DeckConfigurationType] = None) -> None: diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 6ab8665c454..aa777222788 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -11,7 +11,12 @@ from pydantic import ValidationError from opentrons.util.helpers import utc_now -from opentrons.protocol_engine import StateSummary, CommandSlice, CommandIntent +from opentrons.protocol_engine import ( + StateSummary, + CommandSlice, + CommandIntent, + ErrorOccurrence, +) from opentrons.protocol_engine.commands import Command from opentrons.protocol_engine.types import RunTimeParameter @@ -119,6 +124,7 @@ def update_run_state( run_id: str, summary: StateSummary, commands: List[Command], + command_errors: List[ErrorOccurrence], run_time_parameters: List[RunTimeParameter], ) -> RunResource: """Update the run's state summary and commands list. @@ -127,6 +133,7 @@ def update_run_state( run_id: The run to update summary: The run's equipment and status summary. commands: The run's commands. + command_errors: The run's commands errors. run_time_parameters: The run's run time parameters, if any. Returns: From 1bb8d7cc0109f236464dca7ec779b946c059fea7 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 1 Nov 2024 16:40:28 -0400 Subject: [PATCH 02/53] changed schema and imports to 8 --- robot-server/robot_server/persistence/tables/__init__.py | 4 +++- robot-server/robot_server/persistence/tables/schema_8.py | 2 +- robot-server/robot_server/runs/run_store.py | 3 +++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/tables/__init__.py b/robot-server/robot_server/persistence/tables/__init__.py index fa0129a4ee6..fead828e04d 100644 --- a/robot-server/robot_server/persistence/tables/__init__.py +++ b/robot-server/robot_server/persistence/tables/__init__.py @@ -1,7 +1,7 @@ """SQL database schemas.""" # Re-export the latest schema. -from .schema_7 import ( +from .schema_8 import ( metadata, protocol_table, analysis_table, @@ -9,6 +9,7 @@ analysis_csv_rtp_table, run_table, run_command_table, + run_command_errors_table, action_table, run_csv_rtp_table, data_files_table, @@ -28,6 +29,7 @@ "analysis_csv_rtp_table", "run_table", "run_command_table", + "run_command_errors_table", "action_table", "run_csv_rtp_table", "data_files_table", diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 5d306a69d2e..9403e7bcb03 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -222,7 +222,7 @@ class DataFileSourceSQLEnum(enum.Enum): ), ) -run_command_table = sqlalchemy.Table( +run_command_errors_table = sqlalchemy.Table( "run_command_errors", metadata, sqlalchemy.Column("row_id", sqlalchemy.Integer, primary_key=True), diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index aa777222788..f87665db2cd 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -30,6 +30,7 @@ from robot_server.persistence.tables import ( run_table, run_command_table, + run_command_errors_table, action_table, run_csv_rtp_table, ) @@ -160,6 +161,8 @@ def update_run_state( ) insert_command = sqlalchemy.insert(run_command_table) + insert_command_errors = sqlalchemy.insert(run_command_errors_table) + select_run_resource = sqlalchemy.select(*_run_columns).where( run_table.c.id == run_id ) From 77902c691c855bee092b7f36951bcd308c83f76c Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Mon, 4 Nov 2024 13:05:16 -0500 Subject: [PATCH 03/53] insert the errors to the DB --- .../robot_server/runs/run_data_manager.py | 15 +++++---------- robot-server/robot_server/runs/run_store.py | 14 +++++++++++--- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index fb5c78425fe..b39ccbcc79d 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -366,20 +366,15 @@ async def update(self, run_id: str, current: Optional[bool]) -> Union[Run, BadRu next_current = current if current is False else True if next_current is False: - ( - commands, - command_errors, - state_summary, - parameters, - ) = await self._run_orchestrator_store.clear() + run_result = await self._run_orchestrator_store.clear() run_resource: Union[ RunResource, BadRunResource ] = self._run_store.update_run_state( run_id=run_id, - summary=state_summary, - commands=commands, - command_errors=command_errors, - run_time_parameters=parameters, + summary=run_result.state_summary, + commands=run_result.commands, + command_errors=run_result.command_errors, + run_time_parameters=run_result.parameters, ) self._runs_publisher.publish_pre_serialized_commands_notification(run_id) else: diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index f87665db2cd..14922df4a41 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -125,8 +125,8 @@ def update_run_state( run_id: str, summary: StateSummary, commands: List[Command], - command_errors: List[ErrorOccurrence], run_time_parameters: List[RunTimeParameter], + command_errors: Optional[List[ErrorOccurrence]] = None, ) -> RunResource: """Update the run's state summary and commands list. @@ -161,8 +161,6 @@ def update_run_state( ) insert_command = sqlalchemy.insert(run_command_table) - insert_command_errors = sqlalchemy.insert(run_command_errors_table) - select_run_resource = sqlalchemy.select(*_run_columns).where( run_table.c.id == run_id ) @@ -191,6 +189,16 @@ def update_run_state( else CommandIntent.PROTOCOL, }, ) + if command_errors: + insert_command_errors = sqlalchemy.insert(run_command_errors_table) + transaction.execute( + insert_command_errors, + { + "run_id": run_id, + "index_in_run": command_index, + "command_error": pydantic_list_to_json(command_errors), + }, + ) run_row = transaction.execute(select_run_resource).one() action_rows = transaction.execute(select_actions).all() From 15db24619a669bae46a42fb81189209de3d02db8 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Mon, 4 Nov 2024 15:00:45 -0500 Subject: [PATCH 04/53] refactor into commands table --- .../protocol_runner/protocol_runner.py | 4 +- .../persistence/tables/__init__.py | 2 - .../persistence/tables/schema_8.py | 18 +---- .../robot_server/runs/run_data_manager.py | 4 +- .../runs/run_orchestrator_store.py | 1 - robot-server/robot_server/runs/run_store.py | 75 ++++++++++++++++--- 6 files changed, 66 insertions(+), 38 deletions(-) diff --git a/api/src/opentrons/protocol_runner/protocol_runner.py b/api/src/opentrons/protocol_runner/protocol_runner.py index 20e4d8ec005..8ca1252ae9a 100644 --- a/api/src/opentrons/protocol_runner/protocol_runner.py +++ b/api/src/opentrons/protocol_runner/protocol_runner.py @@ -6,7 +6,6 @@ import anyio -from api.src.opentrons.protocol_engine.errors.error_occurrence import ErrorOccurrence from opentrons.hardware_control import HardwareControlAPI from opentrons import protocol_reader from opentrons.legacy_broker import LegacyBroker @@ -24,6 +23,7 @@ StateSummary, Command, commands as pe_commands, + ErrorOccurrence ) from opentrons.protocols.parse import PythonParseMode from opentrons.util.async_helpers import asyncio_yield @@ -57,8 +57,6 @@ class RunResult(NamedTuple): commands: List[Command] state_summary: StateSummary parameters: List[RunTimeParameter] - command_errors: Optional[List[ErrorOccurrence]] = None - class AbstractRunner(ABC): """An interface to manage and control a protocol run. diff --git a/robot-server/robot_server/persistence/tables/__init__.py b/robot-server/robot_server/persistence/tables/__init__.py index fead828e04d..390b381bfed 100644 --- a/robot-server/robot_server/persistence/tables/__init__.py +++ b/robot-server/robot_server/persistence/tables/__init__.py @@ -9,7 +9,6 @@ analysis_csv_rtp_table, run_table, run_command_table, - run_command_errors_table, action_table, run_csv_rtp_table, data_files_table, @@ -29,7 +28,6 @@ "analysis_csv_rtp_table", "run_table", "run_command_table", - "run_command_errors_table", "action_table", "run_csv_rtp_table", "data_files_table", diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 9403e7bcb03..c13d83eb45d 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -208,6 +208,7 @@ class DataFileSourceSQLEnum(enum.Enum): sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), sqlalchemy.Column("command", sqlalchemy.String, nullable=False), sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=False, index=True), + sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True, index=True), sqlalchemy.Index( "ix_run_run_id_command_id", # An arbitrary name for the index. "run_id", @@ -222,23 +223,6 @@ class DataFileSourceSQLEnum(enum.Enum): ), ) -run_command_errors_table = sqlalchemy.Table( - "run_command_errors", - metadata, - sqlalchemy.Column("row_id", sqlalchemy.Integer, primary_key=True), - sqlalchemy.Column( - "run_id", sqlalchemy.String, sqlalchemy.ForeignKey("run.id"), nullable=False - ), - sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), - sqlalchemy.Column("command_error", sqlalchemy.String, nullable=False), - sqlalchemy.Index( - "ix_run_run_id_index_in_run", # An arbitrary name for the index. - "run_id", - "index_in_run", - unique=True, - ), -) - data_files_table = sqlalchemy.Table( "data_files", metadata, diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index b39ccbcc79d..94e441f9d95 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -207,7 +207,6 @@ async def create( run_id=prev_run_id, summary=prev_run_result.state_summary, commands=prev_run_result.commands, - command_errors=prev_run_result.command_errors, run_time_parameters=prev_run_result.parameters, ) @@ -373,7 +372,6 @@ async def update(self, run_id: str, current: Optional[bool]) -> Union[Run, BadRu run_id=run_id, summary=run_result.state_summary, commands=run_result.commands, - command_errors=run_result.command_errors, run_time_parameters=run_result.parameters, ) self._runs_publisher.publish_pre_serialized_commands_notification(run_id) @@ -424,7 +422,7 @@ def get_commands_slice( def get_command_error_slice( self, run_id: str, cursor: int, length: int ) -> CommandErrorSlice: - """Get a slice of run commands. + """Get a slice of run commands errors. Args: run_id: ID of the run. diff --git a/robot-server/robot_server/runs/run_orchestrator_store.py b/robot-server/robot_server/runs/run_orchestrator_store.py index d8c0b1ca1bf..9ea82aefe5d 100644 --- a/robot-server/robot_server/runs/run_orchestrator_store.py +++ b/robot-server/robot_server/runs/run_orchestrator_store.py @@ -295,7 +295,6 @@ async def clear(self) -> RunResult: return RunResult( state_summary=run_data, commands=commands, - command_errors=command_errors, parameters=run_time_parameters, ) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 14922df4a41..22518deca30 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -16,6 +16,7 @@ CommandSlice, CommandIntent, ErrorOccurrence, + CommandErrorSlice, ) from opentrons.protocol_engine.commands import Command from opentrons.protocol_engine.types import RunTimeParameter @@ -30,7 +31,6 @@ from robot_server.persistence.tables import ( run_table, run_command_table, - run_command_errors_table, action_table, run_csv_rtp_table, ) @@ -126,7 +126,6 @@ def update_run_state( summary: StateSummary, commands: List[Command], run_time_parameters: List[RunTimeParameter], - command_errors: Optional[List[ErrorOccurrence]] = None, ) -> RunResource: """Update the run's state summary and commands list. @@ -187,18 +186,11 @@ def update_run_state( "command_intent": str(command.intent.value) if command.intent else CommandIntent.PROTOCOL, + "command_error": pydantic_to_json(command.error) + if command.error + else None, }, ) - if command_errors: - insert_command_errors = sqlalchemy.insert(run_command_errors_table) - transaction.execute( - insert_command_errors, - { - "run_id": run_id, - "index_in_run": command_index, - "command_error": pydantic_list_to_json(command_errors), - }, - ) run_row = transaction.execute(select_run_resource).one() action_rows = transaction.execute(select_actions).all() @@ -555,6 +547,65 @@ def get_all_commands_as_preserialized_list( commands_result = transaction.scalars(select_commands).all() return commands_result + def get_commands_errors_slice( + self, + run_id: str, + length: int, + cursor: Optional[int], + ) -> CommandErrorSlice: + """Get a slice of run commands errors from the store. + + Args: + run_id: Run ID to pull commands from. + length: Number of commands to return. + cursor: The starting index of the slice in the whole collection. + If `None`, up to `length` elements at the end of the collection will + be returned. + + Returns: + A collection of command errors as well as the actual cursor used and + the total length of the collection. + + Raises: + RunNotFoundError: The given run ID was not found. + """ + with self._sql_engine.begin() as transaction: + if not self._run_exists(run_id, transaction): + raise RunNotFoundError(run_id=run_id) + + select_count = sqlalchemy.select(sqlalchemy.func.count()).where( + run_command_errors_table.c.run_id == run_id + ) + count_result: int = transaction.execute(select_count).scalar_one() + + actual_cursor = cursor if cursor is not None else count_result - length + # Clamp to [0, count_result). + actual_cursor = max(0, min(actual_cursor, count_result - 1)) + select_slice = ( + sqlalchemy.select( + run_command_errors_table.c.index_in_run, + run_command_errors_table.c.command, + ) + .where( + run_command_errors_table.c.run_id == run_id, + run_command_errors_table.c.index_in_run >= actual_cursor, + run_command_errors_table.c.index_in_run < actual_cursor + length, + ) + .order_by(run_command_errors_table.c.index_in_run) + ) + slice_result = transaction.execute(select_slice).all() + + sliced_commands: List[Command] = [ + json_to_pydantic(Command, row.command) # type: ignore[arg-type] + for row in slice_result + ] + + return CommandErrorSlice( + cursor=actual_cursor, + total_length=count_result, + commands_errors=sliced_commands, + ) + @lru_cache(maxsize=_CACHE_ENTRIES) def get_command(self, run_id: str, command_id: str) -> Command: """Get run command by id. From b88e722810b2a1db3fdb39347907be94aea8285b Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Mon, 4 Nov 2024 15:59:29 -0500 Subject: [PATCH 05/53] select from commands --- robot-server/robot_server/runs/run_store.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 22518deca30..f360fa04ae3 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -574,7 +574,10 @@ def get_commands_errors_slice( raise RunNotFoundError(run_id=run_id) select_count = sqlalchemy.select(sqlalchemy.func.count()).where( - run_command_errors_table.c.run_id == run_id + and_( + run_command_table.c.run_id == run_id, + run_command_table.c.command_error is not None, + ) ) count_result: int = transaction.execute(select_count).scalar_one() @@ -583,15 +586,18 @@ def get_commands_errors_slice( actual_cursor = max(0, min(actual_cursor, count_result - 1)) select_slice = ( sqlalchemy.select( - run_command_errors_table.c.index_in_run, - run_command_errors_table.c.command, + run_command_table.c.index_in_run, + run_command_table.c.command_error, ) .where( - run_command_errors_table.c.run_id == run_id, - run_command_errors_table.c.index_in_run >= actual_cursor, - run_command_errors_table.c.index_in_run < actual_cursor + length, + and_( + run_command_table.c.run_id == run_id, + run_command_table.c.index_in_run >= actual_cursor, + run_command_table.c.index_in_run < actual_cursor + length, + run_command_table.c.command_error is not None, + ) ) - .order_by(run_command_errors_table.c.index_in_run) + .order_by(run_command_table.c.index_in_run) ) slice_result = transaction.execute(select_slice).all() From b6d33b6c5e347c193e008e85e3eaa9c4d8a45e58 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 5 Nov 2024 10:15:57 -0500 Subject: [PATCH 06/53] linting and command error slice done --- robot-server/robot_server/runs/run_orchestrator_store.py | 1 - robot-server/robot_server/runs/run_store.py | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/robot-server/robot_server/runs/run_orchestrator_store.py b/robot-server/robot_server/runs/run_orchestrator_store.py index 9ea82aefe5d..ac23d1e1c48 100644 --- a/robot-server/robot_server/runs/run_orchestrator_store.py +++ b/robot-server/robot_server/runs/run_orchestrator_store.py @@ -287,7 +287,6 @@ async def clear(self) -> RunResult: run_data = self.run_orchestrator.get_state_summary() commands = self.run_orchestrator.get_all_commands() - command_errors = self.run_orchestrator.get_command_errors() run_time_parameters = self.run_orchestrator.get_run_time_parameters() self._run_orchestrator = None diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index f360fa04ae3..e4432742952 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -601,9 +601,8 @@ def get_commands_errors_slice( ) slice_result = transaction.execute(select_slice).all() - sliced_commands: List[Command] = [ - json_to_pydantic(Command, row.command) # type: ignore[arg-type] - for row in slice_result + sliced_commands: List[ErrorOccurrence] = [ + json_to_pydantic(ErrorOccurrence, row.command) for row in slice_result ] return CommandErrorSlice( From 85eda17369388bddb9abbcd8b504970fc6cfa648 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 5 Nov 2024 14:04:10 -0500 Subject: [PATCH 07/53] added tests and fixed bugs --- robot-server/robot_server/runs/run_store.py | 2 +- robot-server/tests/runs/test_run_store.py | 77 +++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index e4432742952..fad6a31d8b1 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -602,7 +602,7 @@ def get_commands_errors_slice( slice_result = transaction.execute(select_slice).all() sliced_commands: List[ErrorOccurrence] = [ - json_to_pydantic(ErrorOccurrence, row.command) for row in slice_result + json_to_pydantic(ErrorOccurrence, row.command_error) for row in slice_result ] return CommandErrorSlice( diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 17a5c3b252f..e2850f1ce47 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -36,6 +36,7 @@ CommandSlice, Liquid, EngineStatus, + ErrorOccurrence, ) from opentrons.types import MountType, DeckSlotName @@ -99,6 +100,43 @@ def protocol_commands() -> List[pe_commands.Command]: ] +@pytest.fixture +def protocol_commands_errors() -> List[pe_commands.Command]: + """Get a StateSummary value object.""" + return [ + pe_commands.WaitForResume( + id="pause-1", + key="command-key", + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=pe_commands.WaitForResumeParams(message="hello world"), + result=pe_commands.WaitForResumeResult(), + intent=pe_commands.CommandIntent.PROTOCOL, + error=ErrorOccurrence.construct( + id="error-id", + createdAt=datetime(2024, 1, 1), + errorType="blah-blah", + detail="test details", + ), + ), + pe_commands.WaitForResume( + id="pause-2", + key="command-key", + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2022, month=2, day=2), + params=pe_commands.WaitForResumeParams(message="hey world"), + result=pe_commands.WaitForResumeResult(), + intent=pe_commands.CommandIntent.PROTOCOL, + error=ErrorOccurrence.construct( + id="error-id-2", + createdAt=datetime(2024, 1, 1), + errorType="blah-blah", + detail="test details", + ), + ), + ] + + @pytest.fixture def state_summary() -> StateSummary: """Get a StateSummary test object.""" @@ -289,6 +327,45 @@ async def test_update_run_state( ) +async def test_update_run_state_command_with_errors( + subject: RunStore, + state_summary: StateSummary, + protocol_commands_errors: List[pe_commands.Command], + run_time_parameters: List[pe_types.RunTimeParameter], + mock_runs_publisher: mock.Mock, +) -> None: + """It should be able to update a run state to the store.""" + action = RunAction( + actionType=RunActionType.PLAY, + createdAt=datetime(year=2022, month=2, day=2, tzinfo=timezone.utc), + id="action-id", + ) + + subject.insert( + run_id="run-id", + protocol_id=None, + created_at=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc), + ) + + subject.update_run_state( + run_id="run-id", + summary=state_summary, + commands=protocol_commands_errors, + run_time_parameters=run_time_parameters, + ) + + subject.insert_action(run_id="run-id", action=action) + command_errors_result = subject.get_commands_errors_slice( + run_id="run-id", + length=len(protocol_commands_errors), + cursor=0, + ) + + assert command_errors_result.commands_errors == [ + item.error for item in protocol_commands_errors + ] + + async def test_insert_and_get_csv_rtp( subject: RunStore, data_files_store: DataFilesStore, From 5bc0500bd1ccdf3c1f744de7b23d0e0c08e4311e Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 5 Nov 2024 15:26:24 -0500 Subject: [PATCH 08/53] changed logic for current command errors --- api/src/opentrons/protocol_engine/state/commands.py | 7 ++++++- api/src/opentrons/protocol_runner/protocol_runner.py | 2 +- .../protocol_engine/state/test_command_view_old.py | 9 ++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 4d2009aae80..988d3606838 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -706,7 +706,12 @@ def get_error(self) -> Optional[ErrorOccurrence]: def get_all_errors(self) -> List[ErrorOccurrence]: """Get the run's full error list, if there was none, returns an empty list.""" - return self._state.failed_command_errors + command_errors = self._state.command_history.get_all_commands() + return [ + command_error.error + for command_error in command_errors + if command_error.error is not None + ] def get_has_entered_recovery_mode(self) -> bool: """Get whether the run has entered recovery mode.""" diff --git a/api/src/opentrons/protocol_runner/protocol_runner.py b/api/src/opentrons/protocol_runner/protocol_runner.py index 8ca1252ae9a..aec2aae80df 100644 --- a/api/src/opentrons/protocol_runner/protocol_runner.py +++ b/api/src/opentrons/protocol_runner/protocol_runner.py @@ -23,7 +23,6 @@ StateSummary, Command, commands as pe_commands, - ErrorOccurrence ) from opentrons.protocols.parse import PythonParseMode from opentrons.util.async_helpers import asyncio_yield @@ -58,6 +57,7 @@ class RunResult(NamedTuple): state_summary: StateSummary parameters: List[RunTimeParameter] + class AbstractRunner(ABC): """An interface to manage and control a protocol run. diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index f7b1d6cd31f..71711caca99 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -1046,8 +1046,15 @@ def test_get_errors_slice() -> None: error_3 = ErrorOccurrence.construct(id="error-id-3") # type: ignore[call-arg] error_4 = ErrorOccurrence.construct(id="error-id-4") # type: ignore[call-arg] + command_1 = create_failed_command(command_id="command-id-1", error=error_1) + command_2 = create_failed_command(command_id="command-id-2", error=error_2) + command_3 = create_failed_command(command_id="command-id-3", error=error_3) + command_4 = create_failed_command(command_id="command-id-4", error=error_4) + command_5 = create_running_command(command_id="command-id-5") + command_6 = create_queued_command(command_id="command-id-6") + subject = get_command_view( - failed_command_errors=[error_1, error_2, error_3, error_4] + commands=[command_1, command_2, command_3, command_4, command_5, command_6] ) result = subject.get_errors_slice(cursor=1, length=3) From e423f37575bf1e5b6548cd0cebb5e341836113d2 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 5 Nov 2024 15:52:05 -0500 Subject: [PATCH 09/53] removed failed_command_errors from state --- api/src/opentrons/protocol_engine/state/commands.py | 5 ----- .../protocol_engine/state/test_command_state.py | 8 ++++---- .../protocol_engine/state/test_command_store_old.py | 11 ----------- .../protocol_engine/state/test_command_view_old.py | 4 +--- 4 files changed, 5 insertions(+), 23 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 988d3606838..2f93024ab74 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -228,9 +228,6 @@ class CommandState: This value can be used to generate future hashes. """ - failed_command_errors: List[ErrorOccurrence] - """List of command errors that occurred during run execution.""" - has_entered_error_recovery: bool """Whether the run has entered error recovery.""" @@ -269,7 +266,6 @@ def __init__( run_started_at=None, latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=error_recovery_policy, has_entered_error_recovery=False, ) @@ -366,7 +362,6 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None: notes=action.notes, ) self._state.failed_command = self._state.command_history.get(action.command_id) - self._state.failed_command_errors.append(public_error_occurrence) if ( prev_entry.command.intent in (CommandIntent.PROTOCOL, None) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_state.py b/api/tests/opentrons/protocol_engine/state/test_command_state.py index fde0d66e654..76fa39b14e4 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_state.py @@ -193,7 +193,7 @@ def test_command_failure(error_recovery_type: ErrorRecoveryType) -> None: ) assert subject_view.get("command-id") == expected_failed_command - assert subject.state.failed_command_errors == [expected_error_occurrence] + assert subject_view.get_all_errors() == [expected_error_occurrence] def test_command_failure_clears_queues() -> None: @@ -255,7 +255,7 @@ def test_command_failure_clears_queues() -> None: assert subject_view.get_running_command_id() is None assert subject_view.get_queue_ids() == OrderedSet() assert subject_view.get_next_to_execute() is None - assert subject.state.failed_command_errors == [expected_error_occurance] + assert subject_view.get_all_errors() == [expected_error_occurance] def test_setup_command_failure_only_clears_setup_command_queue() -> None: @@ -555,7 +555,7 @@ def test_door_during_error_recovery() -> None: subject.handle_action(play) assert subject_view.get_status() == EngineStatus.AWAITING_RECOVERY assert subject_view.get_next_to_execute() == "command-id-2" - assert subject.state.failed_command_errors == [expected_error_occurance] + assert subject_view.get_all_errors() == [expected_error_occurance] @pytest.mark.parametrize("close_door_before_queueing", [False, True]) @@ -732,7 +732,7 @@ def test_error_recovery_type_tracking() -> None: id="c2-error", createdAt=datetime(year=2023, month=3, day=3), error=exception ) - assert subject.state.failed_command_errors == [ + assert view.get_all_errors() == [ error_occurrence_1, error_occurrence_2, ] diff --git a/api/tests/opentrons/protocol_engine/state/test_command_store_old.py b/api/tests/opentrons/protocol_engine/state/test_command_store_old.py index d5f171b7ea9..881719ba7ad 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_store_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_store_old.py @@ -333,7 +333,6 @@ def test_command_store_handles_pause_action(pause_source: PauseSource) -> None: recovery_target=None, latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -363,7 +362,6 @@ def test_command_store_handles_play_action(pause_source: PauseSource) -> None: run_started_at=datetime(year=2021, month=1, day=1), latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -398,7 +396,6 @@ def test_command_store_handles_finish_action() -> None: run_started_at=datetime(year=2021, month=1, day=1), latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -453,7 +450,6 @@ def test_command_store_handles_stop_action( run_started_at=datetime(year=2021, month=1, day=1), latest_protocol_command_hash=None, stopped_by_estop=from_estop, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -491,7 +487,6 @@ def test_command_store_handles_stop_action_when_awaiting_recovery() -> None: run_started_at=datetime(year=2021, month=1, day=1), latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -525,7 +520,6 @@ def test_command_store_cannot_restart_after_should_stop() -> None: run_started_at=None, latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -672,7 +666,6 @@ def test_command_store_wraps_unknown_errors() -> None: recovery_target=None, latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -742,7 +735,6 @@ def __init__(self, message: str) -> None: run_started_at=None, latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -778,7 +770,6 @@ def test_command_store_ignores_stop_after_graceful_finish() -> None: run_started_at=datetime(year=2021, month=1, day=1), latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -814,7 +805,6 @@ def test_command_store_ignores_finish_after_non_graceful_stop() -> None: run_started_at=datetime(year=2021, month=1, day=1), latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) @@ -850,7 +840,6 @@ def test_handles_hardware_stopped() -> None: run_started_at=None, latest_protocol_command_hash=None, stopped_by_estop=False, - failed_command_errors=[], error_recovery_policy=matchers.Anything(), has_entered_error_recovery=False, ) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index 71711caca99..a5759f8ba62 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -77,7 +77,6 @@ def get_command_view( # noqa: C901 finish_error: Optional[errors.ErrorOccurrence] = None, commands: Sequence[cmd.Command] = (), latest_command_hash: Optional[str] = None, - failed_command_errors: Optional[List[ErrorOccurrence]] = None, has_entered_error_recovery: bool = False, ) -> CommandView: """Get a command view test subject.""" @@ -121,7 +120,6 @@ def get_command_view( # noqa: C901 run_started_at=run_started_at, latest_protocol_command_hash=latest_command_hash, stopped_by_estop=False, - failed_command_errors=failed_command_errors or [], has_entered_error_recovery=has_entered_error_recovery, error_recovery_policy=_placeholder_error_recovery_policy, ) @@ -1033,7 +1031,7 @@ def test_get_slice_default_cursor_running() -> None: def test_get_errors_slice_empty() -> None: """It should return a slice from the tail if no current command.""" - subject = get_command_view(failed_command_errors=[]) + subject = get_command_view() result = subject.get_errors_slice(cursor=0, length=2) assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0) From e29d95f1aa6cc56aa5be0879f78e78c43e5b2aca Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 5 Nov 2024 16:52:10 -0500 Subject: [PATCH 10/53] refactor get command errors --- .../protocol_engine/state/command_history.py | 14 ++++++++++++++ .../opentrons/protocol_engine/state/commands.py | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/command_history.py b/api/src/opentrons/protocol_engine/state/command_history.py index d555764e54e..cac7f04089a 100644 --- a/api/src/opentrons/protocol_engine/state/command_history.py +++ b/api/src/opentrons/protocol_engine/state/command_history.py @@ -24,6 +24,9 @@ class CommandHistory: _all_command_ids: List[str] """All command IDs, in insertion order.""" + _all_failed_command_ids: List[str] + """All failed command IDs, in insertion order.""" + _all_command_ids_but_fixit_command_ids: List[str] """All command IDs besides fixit command intents, in insertion order.""" @@ -101,6 +104,13 @@ def get_all_commands(self) -> List[Command]: for command_id in self._all_command_ids ] + def get_all_failed_commands(self) -> List[Command]: + """Get all failed commands.""" + return [ + self._commands_by_id[command_id].command + for command_id in self._all_failed_command_ids + ] + def get_filtered_command_ids(self, include_fixit_commands: bool) -> List[str]: """Get all fixit command IDs.""" if include_fixit_commands: @@ -251,6 +261,10 @@ def _add(self, command_id: str, command_entry: CommandEntry) -> None: self._all_command_ids_but_fixit_command_ids.append(command_id) self._commands_by_id[command_id] = command_entry + def append_failed_command_id(self, command_id: str) -> None: + """Create or update a failed command id.""" + self._all_failed_command_ids.append(command_id) + def _add_to_queue(self, command_id: str) -> None: """Add new ID to the queued.""" self._queued_command_ids.add(command_id) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 2f93024ab74..8cdba6aefec 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -362,6 +362,7 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None: notes=action.notes, ) self._state.failed_command = self._state.command_history.get(action.command_id) + self._state.command_history.append_failed_command_id(action.command_id) if ( prev_entry.command.intent in (CommandIntent.PROTOCOL, None) @@ -701,10 +702,10 @@ def get_error(self) -> Optional[ErrorOccurrence]: def get_all_errors(self) -> List[ErrorOccurrence]: """Get the run's full error list, if there was none, returns an empty list.""" - command_errors = self._state.command_history.get_all_commands() + failed_commands = self._state.command_history.get_all_failed_commands() return [ command_error.error - for command_error in command_errors + for command_error in failed_commands if command_error.error is not None ] From a950cb857594f1fb3ec814020940abf36046583a Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 6 Nov 2024 11:49:33 -0500 Subject: [PATCH 11/53] migration and initialization fixes --- .../protocol_engine/state/command_history.py | 1 + .../state/test_command_view_old.py | 5 +- .../persistence/_migrations/v7_to_v8.py | 86 +++++++++++++++++++ .../persistence/file_and_directory_names.py | 2 +- .../persistence/persistence_directory.py | 5 +- .../robot_server/runs/run_data_manager.py | 2 + robot-server/robot_server/runs/run_store.py | 1 + 7 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 robot-server/robot_server/persistence/_migrations/v7_to_v8.py diff --git a/api/src/opentrons/protocol_engine/state/command_history.py b/api/src/opentrons/protocol_engine/state/command_history.py index cac7f04089a..34dd2c88f2a 100644 --- a/api/src/opentrons/protocol_engine/state/command_history.py +++ b/api/src/opentrons/protocol_engine/state/command_history.py @@ -50,6 +50,7 @@ class CommandHistory: def __init__(self) -> None: self._all_command_ids = [] + self._all_failed_command_ids = [] self._all_command_ids_but_fixit_command_ids = [] self._queued_command_ids = OrderedSet() self._queued_setup_command_ids = OrderedSet() diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index a5759f8ba62..27aecc545d0 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -34,13 +34,12 @@ QueueStatus, ) -from opentrons.protocol_engine.state.command_history import CommandEntry +from opentrons.protocol_engine.state.command_history import CommandEntry, CommandHistory from opentrons.protocol_engine.errors import ProtocolCommandFailedError, ErrorOccurrence from opentrons_shared_data.errors.codes import ErrorCodes -from opentrons.protocol_engine.state.command_history import CommandHistory from opentrons.protocol_engine.state.update_types import StateUpdate from .command_fixtures import ( @@ -100,6 +99,8 @@ def get_command_view( # noqa: C901 command_id=command.id, command_entry=CommandEntry(index=index, command=command), ) + if command.error: + command_history.append_failed_command_id(command.id) state = CommandState( command_history=command_history, diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py new file mode 100644 index 00000000000..fd791c10f92 --- /dev/null +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -0,0 +1,86 @@ +"""Migrate the persistence directory from schema 7 to 8. + +Summary of changes from schema 7: + +- Adds a new command_intent to store the commands intent in the commands table +- Adds a new source to store the data files origin in the data_files table +- Adds the `boolean_setting` table. +""" + +import json +from pathlib import Path +from contextlib import ExitStack +import shutil +from typing import Any + +import sqlalchemy + +from ..database import sql_engine_ctx, sqlite_rowid +from ..tables import schema_8 +from .._folder_migrator import Migration + +from ..file_and_directory_names import ( + DB_FILE, +) + + +class Migration7to8(Migration): # noqa: D101 + def migrate(self, source_dir: Path, dest_dir: Path) -> None: + """Migrate the persistence directory from schema 6 to 7.""" + # 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 + + # Append the new column to existing protocols and data_files in v6 database + with ExitStack() as exit_stack: + dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file)) + + schema_8.metadata.create_all(dest_engine) + + dest_transaction = exit_stack.enter_context(dest_engine.begin()) + + 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_8.run_command_table.name, + schema_8.run_command_table.c.command_error, + ) + + _migrate_command_table_with_new_command_error_col( + dest_transaction=dest_transaction + ) + + +def _migrate_command_table_with_new_command_error_col( + dest_transaction: sqlalchemy.engine.Connection, +) -> None: + """Add a new 'command_intent' column to run_command_table table.""" + select_commands = sqlalchemy.select(schema_8.run_command_table).order_by( + sqlite_rowid + ) + for row in dest_transaction.execute(select_commands).all(): + data = json.loads(row.command) + new_command_error = ( + # Account for old_row.command["error"] being NULL. + None + if "error" not in row.command or data["error"] == None # noqa: E711 + else data["error"] + ) + + dest_transaction.execute( + f"UPDATE run_command SET command_error='{new_command_error}' WHERE row_id={row.row_id}" + ) diff --git a/robot-server/robot_server/persistence/file_and_directory_names.py b/robot-server/robot_server/persistence/file_and_directory_names.py index 7074dd6db2f..2064b3f59fd 100644 --- a/robot-server/robot_server/persistence/file_and_directory_names.py +++ b/robot-server/robot_server/persistence/file_and_directory_names.py @@ -8,7 +8,7 @@ from typing import Final -LATEST_VERSION_DIRECTORY: Final = "7.1" +LATEST_VERSION_DIRECTORY: Final = "8.1" DECK_CONFIGURATION_FILE: Final = "deck_configuration.json" PROTOCOLS_DIRECTORY: Final = "protocols" diff --git a/robot-server/robot_server/persistence/persistence_directory.py b/robot-server/robot_server/persistence/persistence_directory.py index 2dd46711697..19e67fe8333 100644 --- a/robot-server/robot_server/persistence/persistence_directory.py +++ b/robot-server/robot_server/persistence/persistence_directory.py @@ -11,7 +11,7 @@ from anyio import Path as AsyncPath, to_thread from ._folder_migrator import MigrationOrchestrator -from ._migrations import up_to_3, v3_to_v4, v4_to_v5, v5_to_v6, v6_to_v7 +from ._migrations import up_to_3, v3_to_v4, v4_to_v5, v5_to_v6, v6_to_v7, v7_to_v8 from .file_and_directory_names import LATEST_VERSION_DIRECTORY _TEMP_PERSISTENCE_DIR_PREFIX: Final = "opentrons-robot-server-" @@ -55,7 +55,8 @@ async def prepare_active_subdirectory(prepared_root: Path) -> Path: # Subdirectory "7" was previously used on our edge branch for an in-dev # schema that was never released to the public. It may be present on # internal robots. - v6_to_v7.Migration6to7(subdirectory=LATEST_VERSION_DIRECTORY), + v6_to_v7.Migration6to7(subdirectory="7"), + v7_to_v8.Migration7to8(subdirectory=LATEST_VERSION_DIRECTORY), ], temp_file_prefix="temp-", ) diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index 94e441f9d95..fd33f72734e 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -366,6 +366,8 @@ async def update(self, run_id: str, current: Optional[bool]) -> Union[Run, BadRu if next_current is False: run_result = await self._run_orchestrator_store.clear() + state_summary = run_result.state_summary + parameters = run_result.parameters run_resource: Union[ RunResource, BadRunResource ] = self._run_store.update_run_state( diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index fad6a31d8b1..1ae9a911267 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -176,6 +176,7 @@ def update_run_state( transaction.execute(update_run) transaction.execute(delete_existing_commands) for command_index, command in enumerate(commands): + print(command.error) transaction.execute( insert_command, { From 55a809a711b48a63fcb96791115b02d628efa452 Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Wed, 6 Nov 2024 11:50:43 -0500 Subject: [PATCH 12/53] Update robot-server/robot_server/runs/run_store.py --- robot-server/robot_server/runs/run_store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 1ae9a911267..c46cc366bba 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -133,7 +133,6 @@ def update_run_state( run_id: The run to update summary: The run's equipment and status summary. commands: The run's commands. - command_errors: The run's commands errors. run_time_parameters: The run's run time parameters, if any. Returns: From 9fa97bac4fe718e866fd4ea9b0ac9e63f8c87242 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 6 Nov 2024 13:43:15 -0500 Subject: [PATCH 13/53] change get_run_errors to get count --- .../persistence/_migrations/v7_to_v8.py | 2 +- .../robot_server/runs/router/base_router.py | 7 +++-- .../robot_server/runs/run_data_manager.py | 15 +++++------ robot-server/robot_server/runs/run_store.py | 26 ++++++++++++++++++- .../tests/runs/router/test_base_router.py | 15 +++-------- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index fd791c10f92..744af4de432 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -78,7 +78,7 @@ def _migrate_command_table_with_new_command_error_col( # Account for old_row.command["error"] being NULL. None if "error" not in row.command or data["error"] == None # noqa: E711 - else data["error"] + else json.dumps(data["error"]) ) dest_transaction.execute( diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index b7df09f8992..986050a7f5e 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -526,14 +526,13 @@ async def get_run_commands_error( run_data_manager: Run data retrieval interface. """ try: - all_errors = run_data_manager.get_command_errors(run_id=runId) - total_length = len(all_errors) + all_errors_count = run_data_manager.get_command_errors_count(run_id=runId) if cursor is None: - if len(all_errors) > 0: + if all_errors_count > 0: # Get the most recent error, # which we can find just at the end of the list. - cursor = total_length - 1 + cursor = all_errors_count - 1 else: cursor = 0 diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index fd33f72734e..296572460b4 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -15,7 +15,6 @@ CommandErrorSlice, CommandPointer, Command, - ErrorOccurrence, ) from opentrons.protocol_engine.types import ( PrimitiveRunTimeParamValuesType, @@ -438,9 +437,9 @@ def get_command_error_slice( return self._run_orchestrator_store.get_command_error_slice( cursor=cursor, length=length ) - - # TODO(tz, 8-5-2024): Change this to return to error list from the DB when we implement https://opentrons.atlassian.net/browse/EXEC-655. - raise RunNotCurrentError() + return self._run_store.get_commands_errors_slice( + run_id=run_id, cursor=cursor, length=length + ) def get_current_command(self, run_id: str) -> Optional[CommandPointer]: """Get the "current" command, if any. @@ -499,13 +498,11 @@ def get_command(self, run_id: str, command_id: str) -> Command: return self._run_store.get_command(run_id=run_id, command_id=command_id) - def get_command_errors(self, run_id: str) -> list[ErrorOccurrence]: + def get_command_errors_count(self, run_id: str) -> int: """Get all command errors.""" if run_id == self._run_orchestrator_store.current_run_id: - return self._run_orchestrator_store.get_command_errors() - - # TODO(tz, 8-5-2024): Change this to return the error list from the DB when we implement https://opentrons.atlassian.net/browse/EXEC-655. - raise RunNotCurrentError() + return len(self._run_orchestrator_store.get_command_errors()) + return self._run_store.get_command_errors_count(run_id) def get_nozzle_maps(self, run_id: str) -> Dict[str, NozzleMap]: """Get current nozzle maps keyed by pipette id.""" diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index c46cc366bba..df557832d9f 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -175,7 +175,6 @@ def update_run_state( transaction.execute(update_run) transaction.execute(delete_existing_commands) for command_index, command in enumerate(commands): - print(command.error) transaction.execute( insert_command, { @@ -547,6 +546,31 @@ def get_all_commands_as_preserialized_list( commands_result = transaction.scalars(select_commands).all() return commands_result + def get_command_errors_count(self, run_id: str) -> int: + """Get run commands errors count from the store. + + Args: + run_id: Run ID to pull commands from. + + Returns: + The number of commands errors. + + Raises: + RunNotFoundError: The given run ID was not found. + """ + with self._sql_engine.begin() as transaction: + if not self._run_exists(run_id, transaction): + raise RunNotFoundError(run_id=run_id) + + select_count = sqlalchemy.select(sqlalchemy.func.count()).where( + and_( + run_command_table.c.run_id == run_id, + run_command_table.c.command_error is not None, + ) + ) + errors_count: int = transaction.execute(select_count).scalar_one() + return errors_count + def get_commands_errors_slice( self, run_id: str, diff --git a/robot-server/tests/runs/router/test_base_router.py b/robot-server/tests/runs/router/test_base_router.py index 9052b588bc9..0a813b02a2e 100644 --- a/robot-server/tests/runs/router/test_base_router.py +++ b/robot-server/tests/runs/router/test_base_router.py @@ -769,13 +769,7 @@ async def test_get_run_commands_errors( ) ).then_raise(RunNotCurrentError("oh no!")) - error = pe_errors.ErrorOccurrence( - id="error-id", - errorType="PrettyBadError", - createdAt=datetime(year=2024, month=4, day=4), - detail="Things are not looking good.", - ) - decoy.when(mock_run_data_manager.get_command_errors("run-id")).then_return([error]) + decoy.when(mock_run_data_manager.get_command_errors_count("run-id")).then_return(1) with pytest.raises(ApiError): result = await get_run_commands_error( @@ -797,7 +791,7 @@ async def test_get_run_commands_errors_raises_no_run( createdAt=datetime(year=2024, month=4, day=4), detail="Things are not looking good.", ) - decoy.when(mock_run_data_manager.get_command_errors("run-id")).then_return([error]) + decoy.when(mock_run_data_manager.get_command_errors_count("run-id")).then_return(1) command_error_slice = CommandErrorSlice( cursor=1, total_length=3, commands_errors=[error] @@ -841,10 +835,7 @@ async def test_get_run_commands_errors_defualt_cursor( expected_cursor_result: int, ) -> None: """It should return a list of all commands errors in a run.""" - print(error_list) - decoy.when(mock_run_data_manager.get_command_errors("run-id")).then_return( - error_list - ) + decoy.when(mock_run_data_manager.get_command_errors_count("run-id")).then_return(1) command_error_slice = CommandErrorSlice( cursor=expected_cursor_result, total_length=3, commands_errors=error_list From 13c5b87f01f4715ec5e9eda81228b04c6d57bb2a Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 6 Nov 2024 15:37:07 -0500 Subject: [PATCH 14/53] tables tests fixed --- robot-server/tests/persistence/test_tables.py | 133 +++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index 0277919aa30..2f2f79babc5 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -106,6 +106,7 @@ command_id VARCHAR NOT NULL, command VARCHAR NOT NULL, command_intent VARCHAR NOT NULL, + command_error VARCHAR, PRIMARY KEY (row_id), FOREIGN KEY(run_id) REFERENCES run (id) ) @@ -123,6 +124,9 @@ CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) """, """ + CREATE INDEX ix_run_command_command_error ON run_command (command_error) + """, + """ CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) """, """ @@ -158,7 +162,134 @@ ] -EXPECTED_STATEMENTS_V7 = EXPECTED_STATEMENTS_LATEST +EXPECTED_STATEMENTS_V7 = [ + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + protocol_kind VARCHAR(14) NOT NULL, + PRIMARY KEY (id), + CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE analysis_primitive_rtp_table ( + row_id INTEGER NOT NULL, + analysis_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + parameter_type VARCHAR(5) NOT NULL, + parameter_value VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(analysis_id) REFERENCES analysis (id), + CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) + ) + """, + """ + CREATE TABLE analysis_csv_rtp_table ( + row_id INTEGER NOT NULL, + analysis_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + file_id VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(analysis_id) REFERENCES analysis (id), + FOREIGN KEY(file_id) REFERENCES data_files (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary VARCHAR, + engine_status VARCHAR, + _updated_at DATETIME, + run_time_parameters VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE TABLE run_command ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + index_in_run INTEGER NOT NULL, + command_id VARCHAR NOT NULL, + command VARCHAR NOT NULL, + command_intent VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) + """, + """ + CREATE INDEX ix_data_files_source ON data_files (source) + """, + """ + CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) + """, + """ + CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) + """, + """ + CREATE TABLE data_files ( + id VARCHAR NOT NULL, + name VARCHAR NOT NULL, + file_hash VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + source VARCHAR(9) NOT NULL, + PRIMARY KEY (id), + CONSTRAINT datafilesourcesqlenum CHECK (source IN ('uploaded', 'generated')) + ) + """, + """ + CREATE TABLE run_csv_rtp_table ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + file_id VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id), + FOREIGN KEY(file_id) REFERENCES data_files (id) + ) + """, + """ + CREATE TABLE boolean_setting ( + "key" VARCHAR(21) NOT NULL, + value BOOLEAN NOT NULL, + PRIMARY KEY ("key"), + CONSTRAINT booleansettingkey CHECK ("key" IN ('enable_error_recovery')) + ) + """, +] EXPECTED_STATEMENTS_V6 = [ From a88e95d31d891504420c7399392486bdf0a2227a Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 6 Nov 2024 15:52:07 -0500 Subject: [PATCH 15/53] added test for historical run --- .../tests/runs/test_run_data_manager.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index 5e4aed1f3e2..751676a6b58 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -924,16 +924,30 @@ def test_get_commands_slice_current_run( assert expected_command_slice == result -def test_get_commands_errors_slice__not_current_run_raises( +def test_get_commands_errors_slice_historical_run( decoy: Decoy, subject: RunDataManager, mock_run_orchestrator_store: RunOrchestratorStore, + mock_run_store: RunStore, ) -> None: """Should get a sliced command error list from engine store.""" + expected_commands_errors_result = [ + ErrorOccurrence.construct(id="error-id") # type: ignore[call-arg] + ] + + command_error_slice = CommandErrorSlice( + cursor=1, total_length=3, commands_errors=expected_commands_errors_result + ) + decoy.when(mock_run_orchestrator_store.current_run_id).then_return("run-not-id") - with pytest.raises(RunNotCurrentError): - subject.get_command_error_slice("run-id", 1, 2) + decoy.when(mock_run_store.get_commands_errors_slice("run-id", 2, 1)).then_return( + command_error_slice + ) + + result = subject.get_command_error_slice("run-id", 1, 2) + + assert command_error_slice == result def test_get_commands_errors_slice_current_run( From 1bea28eef0ee78374919e60091c612b99ecfdbff Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 6 Nov 2024 16:10:47 -0500 Subject: [PATCH 16/53] fixed persistance path --- .../robot_server/persistence/file_and_directory_names.py | 2 +- robot-server/robot_server/persistence/persistence_directory.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/file_and_directory_names.py b/robot-server/robot_server/persistence/file_and_directory_names.py index 2064b3f59fd..220a32e7673 100644 --- a/robot-server/robot_server/persistence/file_and_directory_names.py +++ b/robot-server/robot_server/persistence/file_and_directory_names.py @@ -8,7 +8,7 @@ from typing import Final -LATEST_VERSION_DIRECTORY: Final = "8.1" +LATEST_VERSION_DIRECTORY: Final = "8" DECK_CONFIGURATION_FILE: Final = "deck_configuration.json" PROTOCOLS_DIRECTORY: Final = "protocols" diff --git a/robot-server/robot_server/persistence/persistence_directory.py b/robot-server/robot_server/persistence/persistence_directory.py index 19e67fe8333..58fbbd5d97b 100644 --- a/robot-server/robot_server/persistence/persistence_directory.py +++ b/robot-server/robot_server/persistence/persistence_directory.py @@ -55,7 +55,7 @@ async def prepare_active_subdirectory(prepared_root: Path) -> Path: # Subdirectory "7" was previously used on our edge branch for an in-dev # schema that was never released to the public. It may be present on # internal robots. - v6_to_v7.Migration6to7(subdirectory="7"), + v6_to_v7.Migration6to7(subdirectory="7.1"), v7_to_v8.Migration7to8(subdirectory=LATEST_VERSION_DIRECTORY), ], temp_file_prefix="temp-", From 8f73bc1627060a1a9d61c1fc4148aeec690355ff Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Thu, 7 Nov 2024 10:40:25 -0500 Subject: [PATCH 17/53] fixed import from schema 7 --- robot-server/robot_server/data_files/data_files_store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/robot_server/data_files/data_files_store.py b/robot-server/robot_server/data_files/data_files_store.py index 28257dbb8d2..e0ef8fefa44 100644 --- a/robot-server/robot_server/data_files/data_files_store.py +++ b/robot-server/robot_server/data_files/data_files_store.py @@ -15,7 +15,7 @@ analysis_csv_rtp_table, run_csv_rtp_table, ) -from robot_server.persistence.tables.schema_7 import DataFileSourceSQLEnum +from robot_server.persistence.tables import DataFileSourceSQLEnum from .models import DataFileSource, FileIdNotFoundError, FileInUseError From c87dc654ecd20c9700a4826bc1c5d5657f70880b Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Thu, 7 Nov 2024 11:39:36 -0500 Subject: [PATCH 18/53] rollback add_column change --- .../persistence/_migrations/v7_to_v8.py | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 744af4de432..aa4f9f940ee 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -2,9 +2,7 @@ Summary of changes from schema 7: -- Adds a new command_intent to store the commands intent in the commands table -- Adds a new source to store the data files origin in the data_files table -- Adds the `boolean_setting` table. +- Adds a new command_error to store the commands error in the commands table """ import json @@ -36,7 +34,6 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None: dest_db_file = dest_dir / DB_FILE - # Append the new column to existing protocols and data_files in v6 database with ExitStack() as exit_stack: dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file)) @@ -45,9 +42,9 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None: dest_transaction = exit_stack.enter_context(dest_engine.begin()) def add_column( - engine: sqlalchemy.engine.Engine, - table_name: str, - column: Any, + engine: sqlalchemy.engine.Engine, + table_name: str, + column: Any, ) -> None: column_type = column.type.compile(engine.dialect) engine.execute( @@ -69,9 +66,9 @@ def _migrate_command_table_with_new_command_error_col( dest_transaction: sqlalchemy.engine.Connection, ) -> None: """Add a new 'command_intent' column to run_command_table table.""" - select_commands = sqlalchemy.select(schema_8.run_command_table).order_by( - sqlite_rowid - ) + commands_table = schema_8.run_command_table + select_commands = sqlalchemy.select(commands_table).order_by(sqlite_rowid) + for row in dest_transaction.execute(select_commands).all(): data = json.loads(row.command) new_command_error = ( @@ -81,6 +78,10 @@ def _migrate_command_table_with_new_command_error_col( else json.dumps(data["error"]) ) - dest_transaction.execute( - f"UPDATE run_command SET command_error='{new_command_error}' WHERE row_id={row.row_id}" + update_commands = ( + sqlalchemy.update(schema_8.run_command_table) + .where(schema_8.run_command_table.c.row_id == row.row_id) + .values(command_error=new_command_error) ) + + dest_transaction.execute(update_commands) From 5405ef355fe564abcc3beb58c36845d5c2f2f1fe Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Thu, 7 Nov 2024 14:31:10 -0500 Subject: [PATCH 19/53] append_failed_command_id when setting a failed command --- api/src/opentrons/protocol_engine/state/command_history.py | 5 +---- api/src/opentrons/protocol_engine/state/commands.py | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/command_history.py b/api/src/opentrons/protocol_engine/state/command_history.py index 34dd2c88f2a..0879a7cd130 100644 --- a/api/src/opentrons/protocol_engine/state/command_history.py +++ b/api/src/opentrons/protocol_engine/state/command_history.py @@ -253,6 +253,7 @@ def set_command_failed(self, command: Command) -> None: self._remove_queue_id(command.id) self._remove_setup_queue_id(command.id) self._set_most_recently_completed_command_id(command.id) + self._all_failed_command_ids.append(command.id) def _add(self, command_id: str, command_entry: CommandEntry) -> None: """Create or update a command entry.""" @@ -262,10 +263,6 @@ def _add(self, command_id: str, command_entry: CommandEntry) -> None: self._all_command_ids_but_fixit_command_ids.append(command_id) self._commands_by_id[command_id] = command_entry - def append_failed_command_id(self, command_id: str) -> None: - """Create or update a failed command id.""" - self._all_failed_command_ids.append(command_id) - def _add_to_queue(self, command_id: str) -> None: """Add new ID to the queued.""" self._queued_command_ids.add(command_id) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 8cdba6aefec..da99c14ef3e 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -362,7 +362,6 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None: notes=action.notes, ) self._state.failed_command = self._state.command_history.get(action.command_id) - self._state.command_history.append_failed_command_id(action.command_id) if ( prev_entry.command.intent in (CommandIntent.PROTOCOL, None) From f888364f8881e563437c980b97d1646bd2d2a016 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Thu, 7 Nov 2024 16:10:38 -0500 Subject: [PATCH 20/53] append_failed_command_id when setting a failed command --- .../protocol_engine/state/test_command_view_old.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index 27aecc545d0..e5d9235bcab 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -17,6 +17,7 @@ PauseSource, StopAction, QueueCommandAction, + FailCommandAction ) from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction @@ -99,8 +100,6 @@ def get_command_view( # noqa: C901 command_id=command.id, command_entry=CommandEntry(index=index, command=command), ) - if command.error: - command_history.append_failed_command_id(command.id) state = CommandState( command_history=command_history, @@ -734,10 +733,8 @@ class GetStatusSpec(NamedTuple): run_result=RunResult.SUCCEEDED, run_completed_at=datetime(year=2021, day=1, month=1), ), - expected_status=EngineStatus.SUCCEEDED, - ), - GetStatusSpec( - subject=get_command_view( + '''''''[]' + 'and_view() run_result=RunResult.STOPPED, run_completed_at=None, ), @@ -1056,6 +1053,8 @@ def test_get_errors_slice() -> None: commands=[command_1, command_2, command_3, command_4, command_5, command_6] ) + + result = subject.get_errors_slice(cursor=1, length=3) assert result == CommandErrorSlice( From e6aa375215bf5cefa64e50a154308e4d41eeba57 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 8 Nov 2024 12:18:06 -0500 Subject: [PATCH 21/53] moved test to command state --- .../state/test_command_state.py | 106 ++++++++++++++++++ .../state/test_command_view_old.py | 53 +-------- 2 files changed, 110 insertions(+), 49 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_state.py b/api/tests/opentrons/protocol_engine/state/test_command_state.py index 76fa39b14e4..b0a348da54b 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_state.py @@ -30,6 +30,7 @@ from opentrons.protocol_engine.state.commands import ( CommandStore, CommandView, + CommandErrorSlice, ) from opentrons.protocol_engine.state.config import Config from opentrons.protocol_engine.state.update_types import StateUpdate @@ -1100,3 +1101,108 @@ def test_get_state_update_for_false_positive() -> None: subject.handle_action(resume_from_recovery) assert subject_view.get_state_update_for_false_positive() == empty_state_update + + +def test_get_errors_slice_empty() -> None: + """It should return a slice from the tail if no current command.""" + subject = CommandStore( + config=_make_config(), + error_recovery_policy=_placeholder_error_recovery_policy, + is_door_open=False, + ) + subject_view = CommandView(subject.state) + result = subject_view.get_errors_slice(cursor=0, length=2) + + assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0) + + +def test_get_errors_slice() -> None: + """It should return a slice of all command errors.""" + subject = CommandStore( + config=_make_config(), + error_recovery_policy=_placeholder_error_recovery_policy, + is_door_open=False, + ) + + subject_view = CommandView(subject.state) + + # error_1 = ErrorOccurrence.construct(id="error-id-1") # type: ignore[call-arg] + # error_2 = ErrorOccurrence.construct(id="error-id-2") # type: ignore[call-arg] + # error_3 = ErrorOccurrence.construct(id="error-id-3") # type: ignore[call-arg] + # error_4 = ErrorOccurrence.construct(id="error-id-4") # type: ignore[call-arg] + + queue_1 = actions.QueueCommandAction( + request=commands.WaitForResumeCreate( + params=commands.WaitForResumeParams(), key="command-key-1" + ), + request_hash=None, + created_at=datetime(year=2021, month=1, day=1), + command_id="command-id-1", + ) + subject.handle_action(queue_1) + queue_2_setup = actions.QueueCommandAction( + request=commands.WaitForResumeCreate( + params=commands.WaitForResumeParams(), + intent=commands.CommandIntent.SETUP, + key="command-key-2", + ), + request_hash=None, + created_at=datetime(year=2021, month=1, day=1), + command_id="command-id-2", + ) + subject.handle_action(queue_2_setup) + queue_3_setup = actions.QueueCommandAction( + request=commands.WaitForResumeCreate( + params=commands.WaitForResumeParams(), + intent=commands.CommandIntent.SETUP, + key="command-key-3", + ), + request_hash=None, + created_at=datetime(year=2021, month=1, day=1), + command_id="command-id-3", + ) + subject.handle_action(queue_3_setup) + + run_2_setup = actions.RunCommandAction( + command_id="command-id-2", + started_at=datetime(year=2022, month=2, day=2), + ) + subject.handle_action(run_2_setup) + fail_2_setup = actions.FailCommandAction( + command_id="command-id-2", + running_command=subject_view.get("command-id-2"), + error_id="error-id", + failed_at=datetime(year=2023, month=3, day=3), + error=errors.ProtocolEngineError(message="oh no"), + notes=[], + type=ErrorRecoveryType.CONTINUE_WITH_ERROR, + ) + subject.handle_action(fail_2_setup) + fail_3_setup = actions.FailCommandAction( + command_id="command-id-3", + running_command=subject_view.get("command-id-3"), + error_id="error-id-2", + failed_at=datetime(year=2023, month=3, day=3), + error=errors.ProtocolEngineError(message="oh hell no"), + notes=[], + type=ErrorRecoveryType.FAIL_RUN, + ) + + result = subject_view.get_errors_slice(cursor=1, length=3) + + assert result == CommandErrorSlice( + [ + ErrorOccurrence( + id="error-id", + createdAt=datetime(2023, 3, 3, 0, 0), + isDefined=False, + errorType="ProtocolEngineError", + errorCode="4000", + detail="oh no", + errorInfo={}, + wrappedErrors=[], + ) + ], + cursor=0, + total_length=1, + ) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index e5d9235bcab..0cbef9cf474 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -17,7 +17,6 @@ PauseSource, StopAction, QueueCommandAction, - FailCommandAction ) from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction @@ -29,7 +28,6 @@ CommandState, CommandView, CommandSlice, - CommandErrorSlice, CommandPointer, RunResult, QueueStatus, @@ -733,8 +731,10 @@ class GetStatusSpec(NamedTuple): run_result=RunResult.SUCCEEDED, run_completed_at=datetime(year=2021, day=1, month=1), ), - '''''''[]' - 'and_view() + expected_status=EngineStatus.SUCCEEDED, + ), + GetStatusSpec( + subject=get_command_view( run_result=RunResult.STOPPED, run_completed_at=None, ), @@ -1027,51 +1027,6 @@ def test_get_slice_default_cursor_running() -> None: ) -def test_get_errors_slice_empty() -> None: - """It should return a slice from the tail if no current command.""" - subject = get_command_view() - result = subject.get_errors_slice(cursor=0, length=2) - - assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0) - - -def test_get_errors_slice() -> None: - """It should return a slice of all command errors.""" - error_1 = ErrorOccurrence.construct(id="error-id-1") # type: ignore[call-arg] - error_2 = ErrorOccurrence.construct(id="error-id-2") # type: ignore[call-arg] - error_3 = ErrorOccurrence.construct(id="error-id-3") # type: ignore[call-arg] - error_4 = ErrorOccurrence.construct(id="error-id-4") # type: ignore[call-arg] - - command_1 = create_failed_command(command_id="command-id-1", error=error_1) - command_2 = create_failed_command(command_id="command-id-2", error=error_2) - command_3 = create_failed_command(command_id="command-id-3", error=error_3) - command_4 = create_failed_command(command_id="command-id-4", error=error_4) - command_5 = create_running_command(command_id="command-id-5") - command_6 = create_queued_command(command_id="command-id-6") - - subject = get_command_view( - commands=[command_1, command_2, command_3, command_4, command_5, command_6] - ) - - - - result = subject.get_errors_slice(cursor=1, length=3) - - assert result == CommandErrorSlice( - commands_errors=[error_2, error_3, error_4], - cursor=1, - total_length=4, - ) - - result = subject.get_errors_slice(cursor=-3, length=10) - - assert result == CommandErrorSlice( - commands_errors=[error_1, error_2, error_3, error_4], - cursor=0, - total_length=4, - ) - - def test_get_slice_without_fixit() -> None: """It should select a cursor based on the running command, if present.""" command_1 = create_succeeded_command(command_id="command-id-1") From 66856ac056eecdc8f0fafe838483e477b0de765d Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 8 Nov 2024 14:01:26 -0500 Subject: [PATCH 22/53] changed index and lint --- .../robot_server/persistence/_migrations/v7_to_v8.py | 6 +++--- robot-server/robot_server/persistence/tables/schema_8.py | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index aa4f9f940ee..53c988c630c 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -42,9 +42,9 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None: dest_transaction = exit_stack.enter_context(dest_engine.begin()) def add_column( - engine: sqlalchemy.engine.Engine, - table_name: str, - column: Any, + engine: sqlalchemy.engine.Engine, + table_name: str, + column: Any, ) -> None: column_type = column.type.compile(engine.dialect) engine.execute( diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index c13d83eb45d..06426f14eb8 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -208,7 +208,7 @@ class DataFileSourceSQLEnum(enum.Enum): sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), sqlalchemy.Column("command", sqlalchemy.String, nullable=False), sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=False, index=True), - sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True, index=True), + sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True), sqlalchemy.Index( "ix_run_run_id_command_id", # An arbitrary name for the index. "run_id", @@ -221,6 +221,11 @@ class DataFileSourceSQLEnum(enum.Enum): "index_in_run", unique=True, ), + sqlalchemy.Index( + "ix_run_id_command_error", # An arbitrary name for the index. + "run_id", + "command_error", + ), ) data_files_table = sqlalchemy.Table( From 02e6806748220571171072049b232fd1d628b799 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 8 Nov 2024 14:15:33 -0500 Subject: [PATCH 23/53] WIP - command status --- .../persistence/_migrations/v7_to_v8.py | 14 +++++++++++--- .../robot_server/persistence/tables/schema_8.py | 6 ++++-- robot-server/robot_server/runs/run_store.py | 1 + 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 53c988c630c..52a763cf2d6 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -57,12 +57,18 @@ def add_column( schema_8.run_command_table.c.command_error, ) - _migrate_command_table_with_new_command_error_col( + add_column( + dest_engine, + schema_8.run_command_table.name, + schema_8.run_command_table.c.command_status, + ) + + _migrate_command_table_with_new_command_error_col_and_command_status( dest_transaction=dest_transaction ) -def _migrate_command_table_with_new_command_error_col( +def _migrate_command_table_with_new_command_error_col_and_command_status( dest_transaction: sqlalchemy.engine.Connection, ) -> None: """Add a new 'command_intent' column to run_command_table table.""" @@ -77,11 +83,13 @@ def _migrate_command_table_with_new_command_error_col( if "error" not in row.command or data["error"] == None # noqa: E711 else json.dumps(data["error"]) ) + # parse json as enum + new_command_status = data["status"] update_commands = ( sqlalchemy.update(schema_8.run_command_table) .where(schema_8.run_command_table.c.row_id == row.row_id) - .values(command_error=new_command_error) + .values(command_error=new_command_error, command_status=new_command_status) ) dest_transaction.execute(update_commands) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 06426f14eb8..71b0744d106 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -209,6 +209,7 @@ class DataFileSourceSQLEnum(enum.Enum): sqlalchemy.Column("command", sqlalchemy.String, nullable=False), sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=False, index=True), sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True), + sqlalchemy.Column("command_status", sqlalchemy.String, nullable=False), sqlalchemy.Index( "ix_run_run_id_command_id", # An arbitrary name for the index. "run_id", @@ -222,9 +223,10 @@ class DataFileSourceSQLEnum(enum.Enum): unique=True, ), sqlalchemy.Index( - "ix_run_id_command_error", # An arbitrary name for the index. + "ix_run_run_id_index_in_run_command_status", # An arbitrary name for the index. "run_id", - "command_error", + "index_in_run", + "command_status", ), ) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index df557832d9f..04e02a65f03 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -188,6 +188,7 @@ def update_run_state( "command_error": pydantic_to_json(command.error) if command.error else None, + "command_status": command.status.value, }, ) From 37a4affd9eb4a6e1e2e78a4935e95bc26f615070 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 8 Nov 2024 15:46:42 -0500 Subject: [PATCH 24/53] fixed failing tests --- .../protocol_engine/state/test_command_state.py | 9 --------- robot-server/tests/persistence/test_tables.py | 3 ++- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_state.py b/api/tests/opentrons/protocol_engine/state/test_command_state.py index b0a348da54b..1eda4117143 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_state.py @@ -1178,15 +1178,6 @@ def test_get_errors_slice() -> None: type=ErrorRecoveryType.CONTINUE_WITH_ERROR, ) subject.handle_action(fail_2_setup) - fail_3_setup = actions.FailCommandAction( - command_id="command-id-3", - running_command=subject_view.get("command-id-3"), - error_id="error-id-2", - failed_at=datetime(year=2023, month=3, day=3), - error=errors.ProtocolEngineError(message="oh hell no"), - notes=[], - type=ErrorRecoveryType.FAIL_RUN, - ) result = subject_view.get_errors_slice(cursor=1, length=3) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index 2f2f79babc5..e1b1251670d 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -107,6 +107,7 @@ command VARCHAR NOT NULL, command_intent VARCHAR NOT NULL, command_error VARCHAR, + command_status VARCHAR NOT NULL, PRIMARY KEY (row_id), FOREIGN KEY(run_id) REFERENCES run (id) ) @@ -124,7 +125,7 @@ CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) """, """ - CREATE INDEX ix_run_command_command_error ON run_command (command_error) + CREATE INDEX ix_run_run_id_index_in_run_command_status ON run_command (run_id, index_in_run, command_status) """, """ CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) From 6c6257f59cfaae46787482e82aed9543a40e05af Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 8 Nov 2024 15:56:24 -0500 Subject: [PATCH 25/53] insert and get with command_status --- robot-server/robot_server/runs/run_store.py | 4 ++-- robot-server/tests/runs/test_run_store.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 04e02a65f03..db751ff9ade 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -566,7 +566,7 @@ def get_command_errors_count(self, run_id: str) -> int: select_count = sqlalchemy.select(sqlalchemy.func.count()).where( and_( run_command_table.c.run_id == run_id, - run_command_table.c.command_error is not None, + run_command_table.c.command_status == "failed", ) ) errors_count: int = transaction.execute(select_count).scalar_one() @@ -601,7 +601,7 @@ def get_commands_errors_slice( select_count = sqlalchemy.select(sqlalchemy.func.count()).where( and_( run_command_table.c.run_id == run_id, - run_command_table.c.command_error is not None, + run_command_table.c.command_status == "failed", ) ) count_result: int = transaction.execute(select_count).scalar_one() diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index e2850f1ce47..519fbaba83e 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -107,7 +107,7 @@ def protocol_commands_errors() -> List[pe_commands.Command]: pe_commands.WaitForResume( id="pause-1", key="command-key", - status=pe_commands.CommandStatus.SUCCEEDED, + status=pe_commands.CommandStatus.FAILED, createdAt=datetime(year=2021, month=1, day=1), params=pe_commands.WaitForResumeParams(message="hello world"), result=pe_commands.WaitForResumeResult(), @@ -122,7 +122,7 @@ def protocol_commands_errors() -> List[pe_commands.Command]: pe_commands.WaitForResume( id="pause-2", key="command-key", - status=pe_commands.CommandStatus.SUCCEEDED, + status=pe_commands.CommandStatus.FAILED, createdAt=datetime(year=2022, month=2, day=2), params=pe_commands.WaitForResumeParams(message="hey world"), result=pe_commands.WaitForResumeResult(), @@ -360,6 +360,7 @@ async def test_update_run_state_command_with_errors( length=len(protocol_commands_errors), cursor=0, ) + print(command_errors_result) assert command_errors_result.commands_errors == [ item.error for item in protocol_commands_errors From 56d12c41382d757cf052b4c516915a2f1f45d30c Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 8 Nov 2024 16:25:54 -0500 Subject: [PATCH 26/53] translate to CommandStatusSQLEnum --- .../persistence/_migrations/v7_to_v8.py | 3 ++- .../persistence/tables/schema_8.py | 20 ++++++++++++++++++- robot-server/robot_server/runs/run_store.py | 3 ++- robot-server/tests/persistence/test_tables.py | 5 +++-- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 52a763cf2d6..897c3d7e3ef 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -20,6 +20,7 @@ from ..file_and_directory_names import ( DB_FILE, ) +from ..tables.schema_8 import CommandStatusSQLEnum class Migration7to8(Migration): # noqa: D101 @@ -84,7 +85,7 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( else json.dumps(data["error"]) ) # parse json as enum - new_command_status = data["status"] + new_command_status = CommandStatusSQLEnum(data["status"]) update_commands = ( sqlalchemy.update(schema_8.run_command_table) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 71b0744d106..2cca4cd327d 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -30,6 +30,15 @@ class DataFileSourceSQLEnum(enum.Enum): GENERATED = "generated" +class CommandStatusSQLEnum(enum.Enum): + """Command status sql enum.""" + + QUEUED = "queued" + RUNNING = "running" + SUCCEEDED = "succeeded" + FAILED = "failed" + + protocol_table = sqlalchemy.Table( "protocol", metadata, @@ -209,7 +218,16 @@ class DataFileSourceSQLEnum(enum.Enum): sqlalchemy.Column("command", sqlalchemy.String, nullable=False), sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=False, index=True), sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True), - sqlalchemy.Column("command_status", sqlalchemy.String, nullable=False), + sqlalchemy.Column( + "command_status", + sqlalchemy.Enum( + CommandStatusSQLEnum, + values_callable=lambda obj: [e.value for e in obj], + validate_strings=True, + create_constraint=True, + ), + nullable=False, + ), sqlalchemy.Index( "ix_run_run_id_command_id", # An arbitrary name for the index. "run_id", diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index db751ff9ade..bf384803c4d 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -44,6 +44,7 @@ from .action_models import RunAction, RunActionType from .run_models import RunNotFoundError +from ..persistence.tables.schema_8 import CommandStatusSQLEnum log = logging.getLogger(__name__) @@ -188,7 +189,7 @@ def update_run_state( "command_error": pydantic_to_json(command.error) if command.error else None, - "command_status": command.status.value, + "command_status": CommandStatusSQLEnum(command.status.value), }, ) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index e1b1251670d..70d25ae3625 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -107,9 +107,10 @@ command VARCHAR NOT NULL, command_intent VARCHAR NOT NULL, command_error VARCHAR, - command_status VARCHAR NOT NULL, + command_status VARCHAR(9) NOT NULL, PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) + FOREIGN KEY(run_id) REFERENCES run (id), + CONSTRAINT commandstatussqlenum CHECK (command_status IN ('queued', 'running', 'succeeded', 'failed')) ) """, """ From c33745bc70b086ef557ad9e4f6d86864afac4e04 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 12 Nov 2024 09:54:04 -0500 Subject: [PATCH 27/53] nips --- robot-server/robot_server/persistence/tables/schema_8.py | 1 + robot-server/robot_server/runs/run_store.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 2cca4cd327d..f47a5cd8bd4 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -240,6 +240,7 @@ class CommandStatusSQLEnum(enum.Enum): "index_in_run", unique=True, ), + # TODO(tz, 12-11-2024): change index to command_error when partial index is supported. sqlalchemy.Index( "ix_run_run_id_index_in_run_command_status", # An arbitrary name for the index. "run_id", diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index bf384803c4d..1174338cbee 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -567,7 +567,7 @@ def get_command_errors_count(self, run_id: str) -> int: select_count = sqlalchemy.select(sqlalchemy.func.count()).where( and_( run_command_table.c.run_id == run_id, - run_command_table.c.command_status == "failed", + run_command_table.c.command_status == CommandStatusSQLEnum.FAILED, ) ) errors_count: int = transaction.execute(select_count).scalar_one() @@ -602,7 +602,7 @@ def get_commands_errors_slice( select_count = sqlalchemy.select(sqlalchemy.func.count()).where( and_( run_command_table.c.run_id == run_id, - run_command_table.c.command_status == "failed", + run_command_table.c.command_status == CommandStatusSQLEnum.FAILED, ) ) count_result: int = transaction.execute(select_count).scalar_one() From 635a5afd822b3e566ad6efeb9a71d393b6f115af Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Tue, 12 Nov 2024 16:07:08 -0500 Subject: [PATCH 28/53] Update robot-server/tests/runs/test_run_store.py Co-authored-by: Max Marrone --- robot-server/tests/runs/test_run_store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 519fbaba83e..98b3a660cf0 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -360,7 +360,6 @@ async def test_update_run_state_command_with_errors( length=len(protocol_commands_errors), cursor=0, ) - print(command_errors_result) assert command_errors_result.commands_errors == [ item.error for item in protocol_commands_errors From a16636b1af76ca3f15085c466255d83901c54a23 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Tue, 12 Nov 2024 16:14:10 -0500 Subject: [PATCH 29/53] nits --- .../opentrons/protocol_engine/state/test_command_state.py | 7 +------ .../robot_server/persistence/_migrations/v7_to_v8.py | 1 + robot-server/robot_server/persistence/tables/schema_8.py | 7 ++++--- robot-server/tests/runs/test_run_store.py | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_state.py b/api/tests/opentrons/protocol_engine/state/test_command_state.py index 1eda4117143..c52cd8ca74d 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_state.py @@ -1104,7 +1104,7 @@ def test_get_state_update_for_false_positive() -> None: def test_get_errors_slice_empty() -> None: - """It should return a slice from the tail if no current command.""" + """It should return an empty error list.""" subject = CommandStore( config=_make_config(), error_recovery_policy=_placeholder_error_recovery_policy, @@ -1126,11 +1126,6 @@ def test_get_errors_slice() -> None: subject_view = CommandView(subject.state) - # error_1 = ErrorOccurrence.construct(id="error-id-1") # type: ignore[call-arg] - # error_2 = ErrorOccurrence.construct(id="error-id-2") # type: ignore[call-arg] - # error_3 = ErrorOccurrence.construct(id="error-id-3") # type: ignore[call-arg] - # error_4 = ErrorOccurrence.construct(id="error-id-4") # type: ignore[call-arg] - queue_1 = actions.QueueCommandAction( request=commands.WaitForResumeCreate( params=commands.WaitForResumeParams(), key="command-key-1" diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 897c3d7e3ef..52055c6cc88 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -3,6 +3,7 @@ Summary of changes from schema 7: - Adds a new command_error to store the commands error in the commands table +- Adds a new command_status to store the commands status in the commands table """ import json diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index f47a5cd8bd4..1944d491bda 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -213,6 +213,7 @@ class CommandStatusSQLEnum(enum.Enum): sqlalchemy.Column( "run_id", sqlalchemy.String, sqlalchemy.ForeignKey("run.id"), nullable=False ), + # command_index in commands enumeration sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), sqlalchemy.Column("command", sqlalchemy.String, nullable=False), @@ -240,12 +241,12 @@ class CommandStatusSQLEnum(enum.Enum): "index_in_run", unique=True, ), - # TODO(tz, 12-11-2024): change index to command_error when partial index is supported. sqlalchemy.Index( - "ix_run_run_id_index_in_run_command_status", # An arbitrary name for the index. + "ix_run_run_id_command_status_index_in_run", # An arbitrary name for the index. "run_id", - "index_in_run", "command_status", + "index_in_run", + unique=True, ), ) diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 98b3a660cf0..2996512e672 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -60,7 +60,7 @@ def subject( @pytest.fixture def protocol_commands() -> List[pe_commands.Command]: - """Get a StateSummary value object.""" + """Get protocol commands list.""" return [ pe_commands.WaitForResume( id="pause-1", @@ -102,7 +102,7 @@ def protocol_commands() -> List[pe_commands.Command]: @pytest.fixture def protocol_commands_errors() -> List[pe_commands.Command]: - """Get a StateSummary value object.""" + """Get protocol commands errors list.""" return [ pe_commands.WaitForResume( id="pause-1", From a87336909738635fabe9f9dea1650062718a574c Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 10:55:28 -0500 Subject: [PATCH 30/53] batch edit --- .../persistence/_migrations/v7_to_v8.py | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 52055c6cc88..28a831d5df0 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -14,7 +14,7 @@ import sqlalchemy -from ..database import sql_engine_ctx, sqlite_rowid +from ..database import sql_engine_ctx from ..tables import schema_8 from .._folder_migrator import Migration @@ -75,8 +75,8 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( ) -> None: """Add a new 'command_intent' column to run_command_table table.""" commands_table = schema_8.run_command_table - select_commands = sqlalchemy.select(commands_table).order_by(sqlite_rowid) - + select_commands = sqlalchemy.select(commands_table) + commands_to_update = [] for row in dest_transaction.execute(select_commands).all(): data = json.loads(row.command) new_command_error = ( @@ -87,11 +87,22 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( ) # parse json as enum new_command_status = CommandStatusSQLEnum(data["status"]) - - update_commands = ( - sqlalchemy.update(schema_8.run_command_table) - .where(schema_8.run_command_table.c.row_id == row.row_id) - .values(command_error=new_command_error, command_status=new_command_status) + commands_to_update.append( + { + "command_error": new_command_error, + "command_status": new_command_status, + "_id": row.row_id, + } ) - dest_transaction.execute(update_commands) + update_commands = ( + sqlalchemy.update(schema_8.run_command_table) + .where(schema_8.run_command_table.c.row_id == sqlalchemy.bindparam("_id")) + .values( + { + "command_error": sqlalchemy.bindparam("command_error"), + "command_status": sqlalchemy.bindparam("command_status"), + } + ) + ) + dest_transaction.execute(update_commands, commands_to_update) From 67b6d53dbd739f30002c41cdee4366e701ce80bd Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 12:45:26 -0500 Subject: [PATCH 31/53] batch update without binding --- .../persistence/_migrations/v7_to_v8.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 28a831d5df0..cca04d8aea0 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -73,7 +73,7 @@ def add_column( def _migrate_command_table_with_new_command_error_col_and_command_status( dest_transaction: sqlalchemy.engine.Connection, ) -> None: - """Add a new 'command_intent' column to run_command_table table.""" + """Add a new 'command_error' and 'command_status' column to run_command_table table.""" commands_table = schema_8.run_command_table select_commands = sqlalchemy.select(commands_table) commands_to_update = [] @@ -89,20 +89,14 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( new_command_status = CommandStatusSQLEnum(data["status"]) commands_to_update.append( { + "row_id": row.row_id, "command_error": new_command_error, "command_status": new_command_status, - "_id": row.row_id, } ) - update_commands = ( - sqlalchemy.update(schema_8.run_command_table) - .where(schema_8.run_command_table.c.row_id == sqlalchemy.bindparam("_id")) - .values( - { - "command_error": sqlalchemy.bindparam("command_error"), - "command_status": sqlalchemy.bindparam("command_status"), - } + if len(commands_to_update) > 0: + update_commands = sqlalchemy.update(commands_table).values( + commands_to_update ) - ) - dest_transaction.execute(update_commands, commands_to_update) + dest_transaction.execute(update_commands) From 4d00c994feb2dec4803b354e1dd7cef24e6909ba Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 13:01:39 -0500 Subject: [PATCH 32/53] test table with new index and migration bulk fix --- .../persistence/_migrations/v7_to_v8.py | 16 +++++++++++----- robot-server/tests/persistence/test_tables.py | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index cca04d8aea0..ccf100647a5 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -89,14 +89,20 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( new_command_status = CommandStatusSQLEnum(data["status"]) commands_to_update.append( { - "row_id": row.row_id, + "_id": row.row_id, "command_error": new_command_error, "command_status": new_command_status, } ) - if len(commands_to_update) > 0: - update_commands = sqlalchemy.update(commands_table).values( - commands_to_update + update_commands = ( + sqlalchemy.update(commands_table) + .where(commands_table.c.row_id == sqlalchemy.bindparam("_id")) + .values( + { + "command_error": sqlalchemy.bindparam("command_error"), + "command_status": sqlalchemy.bindparam("command_status"), + } ) - dest_transaction.execute(update_commands) + ) + dest_transaction.execute(update_commands, commands_to_update) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index 70d25ae3625..93501ed2b4d 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -126,7 +126,7 @@ CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) """, """ - CREATE INDEX ix_run_run_id_index_in_run_command_status ON run_command (run_id, index_in_run, command_status) + CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run) """, """ CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) From 38cd6eaf8f88903f6f74a17b44b4820de3be59b8 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 13:16:24 -0500 Subject: [PATCH 33/53] conditional update --- .../persistence/_migrations/v7_to_v8.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index ccf100647a5..e2d6ce58d0e 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -95,14 +95,15 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( } ) - update_commands = ( - sqlalchemy.update(commands_table) - .where(commands_table.c.row_id == sqlalchemy.bindparam("_id")) - .values( - { - "command_error": sqlalchemy.bindparam("command_error"), - "command_status": sqlalchemy.bindparam("command_status"), - } + if len(commands_to_update) > 0: + update_commands = ( + sqlalchemy.update(commands_table) + .where(commands_table.c.row_id == sqlalchemy.bindparam("_id")) + .values( + { + "command_error": sqlalchemy.bindparam("command_error"), + "command_status": sqlalchemy.bindparam("command_status"), + } + ) ) - ) - dest_transaction.execute(update_commands, commands_to_update) + dest_transaction.execute(update_commands, commands_to_update) From 1160174bb63dd10b2e1ce74a4e35ec2a1b49ec7b Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 14:18:39 -0500 Subject: [PATCH 34/53] map command status to sql command status --- .../persistence/_migrations/v7_to_v8.py | 20 ++++++++++++++++++- robot-server/robot_server/runs/run_store.py | 20 ++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index e2d6ce58d0e..5745dee7f04 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -86,7 +86,9 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( else json.dumps(data["error"]) ) # parse json as enum - new_command_status = CommandStatusSQLEnum(data["status"]) + new_command_status = _convert_commands_status_to_sql_command_status( + data["status"].value + ) commands_to_update.append( { "_id": row.row_id, @@ -107,3 +109,19 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( ) ) dest_transaction.execute(update_commands, commands_to_update) + + +def _convert_commands_status_to_sql_command_status( + status: str, +) -> CommandStatusSQLEnum: + match status: + case "queued": + return CommandStatusSQLEnum.QUEUED + case "running": + return CommandStatusSQLEnum.RUNNING + case "failed": + return CommandStatusSQLEnum.FAILED + case "succeeded": + return CommandStatusSQLEnum.SUCCEEDED + case _: + assert False, "command status is unknown" diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 1174338cbee..b1fc792b42c 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -189,7 +189,9 @@ def update_run_state( "command_error": pydantic_to_json(command.error) if command.error else None, - "command_status": CommandStatusSQLEnum(command.status.value), + "command_status": _convert_commands_status_to_sql_command_status( + command.status.value + ), }, ) @@ -812,3 +814,19 @@ def _convert_state_to_sql_values( "_updated_at": utc_now(), "run_time_parameters": pydantic_list_to_json(run_time_parameters), } + + +def _convert_commands_status_to_sql_command_status( + status: str, +) -> CommandStatusSQLEnum: + match status: + case "queued": + return CommandStatusSQLEnum.QUEUED + case "running": + return CommandStatusSQLEnum.RUNNING + case "failed": + return CommandStatusSQLEnum.FAILED + case "succeeded": + return CommandStatusSQLEnum.SUCCEEDED + case _: + assert False, "command status is unknown" From 18ce80cf704188bac2f6c3c706a645eee5de4bfc Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 14:31:24 -0500 Subject: [PATCH 35/53] map command status to sql command status in migration --- robot-server/robot_server/persistence/_migrations/v7_to_v8.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 5745dee7f04..736e24c92ac 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -87,7 +87,7 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( ) # parse json as enum new_command_status = _convert_commands_status_to_sql_command_status( - data["status"].value + data["status"] ) commands_to_update.append( { From 964fddc76a1d8ffa806896d847a8084bb87059c4 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 14:40:44 -0500 Subject: [PATCH 36/53] map command status to sql command status in migration --- robot-server/robot_server/persistence/_migrations/v7_to_v8.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 736e24c92ac..b5a3794aee3 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -39,7 +39,7 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None: with ExitStack() as exit_stack: dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file)) - schema_8.metadata.create_all(dest_engine) + # schema_8.metadata.create_all(dest_engine) dest_transaction = exit_stack.enter_context(dest_engine.begin()) From 5ad4d0db535a35027ff7ac3b314c4ada2972e3b3 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 15:42:49 -0500 Subject: [PATCH 37/53] add missing indexes --- .../robot_server/persistence/_migrations/v7_to_v8.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index b5a3794aee3..a92da18268c 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -39,8 +39,6 @@ def migrate(self, source_dir: Path, dest_dir: Path) -> None: with ExitStack() as exit_stack: dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file)) - # schema_8.metadata.create_all(dest_engine) - dest_transaction = exit_stack.enter_context(dest_engine.begin()) def add_column( @@ -65,11 +63,21 @@ def add_column( schema_8.run_command_table.c.command_status, ) + _add_missing_indexes(dest_transaction=dest_transaction) + _migrate_command_table_with_new_command_error_col_and_command_status( dest_transaction=dest_transaction ) +def _add_missing_indexes(dest_transaction: sqlalchemy.engine.Connection) -> None: + dest_transaction.execute( + "CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run);" + ) + dest_transaction.execute("CREATE INDEX ix_run_command_command_intent ON run_command (command_intent);") + dest_transaction.execute("CREATE INDEX ix_data_files_source ON data_files (source);") + + def _migrate_command_table_with_new_command_error_col_and_command_status( dest_transaction: sqlalchemy.engine.Connection, ) -> None: From de022684ee901e06095657755bcbd4a9de4fb5e3 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 15:43:20 -0500 Subject: [PATCH 38/53] lint --- .../robot_server/persistence/_migrations/v7_to_v8.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index a92da18268c..93aac6bd5a0 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -74,8 +74,12 @@ def _add_missing_indexes(dest_transaction: sqlalchemy.engine.Connection) -> None dest_transaction.execute( "CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run);" ) - dest_transaction.execute("CREATE INDEX ix_run_command_command_intent ON run_command (command_intent);") - dest_transaction.execute("CREATE INDEX ix_data_files_source ON data_files (source);") + dest_transaction.execute( + "CREATE INDEX ix_run_command_command_intent ON run_command (command_intent);" + ) + dest_transaction.execute( + "CREATE INDEX ix_data_files_source ON data_files (source);" + ) def _migrate_command_table_with_new_command_error_col_and_command_status( From 085d4bb0f460674c902a7733fd92af7d8502720d Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 16:23:33 -0500 Subject: [PATCH 39/53] change conversion into enum --- robot-server/robot_server/runs/run_store.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index b1fc792b42c..a4701ec22aa 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -17,6 +17,7 @@ CommandIntent, ErrorOccurrence, CommandErrorSlice, + CommandStatus, ) from opentrons.protocol_engine.commands import Command from opentrons.protocol_engine.types import RunTimeParameter @@ -190,7 +191,7 @@ def update_run_state( if command.error else None, "command_status": _convert_commands_status_to_sql_command_status( - command.status.value + command.status ), }, ) @@ -817,16 +818,14 @@ def _convert_state_to_sql_values( def _convert_commands_status_to_sql_command_status( - status: str, + status: CommandStatus, ) -> CommandStatusSQLEnum: match status: - case "queued": + case CommandStatus.QUEUED: return CommandStatusSQLEnum.QUEUED - case "running": + case CommandStatus.RUNNING: return CommandStatusSQLEnum.RUNNING - case "failed": + case CommandStatus.FAILED: return CommandStatusSQLEnum.FAILED - case "succeeded": + case CommandStatus.SUCCEEDED: return CommandStatusSQLEnum.SUCCEEDED - case _: - assert False, "command status is unknown" From ec64c120fb005b3744c2a1774a6ce3e276b00e4d Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 20:54:17 -0500 Subject: [PATCH 40/53] fixing merge conflicts - WIP --- .../persistence/tables/schema_8.py | 2 +- robot-server/tests/persistence/test_tables.py | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 1944d491bda..907094dc9be 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -217,7 +217,7 @@ class CommandStatusSQLEnum(enum.Enum): sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), sqlalchemy.Column("command", sqlalchemy.String, nullable=False), - sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=False, index=True), + sqlalchemy.Column("command_intent", sqlalchemy.String, index=True), sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True), sqlalchemy.Column( "command_status", diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index 211838975f7..1f68fb9c637 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -80,6 +80,9 @@ CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) """, """ + CREATE INDEX ix_data_files_source ON data_files (source) + """, + """ CREATE TABLE run ( id VARCHAR NOT NULL, created_at DATETIME NOT NULL, @@ -109,12 +112,11 @@ index_in_run INTEGER NOT NULL, command_id VARCHAR NOT NULL, command VARCHAR NOT NULL, - command_error VARCHAR, - command_status VARCHAR(9) NOT NULL, command_intent VARCHAR, + command_error VARCHAR, + command_status VARCHAR(9), PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id), - CONSTRAINT commandstatussqlenum CHECK (command_status IN ('queued', 'running', 'succeeded', 'failed')) + FOREIGN KEY(run_id) REFERENCES run (id) ) """, """ @@ -690,12 +692,12 @@ def _normalize_statement(statement: str) -> str: ("metadata", "expected_statements"), [ (latest_metadata, EXPECTED_STATEMENTS_LATEST), - (schema_7.metadata, EXPECTED_STATEMENTS_V7), - (schema_6.metadata, EXPECTED_STATEMENTS_V6), - (schema_5.metadata, EXPECTED_STATEMENTS_V5), - (schema_4.metadata, EXPECTED_STATEMENTS_V4), - (schema_3.metadata, EXPECTED_STATEMENTS_V3), - (schema_2.metadata, EXPECTED_STATEMENTS_V2), + # (schema_7.metadata, EXPECTED_STATEMENTS_V7), + # (schema_6.metadata, EXPECTED_STATEMENTS_V6), + # (schema_5.metadata, EXPECTED_STATEMENTS_V5), + # (schema_4.metadata, EXPECTED_STATEMENTS_V4), + # (schema_3.metadata, EXPECTED_STATEMENTS_V3), + # (schema_2.metadata, EXPECTED_STATEMENTS_V2), ], ) def test_creating_from_metadata_emits_expected_statements( From b285c993cad90e1e139cbc485c1d9b144598009b Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 13 Nov 2024 21:24:22 -0500 Subject: [PATCH 41/53] WIP - fix test_tables failing --- .../persistence/tables/schema_8.py | 4 +- robot-server/tests/persistence/test_tables.py | 1013 +++++++++-------- 2 files changed, 509 insertions(+), 508 deletions(-) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 907094dc9be..ba02583cf19 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -217,7 +217,7 @@ class CommandStatusSQLEnum(enum.Enum): sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), sqlalchemy.Column("command", sqlalchemy.String, nullable=False), - sqlalchemy.Column("command_intent", sqlalchemy.String, index=True), + sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=True), sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True), sqlalchemy.Column( "command_status", @@ -225,9 +225,9 @@ class CommandStatusSQLEnum(enum.Enum): CommandStatusSQLEnum, values_callable=lambda obj: [e.value for e in obj], validate_strings=True, + nullable=False, create_constraint=True, ), - nullable=False, ), sqlalchemy.Index( "ix_run_run_id_command_id", # An arbitrary name for the index. diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index 1f68fb9c637..a28bf03da99 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -114,9 +114,10 @@ command VARCHAR NOT NULL, command_intent VARCHAR, command_error VARCHAR, - command_status VARCHAR(9), + command_status VARCHAR(9) NOT NULL, PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) + FOREIGN KEY(run_id) REFERENCES run (id), + CONSTRAINT commandstatussqlenum CHECK (command_status IN ('queued', 'running', 'succeeded', 'failed')) ) """, """ @@ -166,510 +167,510 @@ ] -EXPECTED_STATEMENTS_V7 = [ - """ - CREATE TABLE protocol ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_key VARCHAR, - protocol_kind VARCHAR(14) NOT NULL, - PRIMARY KEY (id), - CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) - ) - """, - """ - CREATE TABLE analysis ( - id VARCHAR NOT NULL, - protocol_id VARCHAR NOT NULL, - analyzer_version VARCHAR NOT NULL, - completed_analysis VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE analysis_primitive_rtp_table ( - row_id INTEGER NOT NULL, - analysis_id VARCHAR NOT NULL, - parameter_variable_name VARCHAR NOT NULL, - parameter_type VARCHAR(5) NOT NULL, - parameter_value VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(analysis_id) REFERENCES analysis (id), - CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) - ) - """, - """ - CREATE TABLE analysis_csv_rtp_table ( - row_id INTEGER NOT NULL, - analysis_id VARCHAR NOT NULL, - parameter_variable_name VARCHAR NOT NULL, - file_id VARCHAR, - PRIMARY KEY (row_id), - FOREIGN KEY(analysis_id) REFERENCES analysis (id), - FOREIGN KEY(file_id) REFERENCES data_files (id) - ) - """, - """ - CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) - """, - """ - CREATE TABLE run ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_id VARCHAR, - state_summary VARCHAR, - engine_status VARCHAR, - _updated_at DATETIME, - run_time_parameters VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE action ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - action_type VARCHAR NOT NULL, - run_id VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE TABLE run_command ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - index_in_run INTEGER NOT NULL, - command_id VARCHAR NOT NULL, - command VARCHAR NOT NULL, - command_intent VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) - """, - """ - CREATE INDEX ix_data_files_source ON data_files (source) - """, - """ - CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) - """, - """ - CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) - """, - """ - CREATE TABLE data_files ( - id VARCHAR NOT NULL, - name VARCHAR NOT NULL, - file_hash VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - source VARCHAR(9) NOT NULL, - PRIMARY KEY (id), - CONSTRAINT datafilesourcesqlenum CHECK (source IN ('uploaded', 'generated')) - ) - """, - """ - CREATE TABLE run_csv_rtp_table ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - parameter_variable_name VARCHAR NOT NULL, - file_id VARCHAR, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id), - FOREIGN KEY(file_id) REFERENCES data_files (id) - ) - """, - """ - CREATE TABLE boolean_setting ( - "key" VARCHAR(21) NOT NULL, - value BOOLEAN NOT NULL, - PRIMARY KEY ("key"), - CONSTRAINT booleansettingkey CHECK ("key" IN ('enable_error_recovery')) - ) - """, -] - - -EXPECTED_STATEMENTS_V6 = [ - """ - CREATE TABLE protocol ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_key VARCHAR, - protocol_kind VARCHAR(14) NOT NULL, - PRIMARY KEY (id), - CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) - ) - """, - """ - CREATE TABLE analysis ( - id VARCHAR NOT NULL, - protocol_id VARCHAR NOT NULL, - analyzer_version VARCHAR NOT NULL, - completed_analysis VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE analysis_primitive_rtp_table ( - row_id INTEGER NOT NULL, - analysis_id VARCHAR NOT NULL, - parameter_variable_name VARCHAR NOT NULL, - parameter_type VARCHAR(5) NOT NULL, - parameter_value VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(analysis_id) REFERENCES analysis (id), - CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) - ) - """, - """ - CREATE TABLE analysis_csv_rtp_table ( - row_id INTEGER NOT NULL, - analysis_id VARCHAR NOT NULL, - parameter_variable_name VARCHAR NOT NULL, - file_id VARCHAR, - PRIMARY KEY (row_id), - FOREIGN KEY(analysis_id) REFERENCES analysis (id), - FOREIGN KEY(file_id) REFERENCES data_files (id) - ) - """, - """ - CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) - """, - """ - CREATE TABLE run ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_id VARCHAR, - state_summary VARCHAR, - engine_status VARCHAR, - _updated_at DATETIME, - run_time_parameters VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE action ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - action_type VARCHAR NOT NULL, - run_id VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE TABLE run_command ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - index_in_run INTEGER NOT NULL, - command_id VARCHAR NOT NULL, - command VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) - """, - """ - CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) - """, - """ - CREATE TABLE data_files ( - id VARCHAR NOT NULL, - name VARCHAR NOT NULL, - file_hash VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - PRIMARY KEY (id) - ) - """, - """ - CREATE TABLE run_csv_rtp_table ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - parameter_variable_name VARCHAR NOT NULL, - file_id VARCHAR, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id), - FOREIGN KEY(file_id) REFERENCES data_files (id) - ) - """, -] - - -EXPECTED_STATEMENTS_V5 = [ - """ - CREATE TABLE protocol ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_key VARCHAR, - protocol_kind VARCHAR, - PRIMARY KEY (id) - ) - """, - """ - CREATE TABLE analysis ( - id VARCHAR NOT NULL, - protocol_id VARCHAR NOT NULL, - analyzer_version VARCHAR NOT NULL, - completed_analysis VARCHAR NOT NULL, - run_time_parameter_values_and_defaults VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) - """, - """ - CREATE TABLE run ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_id VARCHAR, - state_summary VARCHAR, - engine_status VARCHAR, - _updated_at DATETIME, - run_time_parameters VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE action ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - action_type VARCHAR NOT NULL, - run_id VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE TABLE run_command ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - index_in_run INTEGER NOT NULL, - command_id VARCHAR NOT NULL, - command VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) - """, - """ - CREATE TABLE data_files ( - id VARCHAR NOT NULL, - name VARCHAR NOT NULL, - file_hash VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - PRIMARY KEY (id) - ) - """, -] - - -EXPECTED_STATEMENTS_V4 = [ - """ - CREATE TABLE protocol ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_key VARCHAR, - PRIMARY KEY (id) - ) - """, - """ - CREATE TABLE analysis ( - id VARCHAR NOT NULL, - protocol_id VARCHAR NOT NULL, - analyzer_version VARCHAR NOT NULL, - completed_analysis VARCHAR NOT NULL, - run_time_parameter_values_and_defaults VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) - """, - """ - CREATE TABLE run ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_id VARCHAR, - state_summary VARCHAR, - engine_status VARCHAR, - _updated_at DATETIME, - run_time_parameters VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE action ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - action_type VARCHAR NOT NULL, - run_id VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE TABLE run_command ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - index_in_run INTEGER NOT NULL, - command_id VARCHAR NOT NULL, - command VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) - """, -] - - -EXPECTED_STATEMENTS_V3 = [ - """ - CREATE TABLE protocol ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_key VARCHAR, - PRIMARY KEY (id) - ) - """, - """ - CREATE TABLE analysis ( - id VARCHAR NOT NULL, - protocol_id VARCHAR NOT NULL, - analyzer_version VARCHAR NOT NULL, - completed_analysis VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) - """, - """ - CREATE TABLE run ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_id VARCHAR, - state_summary VARCHAR, - engine_status VARCHAR, - _updated_at DATETIME, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE action ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - action_type VARCHAR NOT NULL, - run_id VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE TABLE run_command ( - row_id INTEGER NOT NULL, - run_id VARCHAR NOT NULL, - index_in_run INTEGER NOT NULL, - command_id VARCHAR NOT NULL, - command VARCHAR NOT NULL, - PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) - """, - """ - CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) - """, -] - - -EXPECTED_STATEMENTS_V2 = [ - """ - CREATE TABLE migration ( - id INTEGER NOT NULL, - created_at DATETIME NOT NULL, - version INTEGER NOT NULL, - PRIMARY KEY (id) - ) - """, - """ - CREATE TABLE protocol ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_key VARCHAR, - PRIMARY KEY (id) - ) - """, - """ - CREATE TABLE analysis ( - id VARCHAR NOT NULL, - protocol_id VARCHAR NOT NULL, - analyzer_version VARCHAR NOT NULL, - completed_analysis BLOB NOT NULL, - completed_analysis_as_document VARCHAR, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) - """, - """ - CREATE TABLE run ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - protocol_id VARCHAR, - state_summary BLOB, - commands BLOB, - engine_status VARCHAR, - _updated_at DATETIME, - PRIMARY KEY (id), - FOREIGN KEY(protocol_id) REFERENCES protocol (id) - ) - """, - """ - CREATE TABLE action ( - id VARCHAR NOT NULL, - created_at DATETIME NOT NULL, - action_type VARCHAR NOT NULL, - run_id VARCHAR NOT NULL, - PRIMARY KEY (id), - FOREIGN KEY(run_id) REFERENCES run (id) - ) - """, -] +# EXPECTED_STATEMENTS_V7 = [ +# """ +# CREATE TABLE protocol ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_key VARCHAR, +# protocol_kind VARCHAR(14) NOT NULL, +# PRIMARY KEY (id), +# CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) +# ) +# """, +# """ +# CREATE TABLE analysis ( +# id VARCHAR NOT NULL, +# protocol_id VARCHAR NOT NULL, +# analyzer_version VARCHAR NOT NULL, +# completed_analysis VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE analysis_primitive_rtp_table ( +# row_id INTEGER NOT NULL, +# analysis_id VARCHAR NOT NULL, +# parameter_variable_name VARCHAR NOT NULL, +# parameter_type VARCHAR(5) NOT NULL, +# parameter_value VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(analysis_id) REFERENCES analysis (id), +# CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) +# ) +# """, +# """ +# CREATE TABLE analysis_csv_rtp_table ( +# row_id INTEGER NOT NULL, +# analysis_id VARCHAR NOT NULL, +# parameter_variable_name VARCHAR NOT NULL, +# file_id VARCHAR, +# PRIMARY KEY (row_id), +# FOREIGN KEY(analysis_id) REFERENCES analysis (id), +# FOREIGN KEY(file_id) REFERENCES data_files (id) +# ) +# """, +# """ +# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) +# """, +# """ +# CREATE TABLE run ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_id VARCHAR, +# state_summary VARCHAR, +# engine_status VARCHAR, +# _updated_at DATETIME, +# run_time_parameters VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE action ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# action_type VARCHAR NOT NULL, +# run_id VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE TABLE run_command ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# index_in_run INTEGER NOT NULL, +# command_id VARCHAR NOT NULL, +# command VARCHAR NOT NULL, +# command_intent VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) +# """, +# """ +# CREATE INDEX ix_data_files_source ON data_files (source) +# """, +# """ +# CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) +# """, +# """ +# CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) +# """, +# """ +# CREATE TABLE data_files ( +# id VARCHAR NOT NULL, +# name VARCHAR NOT NULL, +# file_hash VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# source VARCHAR(9) NOT NULL, +# PRIMARY KEY (id), +# CONSTRAINT datafilesourcesqlenum CHECK (source IN ('uploaded', 'generated')) +# ) +# """, +# """ +# CREATE TABLE run_csv_rtp_table ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# parameter_variable_name VARCHAR NOT NULL, +# file_id VARCHAR, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id), +# FOREIGN KEY(file_id) REFERENCES data_files (id) +# ) +# """, +# """ +# CREATE TABLE boolean_setting ( +# "key" VARCHAR(21) NOT NULL, +# value BOOLEAN NOT NULL, +# PRIMARY KEY ("key"), +# CONSTRAINT booleansettingkey CHECK ("key" IN ('enable_error_recovery')) +# ) +# """, +# ] +# +# +# EXPECTED_STATEMENTS_V6 = [ +# """ +# CREATE TABLE protocol ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_key VARCHAR, +# protocol_kind VARCHAR(14) NOT NULL, +# PRIMARY KEY (id), +# CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) +# ) +# """, +# """ +# CREATE TABLE analysis ( +# id VARCHAR NOT NULL, +# protocol_id VARCHAR NOT NULL, +# analyzer_version VARCHAR NOT NULL, +# completed_analysis VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE analysis_primitive_rtp_table ( +# row_id INTEGER NOT NULL, +# analysis_id VARCHAR NOT NULL, +# parameter_variable_name VARCHAR NOT NULL, +# parameter_type VARCHAR(5) NOT NULL, +# parameter_value VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(analysis_id) REFERENCES analysis (id), +# CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) +# ) +# """, +# """ +# CREATE TABLE analysis_csv_rtp_table ( +# row_id INTEGER NOT NULL, +# analysis_id VARCHAR NOT NULL, +# parameter_variable_name VARCHAR NOT NULL, +# file_id VARCHAR, +# PRIMARY KEY (row_id), +# FOREIGN KEY(analysis_id) REFERENCES analysis (id), +# FOREIGN KEY(file_id) REFERENCES data_files (id) +# ) +# """, +# """ +# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) +# """, +# """ +# CREATE TABLE run ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_id VARCHAR, +# state_summary VARCHAR, +# engine_status VARCHAR, +# _updated_at DATETIME, +# run_time_parameters VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE action ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# action_type VARCHAR NOT NULL, +# run_id VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE TABLE run_command ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# index_in_run INTEGER NOT NULL, +# command_id VARCHAR NOT NULL, +# command VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) +# """, +# """ +# CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) +# """, +# """ +# CREATE TABLE data_files ( +# id VARCHAR NOT NULL, +# name VARCHAR NOT NULL, +# file_hash VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# PRIMARY KEY (id) +# ) +# """, +# """ +# CREATE TABLE run_csv_rtp_table ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# parameter_variable_name VARCHAR NOT NULL, +# file_id VARCHAR, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id), +# FOREIGN KEY(file_id) REFERENCES data_files (id) +# ) +# """, +# ] +# +# +# EXPECTED_STATEMENTS_V5 = [ +# """ +# CREATE TABLE protocol ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_key VARCHAR, +# protocol_kind VARCHAR, +# PRIMARY KEY (id) +# ) +# """, +# """ +# CREATE TABLE analysis ( +# id VARCHAR NOT NULL, +# protocol_id VARCHAR NOT NULL, +# analyzer_version VARCHAR NOT NULL, +# completed_analysis VARCHAR NOT NULL, +# run_time_parameter_values_and_defaults VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) +# """, +# """ +# CREATE TABLE run ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_id VARCHAR, +# state_summary VARCHAR, +# engine_status VARCHAR, +# _updated_at DATETIME, +# run_time_parameters VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE action ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# action_type VARCHAR NOT NULL, +# run_id VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE TABLE run_command ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# index_in_run INTEGER NOT NULL, +# command_id VARCHAR NOT NULL, +# command VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) +# """, +# """ +# CREATE TABLE data_files ( +# id VARCHAR NOT NULL, +# name VARCHAR NOT NULL, +# file_hash VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# PRIMARY KEY (id) +# ) +# """, +# ] +# +# +# EXPECTED_STATEMENTS_V4 = [ +# """ +# CREATE TABLE protocol ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_key VARCHAR, +# PRIMARY KEY (id) +# ) +# """, +# """ +# CREATE TABLE analysis ( +# id VARCHAR NOT NULL, +# protocol_id VARCHAR NOT NULL, +# analyzer_version VARCHAR NOT NULL, +# completed_analysis VARCHAR NOT NULL, +# run_time_parameter_values_and_defaults VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) +# """, +# """ +# CREATE TABLE run ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_id VARCHAR, +# state_summary VARCHAR, +# engine_status VARCHAR, +# _updated_at DATETIME, +# run_time_parameters VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE action ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# action_type VARCHAR NOT NULL, +# run_id VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE TABLE run_command ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# index_in_run INTEGER NOT NULL, +# command_id VARCHAR NOT NULL, +# command VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) +# """, +# ] +# +# +# EXPECTED_STATEMENTS_V3 = [ +# """ +# CREATE TABLE protocol ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_key VARCHAR, +# PRIMARY KEY (id) +# ) +# """, +# """ +# CREATE TABLE analysis ( +# id VARCHAR NOT NULL, +# protocol_id VARCHAR NOT NULL, +# analyzer_version VARCHAR NOT NULL, +# completed_analysis VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) +# """, +# """ +# CREATE TABLE run ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_id VARCHAR, +# state_summary VARCHAR, +# engine_status VARCHAR, +# _updated_at DATETIME, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE action ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# action_type VARCHAR NOT NULL, +# run_id VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE TABLE run_command ( +# row_id INTEGER NOT NULL, +# run_id VARCHAR NOT NULL, +# index_in_run INTEGER NOT NULL, +# command_id VARCHAR NOT NULL, +# command VARCHAR NOT NULL, +# PRIMARY KEY (row_id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) +# """, +# """ +# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) +# """, +# ] +# +# +# EXPECTED_STATEMENTS_V2 = [ +# """ +# CREATE TABLE migration ( +# id INTEGER NOT NULL, +# created_at DATETIME NOT NULL, +# version INTEGER NOT NULL, +# PRIMARY KEY (id) +# ) +# """, +# """ +# CREATE TABLE protocol ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_key VARCHAR, +# PRIMARY KEY (id) +# ) +# """, +# """ +# CREATE TABLE analysis ( +# id VARCHAR NOT NULL, +# protocol_id VARCHAR NOT NULL, +# analyzer_version VARCHAR NOT NULL, +# completed_analysis BLOB NOT NULL, +# completed_analysis_as_document VARCHAR, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) +# """, +# """ +# CREATE TABLE run ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# protocol_id VARCHAR, +# state_summary BLOB, +# commands BLOB, +# engine_status VARCHAR, +# _updated_at DATETIME, +# PRIMARY KEY (id), +# FOREIGN KEY(protocol_id) REFERENCES protocol (id) +# ) +# """, +# """ +# CREATE TABLE action ( +# id VARCHAR NOT NULL, +# created_at DATETIME NOT NULL, +# action_type VARCHAR NOT NULL, +# run_id VARCHAR NOT NULL, +# PRIMARY KEY (id), +# FOREIGN KEY(run_id) REFERENCES run (id) +# ) +# """, +# ] def _normalize_statement(statement: str) -> str: From 3a29bfdb3d3a8ce46e719aaf0076c6148c5be6e0 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 20 Nov 2024 13:23:56 -0500 Subject: [PATCH 42/53] update test table --- robot-server/tests/persistence/test_tables.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index a28bf03da99..b57389273f2 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -44,6 +44,9 @@ ) """, """ + CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind), + """ + """ CREATE TABLE analysis ( id VARCHAR NOT NULL, protocol_id VARCHAR NOT NULL, @@ -726,7 +729,8 @@ def record_statement( normalized_actual = [_normalize_statement(s) for s in actual_statements] normalized_expected = [_normalize_statement(s) for s in expected_statements] - + print(set(normalized_actual)) + print(set(normalized_expected)) # Compare ignoring order. SQLAlchemy appears to emit CREATE INDEX statements in a # nondeterministic order that varies across runs. Although statement order # theoretically matters, it's unlikely to matter in practice for our purposes here. From dca8269536a42363a8691a7b9b8c4e0dc3f9768a Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 20 Nov 2024 15:05:40 -0500 Subject: [PATCH 43/53] Revert "update test table" This reverts commit 3a29bfdb3d3a8ce46e719aaf0076c6148c5be6e0. --- robot-server/tests/persistence/test_tables.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index b57389273f2..a28bf03da99 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -44,9 +44,6 @@ ) """, """ - CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind), - """ - """ CREATE TABLE analysis ( id VARCHAR NOT NULL, protocol_id VARCHAR NOT NULL, @@ -729,8 +726,7 @@ def record_statement( normalized_actual = [_normalize_statement(s) for s in actual_statements] normalized_expected = [_normalize_statement(s) for s in expected_statements] - print(set(normalized_actual)) - print(set(normalized_expected)) + # Compare ignoring order. SQLAlchemy appears to emit CREATE INDEX statements in a # nondeterministic order that varies across runs. Although statement order # theoretically matters, it's unlikely to matter in practice for our purposes here. From 4d91d6f3e35e90e7c87a6647823c386595af5e76 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Thu, 14 Nov 2024 09:47:18 -0500 Subject: [PATCH 44/53] Fix schema_8.py after merge. Ensure it's based on a copy-paste of the latest (post-merge) schema_7.py. --- .../persistence/tables/schema_8.py | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index ba02583cf19..78a60eaa1a5 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -1,4 +1,4 @@ -"""v7 of our SQLite schema.""" +"""v8 of our SQLite schema.""" import enum import sqlalchemy @@ -217,7 +217,14 @@ class CommandStatusSQLEnum(enum.Enum): sqlalchemy.Column("index_in_run", sqlalchemy.Integer, nullable=False), sqlalchemy.Column("command_id", sqlalchemy.String, nullable=False), sqlalchemy.Column("command", sqlalchemy.String, nullable=False), - sqlalchemy.Column("command_intent", sqlalchemy.String, nullable=True), + sqlalchemy.Column( + "command_intent", + sqlalchemy.String, + # nullable=True to match the underlying SQL, which is nullable because of a bug + # in the migration that introduced this column. This is not intended to ever be + # null in practice. + nullable=True, + ), sqlalchemy.Column("command_error", sqlalchemy.String, nullable=True), sqlalchemy.Column( "command_status", @@ -279,10 +286,16 @@ class CommandStatusSQLEnum(enum.Enum): DataFileSourceSQLEnum, values_callable=lambda obj: [e.value for e in obj], validate_strings=True, - create_constraint=True, + # create_constraint=False to match the underlying SQL, which omits + # the constraint because of a bug in the migration that introduced this + # column. This is not intended to ever have values other than those in + # DataFileSourceSQLEnum. + create_constraint=False, ), - index=True, - nullable=False, + # nullable=True to match the underlying SQL, which is nullable because of a bug + # in the migration that introduced this column. This is not intended to ever be + # null in practice. + nullable=True, ), ) From fd78ad320c604ed2f9af4374c67a1c13763c8f58 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Thu, 14 Nov 2024 09:29:46 -0500 Subject: [PATCH 45/53] Fix up test_tables.py after merge. Ensure EXPECTED_STATEMENTS_V8 is based on a copy-paste of the latest EXPECTED_STATEMENTS_V7. --- robot-server/tests/persistence/test_tables.py | 1025 ++++++++--------- 1 file changed, 508 insertions(+), 517 deletions(-) diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index a28bf03da99..4939cd8726b 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -18,6 +18,7 @@ schema_5, schema_6, schema_7, + schema_8, ) # The statements that we expect to emit when we create a fresh database. @@ -80,9 +81,6 @@ CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) """, """ - CREATE INDEX ix_data_files_source ON data_files (source) - """, - """ CREATE TABLE run ( id VARCHAR NOT NULL, created_at DATETIME NOT NULL, @@ -116,8 +114,7 @@ command_error VARCHAR, command_status VARCHAR(9) NOT NULL, PRIMARY KEY (row_id), - FOREIGN KEY(run_id) REFERENCES run (id), - CONSTRAINT commandstatussqlenum CHECK (command_status IN ('queued', 'running', 'succeeded', 'failed')) + FOREIGN KEY(run_id) REFERENCES run (id) ) """, """ @@ -127,13 +124,136 @@ CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) """, """ + CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run) + """, + """ CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) """, """ - CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run) + CREATE TABLE data_files ( + id VARCHAR NOT NULL, + name VARCHAR NOT NULL, + file_hash VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + source VARCHAR(9), + PRIMARY KEY (id) + ) """, """ - CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) + CREATE TABLE run_csv_rtp_table ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + file_id VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id), + FOREIGN KEY(file_id) REFERENCES data_files (id) + ) + """, + """ + CREATE TABLE boolean_setting ( + "key" VARCHAR(21) NOT NULL, + value BOOLEAN NOT NULL, + PRIMARY KEY ("key"), + CONSTRAINT booleansettingkey CHECK ("key" IN ('enable_error_recovery')) + ) + """, +] + + +EXPECTED_STATEMENTS_V8 = EXPECTED_STATEMENTS_LATEST + + +EXPECTED_STATEMENTS_V7 = [ + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + protocol_kind VARCHAR(14) NOT NULL, + PRIMARY KEY (id), + CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE analysis_primitive_rtp_table ( + row_id INTEGER NOT NULL, + analysis_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + parameter_type VARCHAR(5) NOT NULL, + parameter_value VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(analysis_id) REFERENCES analysis (id), + CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) + ) + """, + """ + CREATE TABLE analysis_csv_rtp_table ( + row_id INTEGER NOT NULL, + analysis_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + file_id VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(analysis_id) REFERENCES analysis (id), + FOREIGN KEY(file_id) REFERENCES data_files (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary VARCHAR, + engine_status VARCHAR, + _updated_at DATETIME, + run_time_parameters VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE TABLE run_command ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + index_in_run INTEGER NOT NULL, + command_id VARCHAR NOT NULL, + command VARCHAR NOT NULL, + command_intent VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) + """, + """ + CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) """, """ CREATE TABLE data_files ( @@ -167,510 +287,380 @@ ] -# EXPECTED_STATEMENTS_V7 = [ -# """ -# CREATE TABLE protocol ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_key VARCHAR, -# protocol_kind VARCHAR(14) NOT NULL, -# PRIMARY KEY (id), -# CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) -# ) -# """, -# """ -# CREATE TABLE analysis ( -# id VARCHAR NOT NULL, -# protocol_id VARCHAR NOT NULL, -# analyzer_version VARCHAR NOT NULL, -# completed_analysis VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE analysis_primitive_rtp_table ( -# row_id INTEGER NOT NULL, -# analysis_id VARCHAR NOT NULL, -# parameter_variable_name VARCHAR NOT NULL, -# parameter_type VARCHAR(5) NOT NULL, -# parameter_value VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(analysis_id) REFERENCES analysis (id), -# CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) -# ) -# """, -# """ -# CREATE TABLE analysis_csv_rtp_table ( -# row_id INTEGER NOT NULL, -# analysis_id VARCHAR NOT NULL, -# parameter_variable_name VARCHAR NOT NULL, -# file_id VARCHAR, -# PRIMARY KEY (row_id), -# FOREIGN KEY(analysis_id) REFERENCES analysis (id), -# FOREIGN KEY(file_id) REFERENCES data_files (id) -# ) -# """, -# """ -# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) -# """, -# """ -# CREATE TABLE run ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_id VARCHAR, -# state_summary VARCHAR, -# engine_status VARCHAR, -# _updated_at DATETIME, -# run_time_parameters VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE action ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# action_type VARCHAR NOT NULL, -# run_id VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE TABLE run_command ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# index_in_run INTEGER NOT NULL, -# command_id VARCHAR NOT NULL, -# command VARCHAR NOT NULL, -# command_intent VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) -# """, -# """ -# CREATE INDEX ix_data_files_source ON data_files (source) -# """, -# """ -# CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) -# """, -# """ -# CREATE INDEX ix_run_command_command_intent ON run_command (command_intent) -# """, -# """ -# CREATE TABLE data_files ( -# id VARCHAR NOT NULL, -# name VARCHAR NOT NULL, -# file_hash VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# source VARCHAR(9) NOT NULL, -# PRIMARY KEY (id), -# CONSTRAINT datafilesourcesqlenum CHECK (source IN ('uploaded', 'generated')) -# ) -# """, -# """ -# CREATE TABLE run_csv_rtp_table ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# parameter_variable_name VARCHAR NOT NULL, -# file_id VARCHAR, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id), -# FOREIGN KEY(file_id) REFERENCES data_files (id) -# ) -# """, -# """ -# CREATE TABLE boolean_setting ( -# "key" VARCHAR(21) NOT NULL, -# value BOOLEAN NOT NULL, -# PRIMARY KEY ("key"), -# CONSTRAINT booleansettingkey CHECK ("key" IN ('enable_error_recovery')) -# ) -# """, -# ] -# -# -# EXPECTED_STATEMENTS_V6 = [ -# """ -# CREATE TABLE protocol ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_key VARCHAR, -# protocol_kind VARCHAR(14) NOT NULL, -# PRIMARY KEY (id), -# CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) -# ) -# """, -# """ -# CREATE TABLE analysis ( -# id VARCHAR NOT NULL, -# protocol_id VARCHAR NOT NULL, -# analyzer_version VARCHAR NOT NULL, -# completed_analysis VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE analysis_primitive_rtp_table ( -# row_id INTEGER NOT NULL, -# analysis_id VARCHAR NOT NULL, -# parameter_variable_name VARCHAR NOT NULL, -# parameter_type VARCHAR(5) NOT NULL, -# parameter_value VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(analysis_id) REFERENCES analysis (id), -# CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) -# ) -# """, -# """ -# CREATE TABLE analysis_csv_rtp_table ( -# row_id INTEGER NOT NULL, -# analysis_id VARCHAR NOT NULL, -# parameter_variable_name VARCHAR NOT NULL, -# file_id VARCHAR, -# PRIMARY KEY (row_id), -# FOREIGN KEY(analysis_id) REFERENCES analysis (id), -# FOREIGN KEY(file_id) REFERENCES data_files (id) -# ) -# """, -# """ -# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) -# """, -# """ -# CREATE TABLE run ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_id VARCHAR, -# state_summary VARCHAR, -# engine_status VARCHAR, -# _updated_at DATETIME, -# run_time_parameters VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE action ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# action_type VARCHAR NOT NULL, -# run_id VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE TABLE run_command ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# index_in_run INTEGER NOT NULL, -# command_id VARCHAR NOT NULL, -# command VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) -# """, -# """ -# CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) -# """, -# """ -# CREATE TABLE data_files ( -# id VARCHAR NOT NULL, -# name VARCHAR NOT NULL, -# file_hash VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# PRIMARY KEY (id) -# ) -# """, -# """ -# CREATE TABLE run_csv_rtp_table ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# parameter_variable_name VARCHAR NOT NULL, -# file_id VARCHAR, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id), -# FOREIGN KEY(file_id) REFERENCES data_files (id) -# ) -# """, -# ] -# -# -# EXPECTED_STATEMENTS_V5 = [ -# """ -# CREATE TABLE protocol ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_key VARCHAR, -# protocol_kind VARCHAR, -# PRIMARY KEY (id) -# ) -# """, -# """ -# CREATE TABLE analysis ( -# id VARCHAR NOT NULL, -# protocol_id VARCHAR NOT NULL, -# analyzer_version VARCHAR NOT NULL, -# completed_analysis VARCHAR NOT NULL, -# run_time_parameter_values_and_defaults VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) -# """, -# """ -# CREATE TABLE run ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_id VARCHAR, -# state_summary VARCHAR, -# engine_status VARCHAR, -# _updated_at DATETIME, -# run_time_parameters VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE action ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# action_type VARCHAR NOT NULL, -# run_id VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE TABLE run_command ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# index_in_run INTEGER NOT NULL, -# command_id VARCHAR NOT NULL, -# command VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) -# """, -# """ -# CREATE TABLE data_files ( -# id VARCHAR NOT NULL, -# name VARCHAR NOT NULL, -# file_hash VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# PRIMARY KEY (id) -# ) -# """, -# ] -# -# -# EXPECTED_STATEMENTS_V4 = [ -# """ -# CREATE TABLE protocol ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_key VARCHAR, -# PRIMARY KEY (id) -# ) -# """, -# """ -# CREATE TABLE analysis ( -# id VARCHAR NOT NULL, -# protocol_id VARCHAR NOT NULL, -# analyzer_version VARCHAR NOT NULL, -# completed_analysis VARCHAR NOT NULL, -# run_time_parameter_values_and_defaults VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) -# """, -# """ -# CREATE TABLE run ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_id VARCHAR, -# state_summary VARCHAR, -# engine_status VARCHAR, -# _updated_at DATETIME, -# run_time_parameters VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE action ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# action_type VARCHAR NOT NULL, -# run_id VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE TABLE run_command ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# index_in_run INTEGER NOT NULL, -# command_id VARCHAR NOT NULL, -# command VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) -# """, -# ] -# -# -# EXPECTED_STATEMENTS_V3 = [ -# """ -# CREATE TABLE protocol ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_key VARCHAR, -# PRIMARY KEY (id) -# ) -# """, -# """ -# CREATE TABLE analysis ( -# id VARCHAR NOT NULL, -# protocol_id VARCHAR NOT NULL, -# analyzer_version VARCHAR NOT NULL, -# completed_analysis VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) -# """, -# """ -# CREATE TABLE run ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_id VARCHAR, -# state_summary VARCHAR, -# engine_status VARCHAR, -# _updated_at DATETIME, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE action ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# action_type VARCHAR NOT NULL, -# run_id VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE TABLE run_command ( -# row_id INTEGER NOT NULL, -# run_id VARCHAR NOT NULL, -# index_in_run INTEGER NOT NULL, -# command_id VARCHAR NOT NULL, -# command VARCHAR NOT NULL, -# PRIMARY KEY (row_id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) -# """, -# """ -# CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) -# """, -# ] -# -# -# EXPECTED_STATEMENTS_V2 = [ -# """ -# CREATE TABLE migration ( -# id INTEGER NOT NULL, -# created_at DATETIME NOT NULL, -# version INTEGER NOT NULL, -# PRIMARY KEY (id) -# ) -# """, -# """ -# CREATE TABLE protocol ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_key VARCHAR, -# PRIMARY KEY (id) -# ) -# """, -# """ -# CREATE TABLE analysis ( -# id VARCHAR NOT NULL, -# protocol_id VARCHAR NOT NULL, -# analyzer_version VARCHAR NOT NULL, -# completed_analysis BLOB NOT NULL, -# completed_analysis_as_document VARCHAR, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) -# """, -# """ -# CREATE TABLE run ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# protocol_id VARCHAR, -# state_summary BLOB, -# commands BLOB, -# engine_status VARCHAR, -# _updated_at DATETIME, -# PRIMARY KEY (id), -# FOREIGN KEY(protocol_id) REFERENCES protocol (id) -# ) -# """, -# """ -# CREATE TABLE action ( -# id VARCHAR NOT NULL, -# created_at DATETIME NOT NULL, -# action_type VARCHAR NOT NULL, -# run_id VARCHAR NOT NULL, -# PRIMARY KEY (id), -# FOREIGN KEY(run_id) REFERENCES run (id) -# ) -# """, -# ] +EXPECTED_STATEMENTS_V6 = [ + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + protocol_kind VARCHAR(14) NOT NULL, + PRIMARY KEY (id), + CONSTRAINT protocolkindsqlenum CHECK (protocol_kind IN ('standard', 'quick-transfer')) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE analysis_primitive_rtp_table ( + row_id INTEGER NOT NULL, + analysis_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + parameter_type VARCHAR(5) NOT NULL, + parameter_value VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(analysis_id) REFERENCES analysis (id), + CONSTRAINT primitiveparamsqlenum CHECK (parameter_type IN ('int', 'float', 'bool', 'str')) + ) + """, + """ + CREATE TABLE analysis_csv_rtp_table ( + row_id INTEGER NOT NULL, + analysis_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + file_id VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(analysis_id) REFERENCES analysis (id), + FOREIGN KEY(file_id) REFERENCES data_files (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary VARCHAR, + engine_status VARCHAR, + _updated_at DATETIME, + run_time_parameters VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE TABLE run_command ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + index_in_run INTEGER NOT NULL, + command_id VARCHAR NOT NULL, + command VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) + """, + """ + CREATE INDEX ix_protocol_protocol_kind ON protocol (protocol_kind) + """, + """ + CREATE TABLE data_files ( + id VARCHAR NOT NULL, + name VARCHAR NOT NULL, + file_hash VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + PRIMARY KEY (id) + ) + """, + """ + CREATE TABLE run_csv_rtp_table ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + parameter_variable_name VARCHAR NOT NULL, + file_id VARCHAR, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id), + FOREIGN KEY(file_id) REFERENCES data_files (id) + ) + """, +] + + +EXPECTED_STATEMENTS_V5 = [ + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + protocol_kind VARCHAR, + PRIMARY KEY (id) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis VARCHAR NOT NULL, + run_time_parameter_values_and_defaults VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary VARCHAR, + engine_status VARCHAR, + _updated_at DATETIME, + run_time_parameters VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE TABLE run_command ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + index_in_run INTEGER NOT NULL, + command_id VARCHAR NOT NULL, + command VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) + """, + """ + CREATE TABLE data_files ( + id VARCHAR NOT NULL, + name VARCHAR NOT NULL, + file_hash VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + PRIMARY KEY (id) + ) + """, +] + + +EXPECTED_STATEMENTS_V4 = [ + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + PRIMARY KEY (id) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis VARCHAR NOT NULL, + run_time_parameter_values_and_defaults VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary VARCHAR, + engine_status VARCHAR, + _updated_at DATETIME, + run_time_parameters VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE TABLE run_command ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + index_in_run INTEGER NOT NULL, + command_id VARCHAR NOT NULL, + command VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) + """, +] + + +EXPECTED_STATEMENTS_V3 = [ + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + PRIMARY KEY (id) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary VARCHAR, + engine_status VARCHAR, + _updated_at DATETIME, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE TABLE run_command ( + row_id INTEGER NOT NULL, + run_id VARCHAR NOT NULL, + index_in_run INTEGER NOT NULL, + command_id VARCHAR NOT NULL, + command VARCHAR NOT NULL, + PRIMARY KEY (row_id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_command_id ON run_command (run_id, command_id) + """, + """ + CREATE UNIQUE INDEX ix_run_run_id_index_in_run ON run_command (run_id, index_in_run) + """, +] + + +EXPECTED_STATEMENTS_V2 = [ + """ + CREATE TABLE migration ( + id INTEGER NOT NULL, + created_at DATETIME NOT NULL, + version INTEGER NOT NULL, + PRIMARY KEY (id) + ) + """, + """ + CREATE TABLE protocol ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_key VARCHAR, + PRIMARY KEY (id) + ) + """, + """ + CREATE TABLE analysis ( + id VARCHAR NOT NULL, + protocol_id VARCHAR NOT NULL, + analyzer_version VARCHAR NOT NULL, + completed_analysis BLOB NOT NULL, + completed_analysis_as_document VARCHAR, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE INDEX ix_analysis_protocol_id ON analysis (protocol_id) + """, + """ + CREATE TABLE run ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + protocol_id VARCHAR, + state_summary BLOB, + commands BLOB, + engine_status VARCHAR, + _updated_at DATETIME, + PRIMARY KEY (id), + FOREIGN KEY(protocol_id) REFERENCES protocol (id) + ) + """, + """ + CREATE TABLE action ( + id VARCHAR NOT NULL, + created_at DATETIME NOT NULL, + action_type VARCHAR NOT NULL, + run_id VARCHAR NOT NULL, + PRIMARY KEY (id), + FOREIGN KEY(run_id) REFERENCES run (id) + ) + """, +] def _normalize_statement(statement: str) -> str: @@ -693,12 +683,13 @@ def _normalize_statement(statement: str) -> str: ("metadata", "expected_statements"), [ (latest_metadata, EXPECTED_STATEMENTS_LATEST), - # (schema_7.metadata, EXPECTED_STATEMENTS_V7), - # (schema_6.metadata, EXPECTED_STATEMENTS_V6), - # (schema_5.metadata, EXPECTED_STATEMENTS_V5), - # (schema_4.metadata, EXPECTED_STATEMENTS_V4), - # (schema_3.metadata, EXPECTED_STATEMENTS_V3), - # (schema_2.metadata, EXPECTED_STATEMENTS_V2), + (schema_8.metadata, EXPECTED_STATEMENTS_V8), + (schema_7.metadata, EXPECTED_STATEMENTS_V7), + (schema_6.metadata, EXPECTED_STATEMENTS_V6), + (schema_5.metadata, EXPECTED_STATEMENTS_V5), + (schema_4.metadata, EXPECTED_STATEMENTS_V4), + (schema_3.metadata, EXPECTED_STATEMENTS_V3), + (schema_2.metadata, EXPECTED_STATEMENTS_V2), ], ) def test_creating_from_metadata_emits_expected_statements( From b59601ee4feed10c6d38e9c591f03b2bd5272517 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 20 Nov 2024 12:44:25 -0500 Subject: [PATCH 46/53] Remove attempts to fix prior migrations, for now. --- .../robot_server/persistence/_migrations/v7_to_v8.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 93aac6bd5a0..5299298c5f1 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -71,15 +71,11 @@ def add_column( def _add_missing_indexes(dest_transaction: sqlalchemy.engine.Connection) -> None: + # todo(2024-11-20): Probably add the indexes missing from prior migrations here. + # https://opentrons.atlassian.net/browse/EXEC-827 dest_transaction.execute( "CREATE UNIQUE INDEX ix_run_run_id_command_status_index_in_run ON run_command (run_id, command_status, index_in_run);" ) - dest_transaction.execute( - "CREATE INDEX ix_run_command_command_intent ON run_command (command_intent);" - ) - dest_transaction.execute( - "CREATE INDEX ix_data_files_source ON data_files (source);" - ) def _migrate_command_table_with_new_command_error_col_and_command_status( From 6496d3c385fbc40d1f984db7bfccb477ff6537d1 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 20 Nov 2024 13:23:56 -0500 Subject: [PATCH 47/53] Make command_status column nullable to match what the migration does. --- robot-server/robot_server/persistence/tables/schema_8.py | 4 +++- robot-server/tests/persistence/test_tables.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 78a60eaa1a5..603d10dd483 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -232,7 +232,9 @@ class CommandStatusSQLEnum(enum.Enum): CommandStatusSQLEnum, values_callable=lambda obj: [e.value for e in obj], validate_strings=True, - nullable=False, + # nullable=True because it was easier for the migration to add the column + # this way. This is not intended to ever be null in practice. + nullable=True, create_constraint=True, ), ), diff --git a/robot-server/tests/persistence/test_tables.py b/robot-server/tests/persistence/test_tables.py index 4939cd8726b..6363ed8f47f 100644 --- a/robot-server/tests/persistence/test_tables.py +++ b/robot-server/tests/persistence/test_tables.py @@ -112,7 +112,7 @@ command VARCHAR NOT NULL, command_intent VARCHAR, command_error VARCHAR, - command_status VARCHAR(9) NOT NULL, + command_status VARCHAR(9), PRIMARY KEY (row_id), FOREIGN KEY(run_id) REFERENCES run (id) ) From 11aa280bf20ee5d73980b8dbcc6233ae95bda185 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 20 Nov 2024 14:22:26 -0500 Subject: [PATCH 48/53] Remove enum constraint, for now. --- robot-server/robot_server/persistence/tables/schema_8.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/robot-server/robot_server/persistence/tables/schema_8.py b/robot-server/robot_server/persistence/tables/schema_8.py index 603d10dd483..c92dd4645c7 100644 --- a/robot-server/robot_server/persistence/tables/schema_8.py +++ b/robot-server/robot_server/persistence/tables/schema_8.py @@ -235,7 +235,11 @@ class CommandStatusSQLEnum(enum.Enum): # nullable=True because it was easier for the migration to add the column # this way. This is not intended to ever be null in practice. nullable=True, - create_constraint=True, + # todo(mm, 2024-11-20): We want create_constraint=True here. Something + # about the way we compare SQL in test_tables.py is making that difficult-- + # even when we correctly add the constraint in the migration, the SQL + # doesn't compare equal to what create_constraint=True here would emit. + create_constraint=False, ), ), sqlalchemy.Index( From 6669e61c70baae09312b7b5053b008ee4dfd6deb Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Thu, 21 Nov 2024 10:46:50 -0500 Subject: [PATCH 49/53] Update robot-server/robot_server/persistence/_migrations/v7_to_v8.py Co-authored-by: Max Marrone --- robot-server/robot_server/persistence/_migrations/v7_to_v8.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 5299298c5f1..6130753904a 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -90,7 +90,7 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( new_command_error = ( # Account for old_row.command["error"] being NULL. None - if "error" not in row.command or data["error"] == None # noqa: E711 + if "error" not in row.command or data["error"] is None else json.dumps(data["error"]) ) # parse json as enum From ce3fbb38a343446f450393f0b88cc25c6667cddc Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Thu, 21 Nov 2024 10:56:08 -0500 Subject: [PATCH 50/53] Update robot-server/robot_server/runs/run_store.py Co-authored-by: Max Marrone --- robot-server/robot_server/runs/run_store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index a4701ec22aa..75c1f485361 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -615,7 +615,6 @@ def get_commands_errors_slice( actual_cursor = max(0, min(actual_cursor, count_result - 1)) select_slice = ( sqlalchemy.select( - run_command_table.c.index_in_run, run_command_table.c.command_error, ) .where( From e0aacbd474277d69b1f575046b53298bf1846efe Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Thu, 21 Nov 2024 11:40:15 -0500 Subject: [PATCH 51/53] select subquery and minor fixes --- .../persistence/_migrations/v7_to_v8.py | 2 +- .../persistence/tables/__init__.py | 2 ++ robot-server/robot_server/runs/run_store.py | 18 +++++++++++++----- robot-server/tests/runs/test_run_store.py | 2 ++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py index 6130753904a..a5ed950a8dc 100644 --- a/robot-server/robot_server/persistence/_migrations/v7_to_v8.py +++ b/robot-server/robot_server/persistence/_migrations/v7_to_v8.py @@ -88,7 +88,7 @@ def _migrate_command_table_with_new_command_error_col_and_command_status( for row in dest_transaction.execute(select_commands).all(): data = json.loads(row.command) new_command_error = ( - # Account for old_row.command["error"] being NULL. + # Account for old_row.command["error"] being null. None if "error" not in row.command or data["error"] is None else json.dumps(data["error"]) diff --git a/robot-server/robot_server/persistence/tables/__init__.py b/robot-server/robot_server/persistence/tables/__init__.py index 390b381bfed..56e149d6dfd 100644 --- a/robot-server/robot_server/persistence/tables/__init__.py +++ b/robot-server/robot_server/persistence/tables/__init__.py @@ -17,6 +17,7 @@ ProtocolKindSQLEnum, BooleanSettingKey, DataFileSourceSQLEnum, + CommandStatusSQLEnum, ) @@ -36,4 +37,5 @@ "ProtocolKindSQLEnum", "BooleanSettingKey", "DataFileSourceSQLEnum", + "CommandStatusSQLEnum", ] diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 75c1f485361..27b028cd037 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -45,7 +45,7 @@ from .action_models import RunAction, RunActionType from .run_models import RunNotFoundError -from ..persistence.tables.schema_8 import CommandStatusSQLEnum +from ..persistence.tables import CommandStatusSQLEnum log = logging.getLogger(__name__) @@ -613,18 +613,26 @@ def get_commands_errors_slice( actual_cursor = cursor if cursor is not None else count_result - length # Clamp to [0, count_result). actual_cursor = max(0, min(actual_cursor, count_result - 1)) + select_command_errors = ( + sqlalchemy.select(run_command_table) + .where(run_command_table.c.command_error is not None) + .subquery() + ) select_slice = ( - sqlalchemy.select( - run_command_table.c.command_error, - ) + sqlalchemy.select(run_command_table.c.command_error) .where( and_( run_command_table.c.run_id == run_id, run_command_table.c.index_in_run >= actual_cursor, run_command_table.c.index_in_run < actual_cursor + length, - run_command_table.c.command_error is not None, ) ) + .join_from( + run_command_table, + select_command_errors, + onclause=run_command_table.c.index_in_run + == select_command_errors.c.index_in_run, + ) .order_by(run_command_table.c.index_in_run) ) slice_result = transaction.execute(select_slice).all() diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 2996512e672..2d6c4c07785 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -361,6 +361,8 @@ async def test_update_run_state_command_with_errors( cursor=0, ) + print(command_errors_result) + assert command_errors_result.commands_errors == [ item.error for item in protocol_commands_errors ] From b10e8a207e9a59fde22f3f199f44d53ae63e63e4 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 22 Nov 2024 13:22:06 -0500 Subject: [PATCH 52/53] fixed join logic and added logic to the tests --- robot-server/robot_server/runs/run_store.py | 18 +++++++++---- robot-server/tests/runs/test_run_store.py | 29 ++++++++++++++++++--- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 27b028cd037..80d4f6b2846 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -612,19 +612,27 @@ def get_commands_errors_slice( actual_cursor = cursor if cursor is not None else count_result - length # Clamp to [0, count_result). + print(actual_cursor) actual_cursor = max(0, min(actual_cursor, count_result - 1)) select_command_errors = ( - sqlalchemy.select(run_command_table) - .where(run_command_table.c.command_error is not None) + sqlalchemy.select(sqlalchemy.func.row_number().over().label('row_num'), run_command_table) + .where( + and_( + run_command_table.c.run_id == run_id, + run_command_table.c.command_status == CommandStatusSQLEnum.FAILED + ) + ) .subquery() ) + print(transaction.execute(select_command_errors).all()) + print(actual_cursor) + print(count_result) select_slice = ( sqlalchemy.select(run_command_table.c.command_error) .where( and_( - run_command_table.c.run_id == run_id, - run_command_table.c.index_in_run >= actual_cursor, - run_command_table.c.index_in_run < actual_cursor + length, + select_command_errors.c.row_num >= actual_cursor, + select_command_errors.c.row_num < actual_cursor + length, ) ) .join_from( diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 2d6c4c07785..9e526d4d12f 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -104,6 +104,15 @@ def protocol_commands() -> List[pe_commands.Command]: def protocol_commands_errors() -> List[pe_commands.Command]: """Get protocol commands errors list.""" return [ + pe_commands.WaitForResume( + id="pause-4", + key="command-key", + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2022, month=2, day=2), + params=pe_commands.WaitForResumeParams(message="hey world"), + result=pe_commands.WaitForResumeResult(), + intent=pe_commands.CommandIntent.PROTOCOL, + ), pe_commands.WaitForResume( id="pause-1", key="command-key", @@ -134,6 +143,15 @@ def protocol_commands_errors() -> List[pe_commands.Command]: detail="test details", ), ), + pe_commands.WaitForResume( + id="pause-3", + key="command-key", + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2022, month=2, day=2), + params=pe_commands.WaitForResumeParams(message="hey world"), + result=pe_commands.WaitForResumeResult(), + intent=pe_commands.CommandIntent.PROTOCOL, + ), ] @@ -335,6 +353,11 @@ async def test_update_run_state_command_with_errors( mock_runs_publisher: mock.Mock, ) -> None: """It should be able to update a run state to the store.""" + commands_with_errors = [ + command + for command in protocol_commands_errors + if command.status == pe_commands.CommandStatus.FAILED + ] action = RunAction( actionType=RunActionType.PLAY, createdAt=datetime(year=2022, month=2, day=2, tzinfo=timezone.utc), @@ -357,14 +380,14 @@ async def test_update_run_state_command_with_errors( subject.insert_action(run_id="run-id", action=action) command_errors_result = subject.get_commands_errors_slice( run_id="run-id", - length=len(protocol_commands_errors), - cursor=0, + length=5, + cursor=2, ) print(command_errors_result) assert command_errors_result.commands_errors == [ - item.error for item in protocol_commands_errors + item.error for item in commands_with_errors ] From 13fd3a8109b2c2a2660fed35e72fa1c39551c58f Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Fri, 22 Nov 2024 13:28:34 -0500 Subject: [PATCH 53/53] fixed logic of cursor --- robot-server/robot_server/runs/run_store.py | 16 +++++++++------- robot-server/tests/runs/test_run_store.py | 4 +--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 80d4f6b2846..eb706d6f88e 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -612,21 +612,23 @@ def get_commands_errors_slice( actual_cursor = cursor if cursor is not None else count_result - length # Clamp to [0, count_result). - print(actual_cursor) - actual_cursor = max(0, min(actual_cursor, count_result - 1)) + # cursor is 0 based index and row number starts from 1. + actual_cursor = max(0, min(actual_cursor, count_result - 1)) + 1 select_command_errors = ( - sqlalchemy.select(sqlalchemy.func.row_number().over().label('row_num'), run_command_table) + sqlalchemy.select( + sqlalchemy.func.row_number().over().label("row_num"), + run_command_table, + ) .where( and_( run_command_table.c.run_id == run_id, - run_command_table.c.command_status == CommandStatusSQLEnum.FAILED + run_command_table.c.command_status + == CommandStatusSQLEnum.FAILED, ) ) .subquery() ) - print(transaction.execute(select_command_errors).all()) - print(actual_cursor) - print(count_result) + select_slice = ( sqlalchemy.select(run_command_table.c.command_error) .where( diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 9e526d4d12f..ab8e5f10fdf 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -381,11 +381,9 @@ async def test_update_run_state_command_with_errors( command_errors_result = subject.get_commands_errors_slice( run_id="run-id", length=5, - cursor=2, + cursor=0, ) - print(command_errors_result) - assert command_errors_result.commands_errors == [ item.error for item in commands_with_errors ]