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

feat(api): Do not enqueue json commands on protocol load #14759

Merged
merged 22 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
07b01f5
added schema definition for fixtures
TamarZanzouri Sep 14, 2023
e44a81a
initial move commands list into task_queue
TamarZanzouri Mar 26, 2024
a073995
moved logic from task queue into runner for json protocols
TamarZanzouri Mar 27, 2024
ce43819
reverted task queue changes
TamarZanzouri Mar 27, 2024
004e498
json runner with Event set and wait. fixed v6 upload test
TamarZanzouri Mar 28, 2024
e2d4658
reverted task queue changes and use runner._run method instead
TamarZanzouri Mar 29, 2024
f87b1db
Delete shared-data/fixture/schemas/1.json
TamarZanzouri Mar 29, 2024
3d7c688
clean up
TamarZanzouri Mar 29, 2024
23c7d40
Merge branch 'edge' into EXEC-352-do-not-enqueue-json-protocol-commands
TamarZanzouri Mar 29, 2024
747974c
fixed failing test from merge
TamarZanzouri Apr 1, 2024
bd2afe5
fixed failing test
TamarZanzouri Apr 1, 2024
335c10d
started adding tests WIP
TamarZanzouri Apr 2, 2024
2e1c4f4
added captors and tested add_and_execute_command
TamarZanzouri Apr 2, 2024
cb83ccf
added test for break on stop
TamarZanzouri Apr 2, 2024
819d957
added test to not execute commands if run stopped
TamarZanzouri Apr 3, 2024
c5249d3
fixed logic with test
TamarZanzouri Apr 3, 2024
6c812d6
added tests for papi and json play and stop in the middle of a run
TamarZanzouri Apr 4, 2024
76aa3e5
fixed unit test to match logic in e2e test
TamarZanzouri Apr 4, 2024
d3b0cd3
changed tests to use delay and wait for a running state
TamarZanzouri Apr 4, 2024
25344bd
Update robot-server/tests/integration/http_api/runs/test_play_stop_pa…
TamarZanzouri Apr 4, 2024
02a6ead
removed retry on get run commands
TamarZanzouri Apr 4, 2024
123f313
removed retry
TamarZanzouri Apr 4, 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
5 changes: 2 additions & 3 deletions api/src/opentrons/protocol_runner/protocol_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ async def load(self, protocol_source: ProtocolSource) -> None:
# this command homes all axes, including pipette plugner and gripper jaw
self._protocol_engine.add_command(request=initial_home_command)

for command in commands:
self._queued_commands.append(command)
self._queued_commands = commands

self._task_queue.set_run_func(func=self._add_command_and_execute)

