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

refactor(api, robot-server): Refactor ProtocolRunner per protocol type #12343

Merged
merged 36 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2410782
WIP: created an abstract interface for ProtocolRunner. started JsonRu…
TamarZanzouri Mar 14, 2023
290e860
WIP create_protocol_runner
TamarZanzouri Mar 16, 2023
842a99e
test json_runner and finished logic for json_runner
TamarZanzouri Mar 17, 2023
f5b5325
WIP python and legacy runner
TamarZanzouri Mar 17, 2023
7de1dae
python and legacy runner implementation
TamarZanzouri Mar 20, 2023
8f30fa7
create_simulating_runner.py
TamarZanzouri Mar 20, 2023
c73922c
fixed robot server dependencies.py
TamarZanzouri Mar 21, 2023
33c260a
MaintenanceRunner
TamarZanzouri Mar 21, 2023
b727804
wip robot-server
TamarZanzouri Mar 21, 2023
5ecd3cb
changed create_simulating_runner to monkeypatch
TamarZanzouri Mar 22, 2023
037f322
removed set_run_started_at and fixed test failing on started_at
TamarZanzouri Mar 22, 2023
d54c1ec
rollback depends for protocol_analyzer.py and moved runner creation i…
TamarZanzouri Mar 22, 2023
af5aaac
reorganize prtocol_runner tests and text fixes
TamarZanzouri Mar 23, 2023
3a870b0
removed task_queue from maintenance runner
TamarZanzouri Mar 23, 2023
4009a37
Merge branch 'edge' into RSS-200-refactor-protocol-runner
TamarZanzouri Mar 23, 2023
50978e3
linting
TamarZanzouri Mar 23, 2023
20d3c3f
fixed failing test in actions router
TamarZanzouri Mar 23, 2023
08524fd
fixed maintenance test with task runner
TamarZanzouri Mar 23, 2023
da0fdba
Update api/src/opentrons/protocol_runner/protocol_runner.py
TamarZanzouri Mar 27, 2023
b38aaf7
pr feedback
TamarZanzouri Mar 27, 2023
14e2fa0
changed ProtocolRunResult to RunnerRunResult
TamarZanzouri Mar 27, 2023
5aa7899
added linting ignore
TamarZanzouri Mar 28, 2023
f36be33
fixed g-code testing
TamarZanzouri Mar 28, 2023
a81df1d
g-code testing fix
TamarZanzouri Mar 28, 2023
b647315
linting
TamarZanzouri Mar 28, 2023
e802d65
linting
TamarZanzouri Mar 28, 2023
923e00c
added pe.play for run live commands
TamarZanzouri Mar 28, 2023
e1e9ddd
added tavern test for posting protocol commands
TamarZanzouri Mar 29, 2023
88cae89
addressed PR comments, fixed live runner bug, updated tests
sanni-t Apr 5, 2023
742e292
updated tests, fixed linter errors
sanni-t Apr 5, 2023
93be36f
Merge branch 'edge' into RSS-200-refactor-protocol-runner
sanni-t Apr 7, 2023
56a1b2f
correct docstring
sanni-t Apr 7, 2023
b272e10
Apply suggestions from code review
sanni-t Apr 13, 2023
433ca47
fix runner smoke tests and linter errors, some renaming from PR review
sanni-t Apr 14, 2023
b6e7385
update runner typing & type naming, renamed set_task_queue_wait() to …
sanni-t Apr 14, 2023
5789c41
address all remaining review comments
sanni-t Apr 14, 2023
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
Prev Previous commit
Next Next commit
changed ProtocolRunResult to RunnerRunResult
  • Loading branch information
