diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index 9ef7f78d0b93..2c0b4cc1925f 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -615,7 +615,7 @@ def set_and_start_queue_worker( ) self._queue_worker.start() - async def set_error_recovery_policy(self, policy: ErrorRecoveryPolicy) -> None: + def set_error_recovery_policy(self, policy: ErrorRecoveryPolicy) -> None: """Replace the run's error recovery policy with a new one.""" self._action_dispatcher.dispatch(SetErrorRecoveryPolicyAction(policy)) diff --git a/api/src/opentrons/protocol_runner/run_orchestrator.py b/api/src/opentrons/protocol_runner/run_orchestrator.py index 70fe62618ca2..c133a5d5b9fb 100644 --- a/api/src/opentrons/protocol_runner/run_orchestrator.py +++ b/api/src/opentrons/protocol_runner/run_orchestrator.py @@ -359,9 +359,9 @@ def get_deck_type(self) -> DeckType: """Get engine deck type.""" return self._protocol_engine.state_view.config.deck_type - async def create_error_recovery_policy(self, policy: ErrorRecoveryPolicy) -> None: + def set_error_recovery_policy(self, policy: ErrorRecoveryPolicy) -> None: """Create error recovery policy for the run.""" - await self._protocol_engine.set_error_recovery_policy(policy) + self._protocol_engine.set_error_recovery_policy(policy) async def command_generator(self) -> AsyncGenerator[str, None]: """Yield next command to execute.""" diff --git a/api/tests/opentrons/protocol_runner/test_run_orchestrator.py b/api/tests/opentrons/protocol_runner/test_run_orchestrator.py index 1c31dc0d501f..b3b6b6689562 100644 --- a/api/tests/opentrons/protocol_runner/test_run_orchestrator.py +++ b/api/tests/opentrons/protocol_runner/test_run_orchestrator.py @@ -528,5 +528,5 @@ async def test_create_error_recovery_policy( ) -> None: """Should call PE set_error_recovery_policy.""" policy = decoy.mock(cls=ErrorRecoveryPolicy) - await live_protocol_subject.create_error_recovery_policy(policy) - decoy.verify(await mock_protocol_engine.set_error_recovery_policy(policy)) + live_protocol_subject.set_error_recovery_policy(policy) + decoy.verify(mock_protocol_engine.set_error_recovery_policy(policy)) diff --git a/robot-server/robot_server/runs/error_recovery_models.py b/robot-server/robot_server/runs/error_recovery_models.py index 65f0e9552db1..c7a00c3ec634 100644 --- a/robot-server/robot_server/runs/error_recovery_models.py +++ b/robot-server/robot_server/runs/error_recovery_models.py @@ -1,5 +1,7 @@ """Request and response models for dealing with error recovery policies.""" from enum import Enum +from typing import List + from pydantic import BaseModel, Field @@ -57,7 +59,7 @@ class MatchCriteria(BaseModel): class ErrorRecoveryRule(BaseModel): - """Request/Response model for new error recovery rule creation.""" + """Model for new error recovery rule.""" matchCriteria: MatchCriteria = Field( ..., @@ -67,3 +69,12 @@ class ErrorRecoveryRule(BaseModel): ..., description="The specific recovery setting that will be in use if the type parameters match.", ) + + +class ErrorRecoveryPolicies(BaseModel): + """Request/Response model for new error recovery policy rules creation.""" + + policyRules: List[ErrorRecoveryRule] = Field( + ..., + description="The criteria that must be met for this rule to be applied.", + ) diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index a5606afdad89..7c3d4739f5c1 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -5,7 +5,7 @@ import logging from datetime import datetime from textwrap import dedent -from typing import Optional, Union, Callable, List +from typing import Optional, Union, Callable from typing_extensions import Literal from fastapi import APIRouter, Depends, status, Query @@ -45,7 +45,7 @@ get_run_auto_deleter, get_quick_transfer_run_auto_deleter, ) -from ..error_recovery_models import ErrorRecoveryRule +from ..error_recovery_models import ErrorRecoveryPolicies from robot_server.deck_configuration.fastapi_dependencies import ( get_deck_configuration_store, @@ -366,36 +366,37 @@ async def update_run( @PydanticResponse.wrap_route( - base_router.post, - path="/runs/{runId}/policies", - summary="Create run policies", + base_router.put, + path="/runs/{runId}/errorRecoveryPolicies", + summary="Set run policies", description=dedent( """ - Create run policies for error recovery. + When in error recovery, add the ability to ignore errors for commands. + The following rules will persist during the run. """ ), status_code=status.HTTP_201_CREATED, responses={ - status.HTTP_201_CREATED: {"model": SimpleBody[Run]}, + status.HTTP_200_OK: {"model": SimpleEmptyBody}, status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]}, }, ) -async def create_run_policies( +async def set_run_policies( runId: str, - request_body: Optional[RequestModel[List[ErrorRecoveryRule]]] = None, + request_body: RequestModel[ErrorRecoveryPolicies], run_data_manager: RunDataManager = Depends(get_run_data_manager), ) -> PydanticResponse[SimpleEmptyBody]: """Create run polices. Arguments: runId: Run ID pulled from URL. - request_body: Optional request body with run creation data. + request_body: Request body with run policies data. run_data_manager: Current and historical run data management. """ - policies = request_body.data if request_body is not None else None + policies = request_body.data.policyRules if policies: try: - await run_data_manager.create_policies(run_id=runId, policies=policies) + run_data_manager.set_policies(run_id=runId, policies=policies) except RunNotCurrentError as e: raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT) from e diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index 37e0d0b46feb..d602b9d33697 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -435,9 +435,7 @@ def get_all_commands_as_preserialized_list(self, run_id: str) -> List[str]: ) return self._run_store.get_all_commands_as_preserialized_list(run_id) - async def create_policies( - self, run_id: str, policies: List[ErrorRecoveryRule] - ) -> None: + def set_policies(self, run_id: str, policies: List[ErrorRecoveryRule]) -> None: """Create run policy rules for error recovery.""" if run_id != self._run_orchestrator_store.current_run_id: raise RunNotCurrentError( @@ -446,7 +444,7 @@ async def create_policies( policy = error_recovery_mapping.create_error_recovery_policy_from_rules( policies ) - await self._run_orchestrator_store.create_error_recovery_policy(policy=policy) + self._run_orchestrator_store.set_error_recovery_policy(policy=policy) def _get_state_summary(self, run_id: str) -> Union[StateSummary, BadStateSummary]: if run_id == self._run_orchestrator_store.current_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 2a2c7c815aac..650768329a33 100644 --- a/robot-server/robot_server/runs/run_orchestrator_store.py +++ b/robot-server/robot_server/runs/run_orchestrator_store.py @@ -360,11 +360,11 @@ def add_labware_definition(self, definition: LabwareDefinition) -> LabwareUri: """Add a new labware definition to state.""" return self.run_orchestrator.add_labware_definition(definition) - async def create_error_recovery_policy( + def set_error_recovery_policy( self, policy: error_recovery_policy.ErrorRecoveryPolicy ) -> None: """Create run policy rules for error recovery.""" - await self.run_orchestrator.create_error_recovery_policy(policy) + self.run_orchestrator.set_error_recovery_policy(policy) async def add_command_and_wait_for_interval( self, diff --git a/robot-server/tests/runs/router/test_base_router.py b/robot-server/tests/runs/router/test_base_router.py index 0e276aa9d375..f03a256eb51d 100644 --- a/robot-server/tests/runs/router/test_base_router.py +++ b/robot-server/tests/runs/router/test_base_router.py @@ -1,6 +1,4 @@ """Tests for base /runs routes.""" -from typing import List - import pytest from datetime import datetime from decoy import Decoy @@ -11,7 +9,7 @@ from opentrons.protocol_reader import ProtocolSource, JsonProtocolConfig from robot_server.errors.error_responses import ApiError -from robot_server.runs.error_recovery_models import ErrorRecoveryRule +from robot_server.runs.error_recovery_models import ErrorRecoveryPolicies from robot_server.service.json_api import ( RequestModel, SimpleBody, @@ -40,7 +38,7 @@ get_runs, remove_run, update_run, - create_run_policies, + set_run_policies, ) from robot_server.deck_configuration.store import DeckConfigurationStore @@ -579,41 +577,34 @@ async def test_create_policies( decoy: Decoy, mock_run_data_manager: RunDataManager ) -> None: """It should call RunDataManager create run policies.""" - policies = decoy.mock(cls=List[ErrorRecoveryRule]) - await create_run_policies( + policies = decoy.mock(cls=ErrorRecoveryPolicies) + await set_run_policies( runId="rud-id", - run_data_manager=mock_run_data_manager, request_body=RequestModel(data=policies), + run_data_manager=mock_run_data_manager, ) decoy.verify( - await mock_run_data_manager.create_policies(run_id="rud-id", policies=policies) + mock_run_data_manager.set_policies( + run_id="rud-id", policies=policies.policyRules + ) ) -# async def test_create_policies_raises_no_policies( -# decoy: Decoy, mock_run_data_manager: RunDataManager -# ) -> None: -# """It should raise that no policies were accepted.""" -# policies = decoy.mock(cls=List[ErrorRecoveryRule]) -# await create_run_policies(runId="rud-id", request_body=RequestModel(data=policies)) -# decoy.verify( -# mock_run_data_manager.create_policies(run_id="rud-id", policies=policies) -# ) - - async def test_create_policies_raises_not_active_run( decoy: Decoy, mock_run_data_manager: RunDataManager ) -> None: """It should raise that the run is not current.""" - policies = decoy.mock(cls=List[ErrorRecoveryRule]) + policies = decoy.mock(cls=ErrorRecoveryPolicies) decoy.when( - await mock_run_data_manager.create_policies(run_id="rud-id", policies=policies) + mock_run_data_manager.set_policies( + run_id="rud-id", policies=policies.policyRules + ) ).then_raise(RunNotCurrentError()) with pytest.raises(ApiError) as exc_info: - await create_run_policies( + await set_run_policies( runId="rud-id", - run_data_manager=mock_run_data_manager, request_body=RequestModel(data=policies), + run_data_manager=mock_run_data_manager, ) assert exc_info.value.status_code == 409 diff --git a/robot-server/tests/runs/test_error_recovery_mapping.py b/robot-server/tests/runs/test_error_recovery_mapping.py index a2e7575c3068..21195872d394 100644 --- a/robot-server/tests/runs/test_error_recovery_mapping.py +++ b/robot-server/tests/runs/test_error_recovery_mapping.py @@ -81,8 +81,10 @@ def test_create_error_recovery_policy_with_rules( robot_type="OT-3 Standard", deck_type=DeckType.OT3_STANDARD, ) - with pytest.raises(NotImplementedError): + assert ( policy(exampleConfig, mock_command, mock_error_data) + == ErrorRecoveryType.IGNORE_AND_CONTINUE + ) def test_create_error_recovery_policy_undefined_error( diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index e353eace543e..ec7a8bef69e2 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -1013,7 +1013,7 @@ async def test_create_policies_raises_run_not_current( "not-current-run-id" ) with pytest.raises(RunNotCurrentError): - await subject.create_policies( + subject.set_policies( run_id="run-id", policies=decoy.mock(cls=List[ErrorRecoveryRule]) ) @@ -1040,7 +1040,5 @@ async def test_create_policies_translates_and_calls_orchestrator( error_recovery_mapping.create_error_recovery_policy_from_rules(input_rules) ).then_return(expected_output) decoy.when(mock_run_orchestrator_store.current_run_id).then_return("run-id") - await subject.create_policies(run_id="run-id", policies=input_rules) - decoy.verify( - await mock_run_orchestrator_store.create_error_recovery_policy(expected_output) - ) + subject.set_policies(run_id="run-id", policies=input_rules) + decoy.verify(mock_run_orchestrator_store.set_error_recovery_policy(expected_output))