Skip to content

Commit

Permalink
fix(api): get_slice should return last executed command if a run fail…
Browse files Browse the repository at this point in the history
…ed (#14449)
  • Loading branch information
TamarZanzouri authored Feb 9, 2024
1 parent d22e93d commit 18fbbe1
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 69 deletions.
11 changes: 11 additions & 0 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ class CommandState:
are stored on the individual commands themselves.
"""

failed_command: Optional[CommandEntry]
"""The command, if any, that made the run fail and the index in the command list."""

finish_error: Optional[ErrorOccurrence]
"""The error that happened during the post-run finish steps (homing & dropping tips), if any."""

Expand Down Expand Up @@ -189,6 +192,7 @@ def __init__(
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_completed_at=None,
run_started_at=None,
latest_command_hash=None,
Expand Down Expand Up @@ -281,6 +285,7 @@ def handle_action(self, action: Action) -> None: # noqa: C901
),
)

self._state.failed_command = self._state.commands_by_id[action.command_id]
if prev_entry.command.intent == CommandIntent.SETUP:
other_command_ids_to_fail = [
*[i for i in self._state.queued_setup_command_ids],
Expand Down Expand Up @@ -464,6 +469,12 @@ def get_slice(
cursor = commands_by_id[running_command_id].index
elif len(queued_command_ids) > 0:
cursor = commands_by_id[queued_command_ids.head()].index - 1
elif (
self._state.run_result
and self._state.run_result == RunResult.FAILED
and self._state.failed_command
):
cursor = self._state.failed_command.index
else:
cursor = total_length - length

Expand Down
12 changes: 12 additions & 0 deletions api/tests/opentrons/protocol_engine/state/test_command_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def test_initial_state(
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
latest_command_hash=None,
stopped_by_estop=False,
)
Expand Down Expand Up @@ -672,6 +673,7 @@ def test_command_store_handles_pause_action(pause_source: PauseSource) -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
latest_command_hash=None,
stopped_by_estop=False,
)
Expand Down Expand Up @@ -699,6 +701,7 @@ def test_command_store_handles_play_action(pause_source: PauseSource) -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=datetime(year=2021, month=1, day=1),
latest_command_hash=None,
stopped_by_estop=False,
Expand Down Expand Up @@ -728,6 +731,7 @@ def test_command_store_handles_finish_action() -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=datetime(year=2021, month=1, day=1),
latest_command_hash=None,
stopped_by_estop=False,
Expand Down Expand Up @@ -772,6 +776,7 @@ def test_command_store_handles_stop_action(from_estop: bool) -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=datetime(year=2021, month=1, day=1),
latest_command_hash=None,
stopped_by_estop=from_estop,
Expand Down Expand Up @@ -800,6 +805,7 @@ def test_command_store_cannot_restart_after_should_stop() -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=None,
latest_command_hash=None,
stopped_by_estop=False,
Expand Down Expand Up @@ -930,6 +936,7 @@ def test_command_store_wraps_unknown_errors() -> None:
},
),
run_started_at=None,
failed_command=None,
latest_command_hash=None,
stopped_by_estop=False,
)
Expand Down Expand Up @@ -989,6 +996,7 @@ def __init__(self, message: str) -> None:
detail="yikes",
errorCode=ErrorCodes.PIPETTE_NOT_PRESENT.value.code,
),
failed_command=None,
run_started_at=None,
latest_command_hash=None,
stopped_by_estop=False,
Expand Down Expand Up @@ -1019,6 +1027,7 @@ def test_command_store_ignores_stop_after_graceful_finish() -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=datetime(year=2021, month=1, day=1),
latest_command_hash=None,
stopped_by_estop=False,
Expand Down Expand Up @@ -1049,6 +1058,7 @@ def test_command_store_ignores_finish_after_non_graceful_stop() -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=datetime(year=2021, month=1, day=1),
latest_command_hash=None,
stopped_by_estop=False,
Expand Down Expand Up @@ -1098,6 +1108,7 @@ def test_command_store_handles_command_failed() -> None:
},
run_error=None,
finish_error=None,
failed_command=CommandEntry(index=0, command=expected_failed_command),
run_started_at=None,
latest_command_hash=None,
stopped_by_estop=False,
Expand All @@ -1124,6 +1135,7 @@ def test_handles_hardware_stopped() -> None:
commands_by_id=OrderedDict(),
run_error=None,
finish_error=None,
failed_command=None,
run_started_at=None,
latest_command_hash=None,
stopped_by_estop=False,
Expand Down
37 changes: 36 additions & 1 deletion api/tests/opentrons/protocol_engine/state/test_command_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
RunResult,
QueueStatus,
)
from opentrons.protocol_engine.errors import ProtocolCommandFailedError
from opentrons.protocol_engine.errors import ProtocolCommandFailedError, ErrorOccurrence

from opentrons_shared_data.errors.codes import ErrorCodes

from .command_fixtures import (
create_queued_command,
Expand All @@ -44,6 +46,7 @@ def get_command_view(
queued_command_ids: Sequence[str] = (),
queued_setup_command_ids: Sequence[str] = (),
run_error: Optional[errors.ErrorOccurrence] = None,
failed_command: Optional[CommandEntry] = None,
finish_error: Optional[errors.ErrorOccurrence] = None,
commands: Sequence[cmd.Command] = (),
latest_command_hash: Optional[str] = None,
Expand All @@ -65,6 +68,7 @@ def get_command_view(
queued_setup_command_ids=OrderedSet(queued_setup_command_ids),
run_error=run_error,
finish_error=finish_error,
failed_command=failed_command,
all_command_ids=all_command_ids,
commands_by_id=commands_by_id,
run_started_at=run_started_at,
Expand Down Expand Up @@ -793,6 +797,37 @@ def test_get_slice_default_cursor_no_current() -> None:
)


def test_get_slice_default_cursor_failed_command() -> None:
"""It should return a slice from the last executed command."""
command_1 = create_failed_command(command_id="command-id-1")
command_2 = create_failed_command(command_id="command-id-2")
command_3 = create_failed_command(
command_id="command-id-3",
error=ErrorOccurrence(
id="error-id",
errorType="ProtocolEngineError",
createdAt=datetime(year=2022, month=2, day=2),
detail="oh no",
errorCode=ErrorCodes.GENERAL_ERROR.value.code,
),
)
command_4 = create_failed_command(command_id="command-id-4")

subject = get_command_view(
commands=[command_1, command_2, command_3, command_4],
run_result=RunResult.FAILED,
failed_command=CommandEntry(index=2, command=command_3),
)

result = subject.get_slice(cursor=None, length=3)

assert result == CommandSlice(
commands=[command_3, command_4],
cursor=2,
total_length=4,
)


def test_get_slice_default_cursor_running() -> None:
"""It should select a cursor based on the running command, if present."""
command_1 = create_succeeded_command(command_id="command-id-1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,3 +555,47 @@ stages:
completedAt: '{setup_command_completed_at}'
status: succeeded
params: {}

- name: Verify commands succeeded with pageLength and cursor
request:
url: '{ot2_server_base_url}/runs/{run_id}/commands?cursor=5&pageLength=2'
method: GET
response:
status_code: 200
json:
links:
current: !anydict
meta:
cursor: 5
totalLength: 15
data:
- id: !anystr
key: !anystr
commandType: loadLabware
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params:
location:
moduleId: magneticModuleId
loadName: foo_8_plate_33ul
namespace: example
version: 1
labwareId: destPlateId
displayName: Sample Collection Plate
- id: !anystr
key: !anystr
commandType: loadLabware
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params:
location:
slotName: '8'
loadName: opentrons_96_tiprack_10ul
namespace: opentrons
version: 1
labwareId: tipRackId
displayName: Opentrons 96 Tip Rack 10 µL
Original file line number Diff line number Diff line change
Expand Up @@ -83,44 +83,9 @@ stages:
links:
current: !anydict
meta:
cursor: 0
cursor: 3
totalLength: 5
data:
# Initial home
- id: !anystr
key: !anystr
commandType: home
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params: {}
- id: !anystr
key: !anystr
commandType: loadLabware
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params:
location:
slotName: '8'
loadName: fixture_1_tiprack_10ul
namespace: fixture
version: 1
labwareId: tipRackId
displayName: Tip Rack
- id: !anystr
key: !anystr
commandType: loadPipette
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params:
pipetteName: p10_single
mount: left
pipetteId: pipetteId
- id: !anystr
key: !anystr
commandType: aspirate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,40 +84,9 @@ stages:
links:
current: !anydict
meta:
cursor: 0
cursor: 3
totalLength: 4
data:
- id: !anystr
key: !anystr
commandType: home
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params: {}
- id: !anystr
key: !anystr
commandType: loadLabware
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params:
location:
slotName: '1'
loadName: opentrons_96_tiprack_300ul
namespace: opentrons
version: 1
- id: !anystr
key: !anystr
commandType: loadPipette
createdAt: !anystr
startedAt: !anystr
completedAt: !anystr
status: succeeded
params:
pipetteName: p300_single
mount: right
- id: !anystr
key: !anystr
commandType: aspirate
Expand Down

0 comments on commit 18fbbe1

Please sign in to comment.