Skip to content

Commit

Permalink
renaming, removed LiveRunner from create_runner. pr feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
TamarZanzouri committed May 16, 2024
1 parent 4da9372 commit b63f97c
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 82 deletions.
59 changes: 26 additions & 33 deletions api/src/opentrons/protocol_runner/protocol_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ async def run( # noqa: D102
def create_protocol_runner(
protocol_engine: ProtocolEngine,
hardware_api: HardwareControlAPI,
protocol_config: Optional[Union[JsonProtocolConfig, PythonProtocolConfig]] = None,
protocol_config: Union[JsonProtocolConfig, PythonProtocolConfig],
task_queue: Optional[TaskQueue] = None,
json_file_reader: Optional[JsonFileReader] = None,
json_translator: Optional[JsonTranslator] = None,
Expand All @@ -422,39 +422,32 @@ def create_protocol_runner(
legacy_executor: Optional[LegacyExecutor] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
) -> AnyRunner:
) -> Union[JsonRunner, PythonAndLegacyRunner]:
"""Create a protocol runner."""
if protocol_config:
if (
isinstance(protocol_config, JsonProtocolConfig)
and protocol_config.schema_version >= LEGACY_JSON_SCHEMA_VERSION_CUTOFF
):
return JsonRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
json_file_reader=json_file_reader,
json_translator=json_translator,
task_queue=task_queue,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)
else:
return PythonAndLegacyRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
task_queue=task_queue,
legacy_file_reader=legacy_file_reader,
legacy_context_creator=legacy_context_creator,
legacy_executor=legacy_executor,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)

return LiveRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
task_queue=task_queue,
)
if (
isinstance(protocol_config, JsonProtocolConfig)
and protocol_config.schema_version >= LEGACY_JSON_SCHEMA_VERSION_CUTOFF
):
return JsonRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
json_file_reader=json_file_reader,
json_translator=json_translator,
task_queue=task_queue,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)
else:
return PythonAndLegacyRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
task_queue=task_queue,
legacy_file_reader=legacy_file_reader,
legacy_context_creator=legacy_context_creator,
legacy_executor=legacy_executor,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)


async def _yield() -> None:
Expand Down
19 changes: 7 additions & 12 deletions api/src/opentrons/protocol_runner/run_orchestrator.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Engine/Runner provider."""
from __future__ import annotations
from typing import Optional, Union, cast
from typing import Optional, Union

from . import protocol_runner, AnyRunner
from ..hardware_control import HardwareControlAPI
Expand Down Expand Up @@ -75,17 +75,12 @@ def build_orchestrator(
)
json_or_python_runner = None
if protocol_config:
json_or_python_runner = cast( # or we can use type ignore
Union[
protocol_runner.JsonRunner, protocol_runner.PythonAndLegacyRunner
],
protocol_runner.create_protocol_runner(
protocol_config=protocol_config,
protocol_engine=protocol_engine,
hardware_api=hardware_api,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
),
json_or_python_runner = protocol_runner.create_protocol_runner(
protocol_config=protocol_config,
protocol_engine=protocol_engine,
hardware_api=hardware_api,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)
return cls(
run_id=run_id,
Expand Down
5 changes: 2 additions & 3 deletions api/tests/opentrons/protocol_runner/test_protocol_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
from decoy import Decoy, matchers
from pathlib import Path
from typing import List, cast, Optional, Union, Type
from typing import List, cast, Union, Type

from opentrons_shared_data.labware.labware_definition import LabwareDefinition
from opentrons_shared_data.protocol.models import ProtocolSchemaV6, ProtocolSchemaV7
Expand Down Expand Up @@ -173,7 +173,6 @@ def live_runner_subject(
(PythonProtocolConfig(api_version=APIVersion(2, 14)), PythonAndLegacyRunner),
(JsonProtocolConfig(schema_version=5), PythonAndLegacyRunner),
(PythonProtocolConfig(api_version=APIVersion(2, 13)), PythonAndLegacyRunner),
(None, LiveRunner),
],
)
def test_create_protocol_runner(
Expand All @@ -185,7 +184,7 @@ def test_create_protocol_runner(
legacy_file_reader: LegacyFileReader,
legacy_context_creator: LegacyContextCreator,
legacy_executor: LegacyExecutor,
config: Optional[Union[JsonProtocolConfig, PythonProtocolConfig]],
config: Union[JsonProtocolConfig, PythonProtocolConfig],
runner_type: Type[AnyRunner],
) -> None:
"""It should return protocol runner type depending on the config."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import pytest
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
from decoy import Decoy
from typing import Union, Optional
from typing import Union

