From 1b0afad4937d859b46d14effaaf9475becff8422 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Fri, 9 Aug 2024 10:13:42 -0400 Subject: [PATCH] fix(robot-server): Publish to `maintenance_runs/current_run` on initial `PLAY` action (#15943) When a client posts a PLAY action that starts a protocol run, any existing maintenance run ID is cleared from the server. This means that MQTT should update subscribers interested in the current maintenance run. --- .../maintenance_run_data_manager.py | 4 ++-- .../robot_server/runs/router/actions_router.py | 11 ++++++++++- .../robot_server/runs/run_controller.py | 6 +++++- .../publishers/maintenance_runs_publisher.py | 8 +++++++- robot-server/tests/runs/test_run_controller.py | 17 ++++++++++++++++- .../test_maintenance_runs_publisher.py | 2 +- 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py b/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py index 6f2cddd1835..76c355af72a 100644 --- a/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py +++ b/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py @@ -115,7 +115,7 @@ async def create( state_summary=state_summary, ) - await self._maintenance_runs_publisher.publish_current_maintenance_run() + await self._maintenance_runs_publisher.publish_current_maintenance_run_async() return maintenance_run_data @@ -157,7 +157,7 @@ async def delete(self, run_id: str) -> None: if run_id == self._run_orchestrator_store.current_run_id: await self._run_orchestrator_store.clear() - await self._maintenance_runs_publisher.publish_current_maintenance_run() + await self._maintenance_runs_publisher.publish_current_maintenance_run_async() else: raise MaintenanceRunNotFoundError(run_id=run_id) diff --git a/robot-server/robot_server/runs/router/actions_router.py b/robot-server/robot_server/runs/router/actions_router.py index 3323562fa22..a9bb535d542 100644 --- a/robot-server/robot_server/runs/router/actions_router.py +++ b/robot-server/robot_server/runs/router/actions_router.py @@ -30,7 +30,12 @@ from robot_server.maintenance_runs.dependencies import ( get_maintenance_run_orchestrator_store, ) -from robot_server.service.notifications import get_runs_publisher, RunsPublisher +from robot_server.service.notifications import ( + get_runs_publisher, + get_maintenance_runs_publisher, + RunsPublisher, + MaintenanceRunsPublisher, +) log = logging.getLogger(__name__) actions_router = APIRouter() @@ -49,6 +54,9 @@ async def get_run_controller( run_orchestrator_store: RunOrchestratorStore = Depends(get_run_orchestrator_store), run_store: RunStore = Depends(get_run_store), runs_publisher: RunsPublisher = Depends(get_runs_publisher), + maintenance_runs_publisher: MaintenanceRunsPublisher = Depends( + get_maintenance_runs_publisher + ), ) -> RunController: """Get a RunController for the current run. @@ -72,6 +80,7 @@ async def get_run_controller( run_orchestrator_store=run_orchestrator_store, run_store=run_store, runs_publisher=runs_publisher, + maintenance_runs_publisher=maintenance_runs_publisher, ) diff --git a/robot-server/robot_server/runs/run_controller.py b/robot-server/robot_server/runs/run_controller.py index 84299c34ded..7c8c0fe8b76 100644 --- a/robot-server/robot_server/runs/run_controller.py +++ b/robot-server/robot_server/runs/run_controller.py @@ -13,7 +13,7 @@ from opentrons.protocol_engine.types import DeckConfigurationType -from robot_server.service.notifications import RunsPublisher +from robot_server.service.notifications import RunsPublisher, MaintenanceRunsPublisher log = logging.getLogger(__name__) @@ -32,12 +32,14 @@ def __init__( run_orchestrator_store: RunOrchestratorStore, run_store: RunStore, runs_publisher: RunsPublisher, + maintenance_runs_publisher: MaintenanceRunsPublisher, ) -> None: self._run_id = run_id self._task_runner = task_runner self._run_orchestrator_store = run_orchestrator_store self._run_store = run_store self._runs_publisher = runs_publisher + self._maintenance_runs_publisher = maintenance_runs_publisher def create_action( self, @@ -80,6 +82,8 @@ def create_action( func=self._run_protocol_and_insert_result, deck_configuration=action_payload, ) + # Playing a protocol run terminates an existing maintenance run. + self._maintenance_runs_publisher.publish_current_maintenance_run() elif action_type == RunActionType.PAUSE: log.info(f'Pausing run "{self._run_id}".') diff --git a/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py b/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py index 1c382d37102..80285cb6452 100644 --- a/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py +++ b/robot-server/robot_server/service/notifications/publishers/maintenance_runs_publisher.py @@ -16,7 +16,7 @@ def __init__(self, client: NotificationClient) -> None: """Returns a configured Maintenance Runs Publisher.""" self._client = client - async def publish_current_maintenance_run( + async def publish_current_maintenance_run_async( self, ) -> None: """Publishes the equivalent of GET /maintenance_run/current_run""" @@ -24,6 +24,12 @@ async def publish_current_maintenance_run( topic=topics.MAINTENANCE_RUNS_CURRENT_RUN ) + def publish_current_maintenance_run( + self, + ) -> None: + """Publishes the equivalent of GET /maintenance_run/current_run""" + self._client.publish_advise_refetch(topic=topics.MAINTENANCE_RUNS_CURRENT_RUN) + _maintenance_runs_publisher_accessor: AppStateAccessor[ MaintenanceRunsPublisher diff --git a/robot-server/tests/runs/test_run_controller.py b/robot-server/tests/runs/test_run_controller.py index a901c988168..89aa79743a6 100644 --- a/robot-server/tests/runs/test_run_controller.py +++ b/robot-server/tests/runs/test_run_controller.py @@ -14,7 +14,7 @@ from opentrons.protocol_engine.types import RunTimeParameter, BooleanParameter from opentrons.protocol_runner import RunResult -from robot_server.service.notifications import RunsPublisher +from robot_server.service.notifications import RunsPublisher, MaintenanceRunsPublisher from robot_server.service.task_runner import TaskRunner from robot_server.runs.action_models import RunAction, RunActionType from robot_server.runs.run_orchestrator_store import RunOrchestratorStore @@ -48,6 +48,12 @@ def mock_runs_publisher(decoy: Decoy) -> RunsPublisher: return decoy.mock(cls=RunsPublisher) +@pytest.fixture() +def mock_maintenance_runs_publisher(decoy: Decoy) -> MaintenanceRunsPublisher: + """Get a mock RunsPublisher.""" + return decoy.mock(cls=MaintenanceRunsPublisher) + + @pytest.fixture def run_id() -> str: """A run identifier value.""" @@ -99,6 +105,7 @@ def subject( mock_run_store: RunStore, mock_task_runner: TaskRunner, mock_runs_publisher: RunsPublisher, + mock_maintenance_runs_publisher: MaintenanceRunsPublisher, ) -> RunController: """Get a RunController test subject.""" return RunController( @@ -107,6 +114,7 @@ def subject( run_store=mock_run_store, task_runner=mock_task_runner, runs_publisher=mock_runs_publisher, + maintenance_runs_publisher=mock_maintenance_runs_publisher, ) @@ -144,6 +152,7 @@ async def test_create_play_action_to_start( mock_run_store: RunStore, mock_task_runner: TaskRunner, mock_runs_publisher: RunsPublisher, + mock_maintenance_runs_publisher: MaintenanceRunsPublisher, engine_state_summary: StateSummary, run_time_parameters: List[RunTimeParameter], protocol_commands: List[pe_commands.Command], @@ -194,6 +203,12 @@ async def test_create_play_action_to_start( times=1, ) + # Verify maintenance run publication after background task execution + decoy.verify( + mock_maintenance_runs_publisher.publish_current_maintenance_run(), + times=1, + ) + def test_create_pause_action( decoy: Decoy, diff --git a/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py b/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py index fcc4cac5aac..bfdbbd26312 100644 --- a/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py +++ b/robot-server/tests/service/notifications/publishers/test_maintenance_runs_publisher.py @@ -24,7 +24,7 @@ async def test_publish_current_maintenance_run( notification_client: AsyncMock, maintenance_runs_publisher: MaintenanceRunsPublisher ) -> None: """It should publish a notify flag for maintenance runs.""" - await maintenance_runs_publisher.publish_current_maintenance_run() + await maintenance_runs_publisher.publish_current_maintenance_run_async() notification_client.publish_advise_refetch_async.assert_awaited_once_with( topic=topics.MAINTENANCE_RUNS_CURRENT_RUN )