Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(api, robot-server): Manage runners via run orchestrator #15190

Merged
merged 51 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
347c193
initial copy of the queues into the runners
TamarZanzouri Apr 25, 2024
d75461b
WIP starting to move setting the queue to the runner
TamarZanzouri Apr 25, 2024
37e24e8
un commenting the command history and coping add to queue to runner
TamarZanzouri Apr 26, 2024
311a2c0
add_command logic and QueuedCommandAction managment
TamarZanzouri Apr 26, 2024
ad54fce
clean-up
TamarZanzouri Apr 28, 2024
a1e2511
cleanup
TamarZanzouri Apr 28, 2024
bc4131c
comments and renaming
TamarZanzouri Apr 29, 2024
27a1438
started creating the orchestrator
TamarZanzouri Apr 29, 2024
3c40e6b
add_command in orchestrator not complete
TamarZanzouri Apr 30, 2024
9cfaf4c
removed list of commands from runners
TamarZanzouri May 1, 2024
fa53544
cleanup and progress with direction
TamarZanzouri May 2, 2024
f5415ea
restructure
TamarZanzouri May 2, 2024
c47ea33
base tests
TamarZanzouri May 2, 2024
8716bd4
tests progress not done!
TamarZanzouri May 2, 2024
ef5e2a0
fixed order of parameters
TamarZanzouri May 6, 2024
aaca024
json specific
TamarZanzouri May 6, 2024
a2719c8
changed test to assert that the command was entered
TamarZanzouri May 7, 2024
3dd5ed8
added provider and tests. not complete
TamarZanzouri May 7, 2024
05df2f0
type cleanup
TamarZanzouri May 7, 2024
0136801
changed import of runners. prepare for monkeypatch
TamarZanzouri May 7, 2024
12f6fd1
added monkeypatch
TamarZanzouri May 7, 2024
b57b38c
moved build method inside orchestrator
TamarZanzouri May 7, 2024
d89186c
add command test
TamarZanzouri May 7, 2024
840e83f
changed to classmethod
TamarZanzouri May 8, 2024
7fb9f21
fixed return type from classmethod
TamarZanzouri May 8, 2024
6294a54
added abstract method and implementations
TamarZanzouri May 8, 2024
5be263b
fixed setting configs
TamarZanzouri May 8, 2024
fc4b912
moved monkeypatch into builer test
TamarZanzouri May 8, 2024
9fb1013
fixed failing tests
TamarZanzouri May 8, 2024
2467b88
split subjects to python and json
TamarZanzouri May 9, 2024
cf79e55
WIP engine_store using orchestrator
TamarZanzouri May 9, 2024
3317464
export and import orchestrator
TamarZanzouri May 10, 2024
e45663f
WIP navigate runner/engine via orchestrator
TamarZanzouri May 10, 2024
0a0bb5c
use orchestrator for runner and not optinal for run_id and runner
TamarZanzouri May 13, 2024
7962013
lint and thoughts
TamarZanzouri May 14, 2024
19fbee9
added missing load and prepare methods
TamarZanzouri May 14, 2024
5702c5c
fixed failing tests and added setup runner for default runner
TamarZanzouri May 14, 2024
4f8a49c
get_default_engine for stateless commands WIP
TamarZanzouri May 14, 2024
072c4e9
fixed get_default_engine
TamarZanzouri May 15, 2024
9e4a3e8
removed add_command changes
TamarZanzouri May 15, 2024
635a4e3
branch cleanup
TamarZanzouri May 15, 2024
1e630f5
pr clean-up
TamarZanzouri May 15, 2024
32a4957
run orchestrator engine and runner as prop
TamarZanzouri May 16, 2024
e85fca5
explicit type for runners
TamarZanzouri May 16, 2024
4da9372
docstrings
TamarZanzouri May 16, 2024
b63f97c
renaming, removed LiveRunner from create_runner. pr feedback.
TamarZanzouri May 16, 2024
3ed461b
docstring
TamarZanzouri May 16, 2024
61baa72
removed comments
TamarZanzouri May 16, 2024
da94cdf
cleanup
TamarZanzouri May 17, 2024
91c8a13
merge conflicts
TamarZanzouri May 17, 2024
62d43a8
fixed merge test fail
TamarZanzouri May 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/src/opentrons/protocol_runner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
AnyRunner,
)
from .create_simulating_runner import create_simulating_runner
from .run_orchestrator import RunOrchestrator

