Skip to content

Commit

Permalink
pr suggestions. PE non optional cursor
Browse files Browse the repository at this point in the history
  • Loading branch information
TamarZanzouri committed Aug 5, 2024
1 parent a43e57d commit 7822aaf
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 28 deletions.
19 changes: 3 additions & 16 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,26 +630,13 @@ def get_slice(

def get_errors_slice(
self,
cursor: Optional[int],
cursor: int,
length: int,
) -> CommandErrorSlice:
"""Get a subset of commands error around a given cursor.
If the cursor is omitted, a cursor will be selected automatically
based most recent error.
"""
"""Get a subset of commands error around a given cursor."""
# start is inclusive, stop is exclusive
all_errors = self.get_all_errors()
total_length = len(all_errors)

if cursor is None:
if len(all_errors) > 0:
# Get the most recent error,
# which we can find just at the end of the list.
cursor = total_length - 1
else:
cursor = total_length - length

# start is inclusive, stop is exclusive
actual_cursor = max(0, min(cursor, total_length - 1))
stop = min(total_length, actual_cursor + length)

Expand Down
18 changes: 18 additions & 0 deletions api/src/opentrons/protocol_runner/run_orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
CommandSlice,
CommandErrorSlice,
DeckType,
ErrorOccurrence,
)
from ..protocol_engine.errors import RunStoppedError
from ..protocol_engine.types import (
Expand Down Expand Up @@ -279,8 +280,21 @@ def get_command_error_slice(
Args:
cursor: Requested index of first error in the returned slice.
If the cursor is omitted, a cursor will be selected automatically
based on the last error occurence.
length: Length of slice to return.
"""
all_errors = self.get_all_command_errors()
total_length = len(all_errors)

if cursor is None:
if len(all_errors) > 0:
# Get the most recent error,
# which we can find just at the end of the list.
cursor = total_length - 1
else:
cursor = 0

return self._protocol_engine.state_view.commands.get_errors_slice(
cursor=cursor, length=length
)
Expand All @@ -297,6 +311,10 @@ def get_all_commands(self) -> List[Command]:
"""Get all run commands."""
return self._protocol_engine.state_view.commands.get_all()

def get_all_command_errors(self) -> List[ErrorOccurrence]:
"""Get all run command errors."""
return self._protocol_engine.state_view.commands.get_all_errors()

def get_run_status(self) -> EngineStatus:
"""Get the current execution status of the engine."""
return self._protocol_engine.state_view.commands.get_status()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ def test_get_current() -> None:
def test_get_slice_empty() -> None:
"""It should return a slice from the tail if no current command."""
subject = get_command_view(commands=[])
result = subject.get_slice(cursor=None, length=2)
result = subject.get_slice(cursor=0, length=2)

assert result == CommandSlice(commands=[], cursor=0, total_length=0)

Expand Down Expand Up @@ -944,7 +944,7 @@ def test_get_slice_default_cursor_no_current() -> None:

subject = get_command_view(commands=[command_1, command_2, command_3, command_4])

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

assert result == CommandSlice(
commands=[command_2, command_3, command_4],
Expand Down Expand Up @@ -975,7 +975,7 @@ def test_get_slice_default_cursor_failed_command() -> None:
failed_command=CommandEntry(index=2, command=command_3),
)

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

assert result == CommandSlice(
commands=[command_3, command_4],
Expand Down Expand Up @@ -1032,7 +1032,7 @@ def test_get_slice_default_cursor_queued() -> None:
def test_get_errors_slice_empty() -> None:
"""It should return a slice from the tail if no current command."""
subject = get_command_view(failed_command_errors=[])
result = subject.get_errors_slice(cursor=None, length=2)
result = subject.get_errors_slice(cursor=0, length=2)

assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0)

Expand Down Expand Up @@ -1077,7 +1077,7 @@ def test_get_slice_default_cursor_last_error() -> None:
failed_command_errors=[error_1, error_2, error_3, error_4, error_5]
)

result = subject.get_errors_slice(cursor=None, length=2)
result = subject.get_errors_slice(cursor=0, length=2)

assert result == CommandErrorSlice(
commands_errors=[error_5],
Expand Down
10 changes: 6 additions & 4 deletions robot-server/robot_server/runs/router/base_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
RequestModel,
SimpleBody,
SimpleEmptyBody,
SimpleMultiBody,
MultiBody,
MultiBodyMeta,
ResourceLink,
Expand Down Expand Up @@ -426,7 +427,7 @@ async def put_error_recovery_policy(
"information available for a given command."
),
responses={
status.HTTP_200_OK: {"model": MultiBody[list[pe_errors.ErrorOccurrence]]},
status.HTTP_200_OK: {"model": SimpleMultiBody[list[pe_errors.ErrorOccurrence]]},
status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]},
},
)
Expand All @@ -445,7 +446,7 @@ async def get_run_commands_error(
description="The maximum number of command errors in the list to return.",
),
run_data_manager: RunDataManager = Depends(get_run_data_manager),
) -> PydanticResponse[MultiBody[list[pe_errors.ErrorOccurrence]]]:
) -> PydanticResponse[SimpleMultiBody[list[pe_errors.ErrorOccurrence]]]:
"""Get a summary of a set of command errors in a run.
Arguments:
Expand All @@ -469,8 +470,9 @@ async def get_run_commands_error(
)

return await PydanticResponse.create(
content=MultiBody.construct(
data=command_error_slice.commands_errors, meta=meta
content=SimpleMultiBody.construct(
data=command_error_slice.commands_errors, # type:ignore[arg-type]
meta=meta,
),
status_code=status.HTTP_200_OK,
)
1 change: 1 addition & 0 deletions robot-server/robot_server/runs/run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ def get_command_error_slice(
cursor=cursor, length=length
)

# TODO(tz, 8-5-2024): Change this to return to error list from the DB when we implement https://opentrons.atlassian.net/browse/EXEC-655.
raise RunNotCurrentError()

def get_current_command(self, run_id: str) -> Optional[CommandPointer]:
Expand Down
2 changes: 1 addition & 1 deletion robot-server/tests/runs/router/test_base_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ async def test_get_run_commands_errors_raises_no_run(
pageLength=42,
)

assert result.content.data == [
assert list(result.content.data) == [
pe_errors.ErrorOccurrence(
id="error-id",
errorType="PrettyBadError",
Expand Down
4 changes: 2 additions & 2 deletions robot-server/tests/runs/router/test_commands_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
)

from robot_server.errors.error_responses import ApiError
from robot_server.service.json_api import MultiBodyMeta, ResponseList
from robot_server.service.json_api import MultiBodyMeta

from robot_server.runs.command_models import (
RequestModelWithCommandCreate,
Expand All @@ -23,7 +23,7 @@
)
from robot_server.runs.run_store import CommandNotFoundError, RunStore
from robot_server.runs.run_orchestrator_store import RunOrchestratorStore
from robot_server.runs.run_data_manager import RunDataManager, RunNotCurrentError
from robot_server.runs.run_data_manager import RunDataManager
from robot_server.runs.run_models import RunCommandSummary, RunNotFoundError
from robot_server.runs.router.commands_router import (
create_run_command,
Expand Down

0 comments on commit 7822aaf

Please sign in to comment.