Expand All @@ -360,7 +359,7 @@ async def run( # noqa: D102
async def _add_command_and_execute(self) -> None:
for command in self._queued_commands:
result = await self._protocol_engine.add_and_execute_command(command)
if result.error:
if result and result.error:
raise ProtocolCommandFailedError(
original_error=result.error,
message=f"{result.error.errorType}: {result.error.detail}",
Expand Down
126 changes: 124 additions & 2 deletions api/tests/opentrons/protocol_runner/test_protocol_runner.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
"""Tests for the PythonAndLegacyRunner, JsonRunner & LiveRunner classes."""
from datetime import datetime

import pytest
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
from decoy import Decoy, matchers
Expand All @@ -18,7 +20,12 @@
from opentrons.util.broker import Broker

from opentrons import protocol_reader
from opentrons.protocol_engine import ProtocolEngine, Liquid, commands as pe_commands
from opentrons.protocol_engine import (
ProtocolEngine,
Liquid,
commands as pe_commands,
errors as pe_errors,
)
from opentrons.protocol_reader import (
ProtocolSource,
JsonProtocolConfig,
Expand Down Expand Up @@ -328,6 +335,96 @@ async def test_run_json_runner(
)


async def test_run_json_runner_stop_requested_stops_enquqing(
decoy: Decoy,
hardware_api: HardwareAPI,
protocol_engine: ProtocolEngine,
task_queue: TaskQueue,
json_runner_subject: JsonRunner,
json_file_reader: JsonFileReader,
json_translator: JsonTranslator,
) -> None:
"""It should run a protocol to completion."""
labware_definition = LabwareDefinition.construct() # type: ignore[call-arg]
json_protocol_source = ProtocolSource(
directory=Path("/dev/null"),
main_file=Path("/dev/null/abc.json"),
files=[],
metadata={},
robot_type="OT-2 Standard",
config=JsonProtocolConfig(schema_version=6),
content_hash="abc123",
)

commands: List[pe_commands.CommandCreate] = [
pe_commands.HomeCreate(params=pe_commands.HomeParams()),
pe_commands.WaitForDurationCreate(
params=pe_commands.WaitForDurationParams(seconds=10)
),
pe_commands.LoadLiquidCreate(
params=pe_commands.LoadLiquidParams(
liquidId="water-id", labwareId="labware-id", volumeByWell={"A1": 30}
)
),
]

liquids: List[Liquid] = [
Liquid(id="water-id", displayName="water", description="water desc")
]

json_protocol = ProtocolSchemaV6.construct() # type: ignore[call-arg]

decoy.when(
await protocol_reader.extract_labware_definitions(json_protocol_source)
).then_return([labware_definition])
decoy.when(json_file_reader.read(json_protocol_source)).then_return(json_protocol)
decoy.when(json_translator.translate_commands(json_protocol)).then_return(commands)
decoy.when(json_translator.translate_liquids(json_protocol)).then_return(liquids)
decoy.when(
await protocol_engine.add_and_execute_command(
pe_commands.HomeCreate(params=pe_commands.HomeParams()),
)
).then_return(
pe_commands.Home.construct(status=pe_commands.CommandStatus.SUCCEEDED) # type: ignore[call-arg]
)
decoy.when(
await protocol_engine.add_and_execute_command(
pe_commands.WaitForDurationCreate(
params=pe_commands.WaitForDurationParams(seconds=10)
),
)
).then_return(
pe_commands.WaitForDuration.construct( # type: ignore[call-arg]
error=pe_errors.ErrorOccurrence.from_failed(
id="some-id",
createdAt=datetime(year=2021, month=1, day=1),
error=pe_errors.ProtocolEngineError(),
)
)
)

await json_runner_subject.load(json_protocol_source)

run_func_captor = matchers.Captor()

decoy.verify(
protocol_engine.add_labware_definition(labware_definition),
protocol_engine.add_liquid(
id="water-id", name="water", description="water desc", color=None
),
protocol_engine.add_command(
request=pe_commands.HomeCreate(params=pe_commands.HomeParams(axes=None))
),
task_queue.set_run_func(func=run_func_captor),
)

# Verify that the run func calls the right things:
run_func = run_func_captor.value

with pytest.raises(pe_errors.ProtocolEngineError):
await run_func()


@pytest.mark.parametrize(
"schema_version, json_protocol",
[
Expand Down Expand Up @@ -385,6 +482,8 @@ async def test_load_json_runner(

await json_runner_subject.load(json_protocol_source)

run_func_captor = matchers.Captor()

decoy.verify(
protocol_engine.add_labware_definition(labware_definition),
protocol_engine.add_liquid(
Expand All @@ -393,7 +492,30 @@ async def test_load_json_runner(
protocol_engine.add_command(
request=pe_commands.HomeCreate(params=pe_commands.HomeParams(axes=None))
),
task_queue.set_run_func(func=json_runner_subject._add_command_and_execute),
task_queue.set_run_func(func=run_func_captor),
)

# Verify that the run func calls the right things:
run_func = run_func_captor.value
await run_func()
decoy.verify(
await protocol_engine.add_and_execute_command(
request=pe_commands.WaitForResumeCreate(
params=pe_commands.WaitForResumeParams(message="hello")
),
),
await protocol_engine.add_and_execute_command(
request=pe_commands.WaitForResumeCreate(
params=pe_commands.WaitForResumeParams(message="goodbye")
),
),
await protocol_engine.add_and_execute_command(
request=pe_commands.LoadLiquidCreate(
params=pe_commands.LoadLiquidParams(
liquidId="water-id", labwareId="labware-id", volumeByWell={"A1": 30}
)
),
),
)


Expand Down
SyntaxColoring marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ stages:
commandType: home
createdAt: !anystr
status: queued
params: { }
params: {}
- id: '{setup_command_id}'
key: '{setup_command_key}'
intent: setup
Expand Down Expand Up @@ -183,6 +183,7 @@ stages:
completedAt: '{setup_command_completed_at}'
status: succeeded
params: { }
notes: []
- id: !anystr
key: !anystr
commandType: loadPipette
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ stages:
completedAt: !anystr
status: succeeded
params: { }
notes: [ ]
- id: '{setup_command_id}'
key: '{setup_command_key}'
intent: setup
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
test_name: Test python protocol run commands are failed when stopped.

marks:
- usefixtures:
- ot2_server_base_url
stages:
- name: Upload a python protocol
request:
url: '{ot2_server_base_url}/protocols'
method: POST
files:
files: 'tests/integration/protocols/wait_for_resume_stop_papi.py'
response:
status_code: 201
save:
json:
protocol_id: data.id

- name: Create run from protocol
request:
url: '{ot2_server_base_url}/runs'
method: POST
json:
data:
protocolId: '{protocol_id}'
response:
status_code: 201
save:
json:
run_id: data.id

- name: Play the run
request:
url: '{ot2_server_base_url}/runs/{run_id}/actions'
method: POST
json:
data:
actionType: play
response:
status_code: 201

- name: Stop the run
request:
url: '{ot2_server_base_url}/runs/{run_id}/actions'
method: POST
json:
data:
actionType: stop
response:
status_code: 201
TamarZanzouri marked this conversation as resolved.
Show resolved Hide resolved

- name: Wait for the run to complete
max_retries: 10
delay_after: 0.2
request:
url: '{ot2_server_base_url}/runs/{run_id}'
method: GET
response:
status_code: 200
strict:
- json:off
json:
data:
status: stopped

- name: Get run commands
max_retries: 10
delay_after: 0.2
TamarZanzouri marked this conversation as resolved.
Show resolved Hide resolved
request:
url: '{ot2_server_base_url}/runs/{run_id}/commands'
method: GET
response:
status_code: 200
strict:
- json:off
json:
data:
- id: !anystr
key: !anystr
commandType: home
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params: {}
notes: []
- id: !anystr
key: !anystr
commandType: home
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params: { }
notes: [ ]
- id: !anystr
key: !anystr
commandType: waitForResume
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: failed
params: { }
notes: [ ]
error:
createdAt: !anystr
detail: 'Run was cancelled'
errorCode: '4000'
errorInfo: { }
errorType: 'RunStoppedError'
id: !anystr
wrappedErrors: [ ]


Loading
Loading