From 9fa97bac4fe718e866fd4ea9b0ac9e63f8c87242 Mon Sep 17 00:00:00 2001 From: tamarzanzouri Date: Wed, 6 Nov 2024 13:43:15 -0500 Subject: [PATCH] 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