Merge branch 'EXEC-302-when-a-command-fails-ignore-it-and-continue-wi…

…th-the-run' into EXEC-302-pe

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))
 

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."""

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))

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.",
+    )

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
 

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:

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,

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

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(

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))