TamarZanzouri committed Mar 27, 2023
commit 14e2fa0fef3f248c0a5422721485ec2a3ffa8d4f
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_runner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"""
from .protocol_runner import (
AbstractRunner,
ProtocolRunResult,
RunnerRunResult,
create_protocol_runner,
JsonRunner,
PythonAndLegacyRunner,
Expand All @@ -15,7 +15,7 @@

__all__ = [
"AbstractRunner",
"ProtocolRunResult",
"RunnerRunResult",
"create_simulating_runner",
"create_protocol_runner",
"JsonRunner",
Expand Down
19 changes: 8 additions & 11 deletions api/src/opentrons/protocol_runner/protocol_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
)


class ProtocolRunResult(NamedTuple):
class RunnerRunResult(NamedTuple):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you have a better name?

This comment was marked as outdated.

"""Result data from a run, pulled from the ProtocolEngine."""

commands: List[Command]
Expand Down Expand Up @@ -81,7 +81,7 @@ async def stop(self) -> None:
async def run(
self,
protocol_source: Optional[ProtocolSource] = None,
) -> ProtocolRunResult:
) -> RunnerRunResult:
"""Run a given protocol to completion."""


Expand Down Expand Up @@ -166,7 +166,7 @@ async def stop(self) -> None:
async def run(
self,
protocol_source: Optional[ProtocolSource] = None,
) -> ProtocolRunResult:
) -> RunnerRunResult:
# TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
# currently `protocol_source` arg is only used by tests
if protocol_source:
Expand All @@ -179,7 +179,7 @@ async def run(

run_data = self._protocol_engine.state_view.get_summary()
commands = self._protocol_engine.state_view.commands.get_all()
return ProtocolRunResult(commands=commands, state_summary=run_data)
return RunnerRunResult(commands=commands, state_summary=run_data)


class JsonRunner(AbstractRunner):
Expand Down Expand Up @@ -268,7 +268,7 @@ async def stop(self) -> None:
async def run(
self,
protocol_source: Optional[ProtocolSource] = None,
) -> ProtocolRunResult:
) -> RunnerRunResult:
# TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can totally do this for this pr

# currently `protocol_source` arg is only used by tests
if protocol_source:
Expand All @@ -281,7 +281,7 @@ async def run(

run_data = self._protocol_engine.state_view.get_summary()
commands = self._protocol_engine.state_view.commands.get_all()
return ProtocolRunResult(commands=commands, state_summary=run_data)
return RunnerRunResult(commands=commands, state_summary=run_data)


class LiveRunner(AbstractRunner):
Expand Down Expand Up @@ -324,15 +324,12 @@ async def stop(self) -> None:
async def run(
self,
protocol_source: Optional[ProtocolSource] = None,
SyntaxColoring marked this conversation as resolved.
Show resolved Hide resolved
) -> ProtocolRunResult:
if protocol_source:
self.load(protocol_source)

) -> RunnerRunResult:
await self._hardware_api.home()

run_data = self._protocol_engine.state_view.get_summary()
commands = self._protocol_engine.state_view.commands.get_all()
return ProtocolRunResult(commands=commands, state_summary=run_data)
return RunnerRunResult(commands=commands, state_summary=run_data)


def create_protocol_runner(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ async def test_create_protocol_runner(
json_file_reader=json_file_reader,
json_translator=json_translator,
),
type(runner_type),
runner_type,
)


Expand Down
12 changes: 6 additions & 6 deletions robot-server/robot_server/runs/engine_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
from opentrons.config import feature_flags
from opentrons.hardware_control import HardwareControlAPI
from opentrons.protocol_runner import (
ProtocolRunner,
ProtocolRunResult,
AbstractRunner,
RunnerRunResult,
create_protocol_runner,
)
from opentrons.protocol_engine import (
Expand All @@ -34,7 +34,7 @@ class RunnerEnginePair(NamedTuple):
"""A stored ProtocolRunner/ProtocolEngine pair."""

run_id: str
runner: ProtocolRunner
runner: AbstractRunner
engine: ProtocolEngine


Expand Down Expand Up @@ -65,7 +65,7 @@ def engine(self) -> ProtocolEngine:
return self._runner_engine_pair.engine

@property
def runner(self) -> ProtocolRunner:
def runner(self) -> AbstractRunner:
"""Get the "current" persisted ProtocolRunner."""
assert self._runner_engine_pair is not None, "Runner not yet created."
return self._runner_engine_pair.runner
Expand Down Expand Up @@ -162,7 +162,7 @@ async def create(

return engine.state_view.get_summary()

async def clear(self) -> ProtocolRunResult:
async def clear(self) -> RunnerRunResult:
"""Remove the persisted ProtocolEngine.