from opentrons.protocols.api_support.types import APIVersion
from opentrons.protocol_engine import ProtocolEngine
Expand Down Expand Up @@ -103,8 +103,6 @@ def python_protocol_subject(
lazy_fixture("mock_protocol_python_runner"),
lazy_fixture("python_protocol_subject"),
),
(None, None, lazy_fixture("python_protocol_subject")),
(None, None, lazy_fixture("json_protocol_subject")),
],
)
def test_build_run_orchestrator_provider(
Expand All @@ -113,35 +111,17 @@ def test_build_run_orchestrator_provider(
subject: RunOrchestrator,
mock_protocol_engine: ProtocolEngine,
mock_hardware_api: HardwareAPI,
input_protocol_config: Optional[Union[PythonProtocolConfig, JsonProtocolConfig]],
input_protocol_config: Union[PythonProtocolConfig, JsonProtocolConfig],
mock_setup_runner: LiveRunner,
mock_fixit_runner: LiveRunner,
mock_protocol_runner: Optional[Union[PythonAndLegacyRunner, JsonRunner]],
mock_protocol_runner: Union[PythonAndLegacyRunner, JsonRunner],
) -> None:
"""Should get a RunOrchestrator instance."""
mock_create_runner_func = decoy.mock(func=protocol_runner.create_protocol_runner)
monkeypatch.setattr(
protocol_runner, "create_protocol_runner", mock_create_runner_func
)

decoy.when(
mock_create_runner_func(
protocol_engine=mock_protocol_engine,
hardware_api=mock_hardware_api,
post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run=True,
)
).then_return(mock_setup_runner)

decoy.when(
mock_create_runner_func(
protocol_engine=mock_protocol_engine,
hardware_api=mock_hardware_api,
post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run=True,
)
).then_return(mock_fixit_runner)

decoy.when(
mock_create_runner_func(
protocol_config=input_protocol_config,
Expand All @@ -150,7 +130,7 @@ def test_build_run_orchestrator_provider(
post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run=True,
)
).then_return(mock_fixit_runner)
).then_return(mock_protocol_runner)

result = subject.build_orchestrator(
protocol_engine=mock_protocol_engine,
Expand All @@ -159,3 +139,5 @@ def test_build_run_orchestrator_provider(
)

assert isinstance(result, RunOrchestrator)
assert isinstance(result._setup_runner, LiveRunner)
assert isinstance(result._fixit_runner, LiveRunner)
4 changes: 2 additions & 2 deletions robot-server/robot_server/runs/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
)

from .run_auto_deleter import RunAutoDeleter
from .engine_store import EngineStore, NoRunnerEnginePairError
from .engine_store import EngineStore, NoRunnerEngineError
from .run_store import RunStore
from .run_data_manager import RunDataManager
from robot_server.errors.robot_errors import (
Expand Down Expand Up @@ -131,7 +131,7 @@ async def get_is_okay_to_create_maintenance_run(
"""Whether a maintenance run can be created if a protocol run already exists."""
try:
protocol_run_state = engine_store.engine.state_view
except NoRunnerEnginePairError:
except NoRunnerEngineError:
return True
return (
not protocol_run_state.commands.has_been_played()
Expand Down
6 changes: 3 additions & 3 deletions robot-server/robot_server/runs/engine_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class EngineConflictError(RuntimeError):
"""


class NoRunnerEnginePairError(RuntimeError):
class NoRunnerEngineError(RuntimeError):
"""Raised if you try to get the current engine or runner while there is none."""


Expand Down Expand Up @@ -126,14 +126,14 @@ def __init__(
def engine(self) -> ProtocolEngine:
"""Get the "current" persisted ProtocolEngine."""
if self._run_orchestrator is None:
raise NoRunnerEnginePairError()
raise NoRunnerEngineError()
return self._run_orchestrator.engine

@property
def runner(self) -> AnyRunner:
"""Get the "current" persisted ProtocolRunner."""
if self._run_orchestrator is None:
raise NoRunnerEnginePairError()
raise NoRunnerEngineError()
return self._run_orchestrator.runner

@property
Expand Down
10 changes: 5 additions & 5 deletions robot-server/tests/runs/test_engine_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from robot_server.runs.engine_store import (
EngineStore,
EngineConflictError,
NoRunnerEnginePairError,
NoRunnerEngineError,
handle_estop_event,
)

Expand Down Expand Up @@ -182,10 +182,10 @@ async def test_clear_engine(subject: EngineStore) -> None:
assert subject.current_run_id is None
assert isinstance(result, RunResult)

with pytest.raises(NoRunnerEnginePairError):
with pytest.raises(NoRunnerEngineError):
subject.engine

with pytest.raises(NoRunnerEnginePairError):
with pytest.raises(NoRunnerEngineError):
subject.runner


Expand Down Expand Up @@ -221,9 +221,9 @@ async def test_clear_idle_engine(subject: EngineStore) -> None:
await subject.clear()

# TODO: test engine finish is called
with pytest.raises(NoRunnerEnginePairError):
with pytest.raises(NoRunnerEngineError):
subject.engine
with pytest.raises(NoRunnerEnginePairError):
with pytest.raises(NoRunnerEngineError):
subject.runner


Expand Down

0 comments on commit b63f97c

Please sign in to comment.