__all__ = [
"AbstractRunner",
Expand All @@ -23,4 +24,5 @@
"PythonAndLegacyRunner",
"LiveRunner",
"AnyRunner",
"RunOrchestrator",
]
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 @@ -416,9 +416,9 @@ async def run( # noqa: D102


def create_protocol_runner(
protocol_config: Optional[Union[JsonProtocolConfig, PythonProtocolConfig]],
protocol_engine: ProtocolEngine,
hardware_api: HardwareControlAPI,
protocol_config: Union[JsonProtocolConfig, PythonProtocolConfig],
task_queue: Optional[TaskQueue] = None,
json_file_reader: Optional[JsonFileReader] = None,
json_translator: Optional[JsonTranslator] = None,
Expand All @@ -427,39 +427,32 @@ def create_protocol_runner(
python_protocol_executor: Optional[PythonProtocolExecutor] = 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,
python_and_legacy_file_reader=python_and_legacy_file_reader,
protocol_context_creator=protocol_context_creator,
python_protocol_executor=python_protocol_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,
python_and_legacy_file_reader=python_and_legacy_file_reader,
protocol_context_creator=protocol_context_creator,
python_protocol_executor=python_protocol_executor,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)


async def _yield() -> None:
Expand Down
101 changes: 101 additions & 0 deletions api/src/opentrons/protocol_runner/run_orchestrator.py
DerekMaggio marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
"""Engine/Runner provider."""
from __future__ import annotations
from typing import Optional, Union

from . import protocol_runner, AnyRunner
from ..hardware_control import HardwareControlAPI
from ..protocol_engine import ProtocolEngine
from ..protocol_engine.types import PostRunHardwareState
from ..protocol_reader import JsonProtocolConfig, PythonProtocolConfig


class RunOrchestrator:
"""Provider for runners and associated protocol engine.

Build runners, manage command execution, run state and in-memory protocol engine associated to the runners.
"""

_protocol_runner: Optional[
Union[protocol_runner.JsonRunner, protocol_runner.PythonAndLegacyRunner, None]
]
_setup_runner: protocol_runner.LiveRunner
_fixit_runner: protocol_runner.LiveRunner
_hardware_api: HardwareControlAPI
_protocol_engine: ProtocolEngine

def __init__(
self,
protocol_engine: ProtocolEngine,
hardware_api: HardwareControlAPI,
fixit_runner: protocol_runner.LiveRunner,
setup_runner: protocol_runner.LiveRunner,
json_or_python_protocol_runner: Optional[
Union[protocol_runner.PythonAndLegacyRunner, protocol_runner.JsonRunner]
] = None,
run_id: Optional[str] = None,
) -> None:
"""Initialize a run orchestrator interface.

Arguments:
protocol_engine: Protocol engine instance.
hardware_api: Hardware control API instance.
fixit_runner: LiveRunner for fixit commands.
setup_runner: LiveRunner for setup commands.
json_or_python_protocol_runner: JsonRunner/PythonAndLegacyRunner for protocol commands.
run_id: run id if any, associated to the runner/engine.
"""
self.run_id = run_id
self._protocol_engine = protocol_engine
self._hardware_api = hardware_api
self._protocol_runner = json_or_python_protocol_runner
self._setup_runner = setup_runner
self._fixit_runner = fixit_runner

@property
def engine(self) -> ProtocolEngine:
"""Get the "current" persisted ProtocolEngine."""
return self._protocol_engine

@property
def runner(self) -> AnyRunner:
"""Get the "current" persisted ProtocolRunner."""
return self._protocol_runner or self._setup_runner
Comment on lines +59 to +62
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this property's return value need to change intelligently between runners depending on the run state? Like, it returns self._setup_runner while the run is in the setup phase, and self._fixit_runner while error recovery is active, and self._protocol_runner otherwise? Or am I misunderstanding the plan?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats not a bad idea. I was thinking about this more like:
if there is a protocol runner always return that, if there is no protocol runner then fixit_runner is not active and we are only able to use setup runner but! this property will probably vanish bc we are going to remove access directly with the runner and let the orchestrator handle the logic. let me know if this makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think this property goes away forever and the orchestrator itself offers semantically relevant api, like a method called run_setup_command and a method called run_fixit_command


@classmethod
def build_orchestrator(
cls,
protocol_engine: ProtocolEngine,
hardware_api: HardwareControlAPI,
protocol_config: Optional[
Union[JsonProtocolConfig, PythonProtocolConfig]
] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
run_id: Optional[str] = None,
) -> "RunOrchestrator":
"""Build a RunOrchestrator provider."""
setup_runner = protocol_runner.LiveRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
)
fixit_runner = protocol_runner.LiveRunner(
protocol_engine=protocol_engine,
hardware_api=hardware_api,
)
json_or_python_runner = None
if protocol_config:
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,
json_or_python_protocol_runner=json_or_python_runner,
setup_runner=setup_runner,
fixit_runner=fixit_runner,
hardware_api=hardware_api,
protocol_engine=protocol_engine,
)
4 changes: 2 additions & 2 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.labware.dev_types import (
Expand Down Expand Up @@ -186,7 +186,7 @@ def test_create_protocol_runner(
python_and_legacy_file_reader: PythonAndLegacyFileReader,
protocol_context_creator: ProtocolContextCreator,
python_protocol_executor: PythonProtocolExecutor,
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
@@ -0,0 +1,143 @@
"""Tests for the RunOrchestrator."""
import pytest
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
from decoy import Decoy
from typing import Union

from opentrons.protocols.api_support.types import APIVersion
from opentrons.protocol_engine import ProtocolEngine
from opentrons.protocol_engine.types import PostRunHardwareState
from opentrons.hardware_control import API as HardwareAPI
from opentrons.protocol_reader import JsonProtocolConfig, PythonProtocolConfig
from opentrons.protocol_runner.run_orchestrator import RunOrchestrator
from opentrons import protocol_runner
from opentrons.protocol_runner.protocol_runner import (
JsonRunner,
PythonAndLegacyRunner,
LiveRunner,
)


@pytest.fixture
def mock_protocol_python_runner(decoy: Decoy) -> PythonAndLegacyRunner:
"""Get a mocked out PythonAndLegacyRunner dependency."""
return decoy.mock(cls=PythonAndLegacyRunner)


@pytest.fixture
def mock_protocol_json_runner(decoy: Decoy) -> JsonRunner:
"""Get a mocked out PythonAndLegacyRunner dependency."""
return decoy.mock(cls=JsonRunner)


@pytest.fixture
def mock_setup_runner(decoy: Decoy) -> LiveRunner:
"""Get a mocked out LiveRunner dependency."""
return decoy.mock(cls=LiveRunner)


@pytest.fixture
def mock_fixit_runner(decoy: Decoy) -> LiveRunner:
"""Get a mocked out LiveRunner dependency."""
return decoy.mock(cls=LiveRunner)


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


@pytest.fixture
def mock_hardware_api(decoy: Decoy) -> HardwareAPI:
"""Get a mocked out HardwareAPI dependency."""
return decoy.mock(cls=HardwareAPI)


@pytest.fixture
def json_protocol_subject(
mock_protocol_engine: ProtocolEngine,
mock_hardware_api: HardwareAPI,
mock_protocol_json_runner: JsonRunner,
mock_fixit_runner: LiveRunner,
mock_setup_runner: LiveRunner,
) -> RunOrchestrator:
"""Get a RunOrchestrator subject with a json runner."""
return RunOrchestrator(
protocol_engine=mock_protocol_engine,
hardware_api=mock_hardware_api,
fixit_runner=mock_fixit_runner,
setup_runner=mock_setup_runner,
json_or_python_protocol_runner=mock_protocol_json_runner,
)


@pytest.fixture
def python_protocol_subject(
mock_protocol_engine: ProtocolEngine,
mock_hardware_api: HardwareAPI,
mock_protocol_python_runner: PythonAndLegacyRunner,
mock_fixit_runner: LiveRunner,
mock_setup_runner: LiveRunner,
) -> RunOrchestrator:
"""Get a RunOrchestrator subject with a python runner."""
return RunOrchestrator(
protocol_engine=mock_protocol_engine,
hardware_api=mock_hardware_api,
fixit_runner=mock_fixit_runner,
setup_runner=mock_setup_runner,
json_or_python_protocol_runner=mock_protocol_python_runner,
)


@pytest.mark.parametrize(
"input_protocol_config, mock_protocol_runner, subject",
[
(
JsonProtocolConfig(schema_version=7),
lazy_fixture("mock_protocol_json_runner"),
lazy_fixture("json_protocol_subject"),
),
(
PythonProtocolConfig(api_version=APIVersion(2, 14)),
lazy_fixture("mock_protocol_python_runner"),
lazy_fixture("python_protocol_subject"),
),
],
)
def test_build_run_orchestrator_provider(
decoy: Decoy,
monkeypatch: pytest.MonkeyPatch,
subject: RunOrchestrator,
mock_protocol_engine: ProtocolEngine,
mock_hardware_api: HardwareAPI,
input_protocol_config: Union[PythonProtocolConfig, JsonProtocolConfig],
mock_setup_runner: LiveRunner,
mock_fixit_runner: LiveRunner,
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_config=input_protocol_config,
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_protocol_runner)

result = subject.build_orchestrator(
protocol_engine=mock_protocol_engine,
hardware_api=mock_hardware_api,
protocol_config=input_protocol_config,
)

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