Raises:
Expand All @@ -181,4 +181,4 @@ async def clear(self) -> ProtocolRunResult:
commands = state_view.commands.get_all()
self._runner_engine_pair = None

return ProtocolRunResult(state_summary=run_data, commands=commands)
return RunnerRunResult(state_summary=run_data, commands=commands)
2 changes: 1 addition & 1 deletion robot-server/tests/protocols/test_protocol_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ async def test_analyze(
).then_return(json_runner)

decoy.when(await json_runner.run(protocol_resource.source)).then_return(
protocol_runner.ProtocolRunResult(
protocol_runner.RunnerRunResult(
commands=[analysis_command],
state_summary=StateSummary(
status=EngineStatus.SUCCEEDED,
Expand Down
8 changes: 4 additions & 4 deletions robot-server/tests/runs/test_engine_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
from opentrons.hardware_control import HardwareControlAPI
from opentrons.protocol_engine import ProtocolEngine, StateSummary, types as pe_types
from opentrons.protocol_runner import (
ProtocolRunResult,
MaintenanceRunner,
RunnerRunResult,
LiveRunner,
)
from opentrons.protocol_reader import ProtocolReader, ProtocolSource

Expand Down Expand Up @@ -50,7 +50,7 @@ async def test_create_engine(subject: EngineStore) -> None:

assert subject.current_run_id == "run-id"
assert isinstance(result, StateSummary)
assert isinstance(subject.runner, MaintenanceRunner)
assert isinstance(subject.runner, LiveRunner)
assert isinstance(subject.engine, ProtocolEngine)


Expand Down Expand Up @@ -134,7 +134,7 @@ async def test_clear_engine(subject: EngineStore) -> None:
result = await subject.clear()

assert subject.current_run_id is None
assert isinstance(result, ProtocolRunResult)
assert isinstance(result, RunnerRunResult)

with pytest.raises(AssertionError):
subject.engine
Expand Down
4 changes: 2 additions & 2 deletions robot-server/tests/runs/test_run_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
commands as pe_commands,
errors as pe_errors,
)
from opentrons.protocol_runner import ProtocolRunResult
from opentrons.protocol_runner import RunnerRunResult

from robot_server.service.task_runner import TaskRunner
from robot_server.runs.action_models import RunAction, RunActionType
Expand Down Expand Up @@ -144,7 +144,7 @@ async def test_create_play_action_to_start(
decoy.verify(mock_task_runner.run(background_task_captor))

decoy.when(await mock_engine_store.runner.run()).then_return(
ProtocolRunResult(
RunnerRunResult(
commands=protocol_commands,
state_summary=engine_state_summary,
)
Expand Down
6 changes: 3 additions & 3 deletions robot-server/tests/runs/test_run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from decoy import Decoy, matchers

from opentrons.types import DeckSlotName
from opentrons.protocol_runner import ProtocolRunResult
from opentrons.protocol_runner import RunnerRunResult
from opentrons.protocol_engine import (
EngineStatus,
StateSummary,
Expand Down Expand Up @@ -485,7 +485,7 @@ async def test_update_current(
run_id = "hello world"
decoy.when(mock_engine_store.current_run_id).then_return(run_id)
decoy.when(await mock_engine_store.clear()).then_return(
ProtocolRunResult(commands=[run_command], state_summary=engine_state_summary)
RunnerRunResult(commands=[run_command], state_summary=engine_state_summary)
)

decoy.when(
Expand Down Expand Up @@ -593,7 +593,7 @@ async def test_create_archives_existing(

decoy.when(mock_engine_store.current_run_id).then_return(run_id_old)
decoy.when(await mock_engine_store.clear()).then_return(
ProtocolRunResult(commands=[run_command], state_summary=engine_state_summary)
RunnerRunResult(commands=[run_command], state_summary=engine_state_summary)
)

decoy.when(
Expand Down