Skip to content

Commit

Permalink
fixed tests and removed props from engine_store
Browse files Browse the repository at this point in the history
  • Loading branch information
TamarZanzouri committed May 31, 2024
1 parent 98bc42e commit 9b9f11f
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 102 deletions.
18 changes: 11 additions & 7 deletions api/src/opentrons/protocol_runner/run_orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ def get_current_command(self) -> Optional[CommandPointer]:
return self._protocol_engine.state_view.commands.get_current()

def get_command_slice(
self,
cursor: Optional[int],
length: int,
self,
cursor: Optional[int],
length: int,
) -> CommandSlice:
"""Get a slice of run commands.
Expand All @@ -186,7 +186,7 @@ def get_command(self, command_id: str) -> Command:
command_id=command_id
)

def get_status(self) -> EngineStatus:
def get_run_status(self) -> EngineStatus:
"""Get the current execution status of the engine."""
return self._protocol_engine.state_view.commands.get_status()

Expand All @@ -206,7 +206,8 @@ def add_labware_definition(self, definition: LabwareDefinition) -> LabwareUri:
return self.run_orchestrator.engine.add_labware_definition(definition)

async def add_command_and_wait_for_interval(self, command: CommandCreate, wait_until_complete: bool = False,
timeout: Optional[int] = None, failed_command_id: Optional[str] = None) -> Command:
timeout: Optional[int] = None,
failed_command_id: Optional[str] = None) -> Command:
added_command = self._protocol_engine.add_command(request=command, failed_command_id=failed_command_id)
if wait_until_complete:
timeout_sec = None if timeout is None else timeout / 1000.0
Expand All @@ -217,5 +218,8 @@ async def add_command_and_wait_for_interval(self, command: CommandCreate, wait_u
def estop(self) -> None:
return self._protocol_engine.estop()

def use_attached_modules(self, modules_by_id: Dict[str, HardwareModuleAPI]) -> None:
self._protocol_engine.use_attached_modules(modules_by_id=modules_by_id)
async def use_attached_modules(self, modules_by_id: Dict[str, HardwareModuleAPI]) -> None:
await self._protocol_engine.use_attached_modules(modules_by_id=modules_by_id)

def get_protocol_runner(self) -> None:
raise NotImplementedError()
2 changes: 1 addition & 1 deletion robot-server/robot_server/commands/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ async def create_command(
Else, return immediately. Comes from a query parameter in the URL.
timeout: The maximum time, in seconds, to wait before returning.
Comes from a query parameter in the URL.
engine: The `ProtocolEngine` on which the command will be enqueued.
orchestrator: The `RunOrchestrator` handling engine for command to be enqueued.
"""
command_create = request_body.data.copy(update={"intent": CommandIntent.SETUP})
command = await orchestrator.add_command_and_wait_for_interval(
Expand Down
34 changes: 15 additions & 19 deletions robot-server/robot_server/runs/engine_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,36 +238,35 @@ async def create(
drop_tips_after_run=drop_tips_after_run,
)

runner = self.run_orchestrator.get_protocol_runner()
# FIXME(mm, 2022-12-21): These `await runner.load()`s introduce a
# concurrency hazard. If two requests simultaneously call this method,
# they will both "succeed" (with undefined results) instead of one
# raising EngineConflictError.
if isinstance(
self.run_orchestrator.get_protocol_runner(), PythonAndLegacyRunner
):
if isinstance(runner, PythonAndLegacyRunner):
assert (
protocol is not None
), "A Python protocol should have a protocol source file."
await self.run_orchestrator.runner.load(
await self.run_orchestrator.load(
protocol.source,
# Conservatively assume that we're re-running a protocol that
# was uploaded before we added stricter validation, and that
# doesn't conform to the new rules.
python_parse_mode=PythonParseMode.ALLOW_LEGACY_METADATA_AND_REQUIREMENTS,
run_time_param_values=run_time_param_values,
)
elif isinstance(self.run_orchestrator.runner, JsonRunner):
elif isinstance(runner, JsonRunner):
assert (
protocol is not None
), "A JSON protocol shouZld have a protocol source file."
await self.run_orchestrator.runner.load(protocol.source)
await self.run_orchestrator.load(protocol.source)
else:
self.run_orchestrator.runner.prepare()
self.run_orchestrator.prepare()

for offset in labware_offsets:
self.run_orchestrator.engine.add_labware_offset(offset)
self.run_orchestrator.add_labware_offset(offset)

return self.run_orchestrator.engine.state_view.get_summary()
return self.run_orchestrator.get_state_summary()

async def clear(self) -> RunResult:
"""Remove the persisted ProtocolEngine.
Expand All @@ -276,21 +275,18 @@ async def clear(self) -> RunResult:
EngineConflictError: The current runner/engine pair is not idle, so
they cannot be cleared.
"""
assert self.run_orchestrator is not None
engine = self.run_orchestrator.engine
runner = self.run_orchestrator.runner
if engine.state_view.commands.get_is_okay_to_clear():
await engine.finish(
if self.run_orchestrator.get_is_okay_to_clear():
await self.run_orchestrator.finish(
drop_tips_after_run=False,
set_run_status=False,
post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE,
)
else:
raise EngineConflictError("Current run is not idle or stopped.")

run_data = engine.state_view.get_summary()
commands = engine.state_view.commands.get_all()
run_time_parameters = runner.run_time_parameters if runner else []
run_data = self.run_orchestrator.get_state_summary()
commands = self.run_orchestrator.get_all_commands()
run_time_parameters = self.run_orchestrator.get_run_time_parameters()

self._run_orchestrator = None

Expand Down Expand Up @@ -323,7 +319,7 @@ async def finish(self, error: Optional[Exception]) -> None:
await self.run_orchestrator.finish(error=error)

def get_state_summary(self) -> StateSummary:
return self.run_orchestrator.get_summary()
return self.run_orchestrator.get_state_summary()

def get_loaded_labware_definitions(self) -> List[LabwareDefinition]:
return self.run_orchestrator.get_loaded_labware_definitions()
Expand Down Expand Up @@ -369,7 +365,7 @@ def get_is_run_terminal(self) -> bool:
return self.run_orchestrator.get_is_run_terminal()

def run_was_started(self) -> bool:
return self.run_orchestrator.was_run_started()
return self.run_orchestrator.run_has_started()

def add_labware_offset(self, request: LabwareOffsetCreate) -> LabwareOffset:
return self.run_orchestrator.add_labware_offset(request)
Expand Down
28 changes: 15 additions & 13 deletions robot-server/tests/commands/test_get_default_engine.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
"""Tests for robot_server.commands.get_default_engine."""
"""Tests for robot_server.commands.get_default_orchestrator."""
import pytest
from decoy import Decoy

from opentrons.hardware_control import HardwareControlAPI
from opentrons.hardware_control.modules import MagDeck, TempDeck
from opentrons.protocol_engine import ProtocolEngine
from opentrons.protocol_runner import RunOrchestrator

from robot_server.errors.error_responses import ApiError
from robot_server.runs.engine_store import EngineStore, EngineConflictError
from robot_server.modules.module_identifier import ModuleIdentifier, ModuleIdentity
from robot_server.commands.get_default_orchestrator import get_default_engine
from robot_server.commands.get_default_orchestrator import get_default_orchestrator


@pytest.fixture()
def protocol_engine(decoy: Decoy) -> ProtocolEngine:
def run_orchestrator(decoy: Decoy) -> RunOrchestrator:
"""Get a mocked out ProtocolEngine."""
return decoy.mock(cls=ProtocolEngine)
return decoy.mock(cls=RunOrchestrator)


@pytest.fixture()
Expand All @@ -30,11 +30,11 @@ def module_identifier(decoy: Decoy) -> ModuleIdentifier:
return decoy.mock(cls=ModuleIdentifier)


async def test_get_default_engine(
async def test_get_default_orchestrator(
decoy: Decoy,
engine_store: EngineStore,
hardware_api: HardwareControlAPI,
protocol_engine: ProtocolEngine,
run_orchestrator: RunOrchestrator,
module_identifier: ModuleIdentifier,
) -> None:
"""It should get a default engine with modules pre-loaded."""
Expand Down Expand Up @@ -63,30 +63,32 @@ async def test_get_default_engine(

decoy.when(hardware_api.attached_modules).then_return([mod_1, mod_2])

decoy.when(await engine_store.get_default_engine()).then_return(protocol_engine)
decoy.when(await engine_store.get_default_orchestrator()).then_return(
run_orchestrator
)

result = await get_default_engine(
result = await get_default_orchestrator(
engine_store=engine_store,
hardware_api=hardware_api,
module_identifier=module_identifier,
)

assert result is protocol_engine
assert result is run_orchestrator

decoy.verify(
await protocol_engine.use_attached_modules({"mod-1": mod_1, "mod-2": mod_2}),
await run_orchestrator.use_attached_modules({"mod-1": mod_1, "mod-2": mod_2}),
times=1,
)


async def test_raises_conflict(decoy: Decoy, engine_store: EngineStore) -> None:
"""It should raise a 409 conflict if the default engine is not availble."""
decoy.when(await engine_store.get_default_engine()).then_raise(
decoy.when(await engine_store.get_default_orchestrator()).then_raise(
EngineConflictError("oh no")
)

with pytest.raises(ApiError) as exc_info:
await get_default_engine(engine_store=engine_store)
await get_default_orchestrator(engine_store=engine_store)

assert exc_info.value.status_code == 409
assert exc_info.value.content["errors"][0]["id"] == "RunActive"
Loading

0 comments on commit 9b9f11f

Please sign in to comment.