Skip to content

Commit

Permalink
cherry pick fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
TamarZanzouri committed Aug 20, 2024
1 parent 0f2dc73 commit be7bd0b
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 44 deletions.
32 changes: 30 additions & 2 deletions api/src/opentrons/protocol_engine/state/command_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,28 @@ def get_all_commands(self) -> List[Command]:
for command_id in self._all_command_ids
]

def get_filtered_command_ids(
self,
command_intents: list[CommandIntent] = [
CommandIntent.PROTOCOL,
CommandIntent.SETUP,
],
) -> List[str]:
filtered_command = self._commands_by_id
if CommandIntent.FIXIT not in command_intents:
filtered_command = {
key: val
for key, val in self._commands_by_id.items()
if val.command.intent != CommandIntent.FIXIT
}
if CommandIntent.SETUP not in command_intents:
filtered_command = {
key: val
for key, val in filtered_command.items()
if val.command.intent != CommandIntent.SETUP
}
return list(filtered_command.keys())

def get_filtered_queue_ids(self, all_commands: bool) -> list[str]:
print(list(self.get_fixit_queue_ids()))
return [
Expand All @@ -109,9 +131,15 @@ def get_all_ids(self) -> List[str]:
"""Get all command IDs."""
return self._all_command_ids

def get_slice(self, start: int, stop: int) -> List[Command]:
"""Get a list of commands between start and stop."""
def get_slice(
self, start: int, stop: int, command_ids: Optional[list[str]] = None
) -> List[Command]:
"""Get a list of commands between start and stop.""" """Get a list of commands between start and stop."""
commands = self._all_command_ids[start:stop]
selected_command_ids = (
command_ids if command_ids is not None else self._all_command_ids
)
commands = selected_command_ids[start:stop]
return [self._commands_by_id[command].command for command in commands]

def get_tail_command(self) -> Optional[CommandEntry]:
Expand Down
20 changes: 3 additions & 17 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -579,22 +579,8 @@ def get_all(self) -> List[Command]:
"""
return self._state.command_history.get_all_commands()

def get_filtered_queue_ids(self, all_commands: bool) -> List[str]:
"""Get a list of all filtered commands in state.
Entries are returned in the order of first-added command to last-added command.
Replacing a command (to change its status, for example) keeps its place in the
ordering.
If all_commands is True, retunred list will contain all command intents.
If False, return list will contain only safe commands.
"""
return self._state.command_history.get_filtered_queue_ids(
all_commands=all_commands
)

def get_slice(
self, cursor: Optional[int], length: int, all_commands: bool
self, cursor: Optional[int], length: int, include_fixit_commands: bool
) -> CommandSlice:
"""Get a subset of commands around a given cursor.
Expand All @@ -610,8 +596,8 @@ def get_slice(
if include_fixit_commands
else [CommandIntent.PROTOCOL, CommandIntent.SETUP]
)
running_command = self._state.command_history.get_running_command()
queued_command_ids = self._state.command_history.get_queue_ids()
running_command = self._state.command_history.get_running_command()
queued_command_ids = self._state.command_history.get_queue_ids()
total_length = len(command_ids)

# TODO(mm, 2024-05-17): This looks like it's attempting to do the same thing
Expand Down
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_runner/run_orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def get_current_command(self) -> Optional[CommandPointer]:
return self._protocol_engine.state_view.commands.get_current()

def get_command_slice(
self, cursor: Optional[int], length: int, all_commands: bool
self, cursor: Optional[int], length: int, include_fixit_commands: bool
) -> CommandSlice:
"""Get a slice of run commands.
Expand All @@ -267,7 +267,7 @@ def get_command_slice(
all_commands: Get all command intents.
"""
return self._protocol_engine.state_view.commands.get_slice(
cursor=cursor, length=length, all_commands=all_commands
cursor=cursor, length=length, include_fixit_commands=include_fixit_commands
)

def get_command_error_slice(
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=0, length=2, all_commands=True)
result = subject.get_slice(cursor=0, length=2, include_fixit_commands=True)

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

Expand All @@ -918,15 +918,15 @@ def test_get_slice() -> None:

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

result = subject.get_slice(cursor=1, length=3, all_commands=True)
result = subject.get_slice(cursor=1, length=3, include_fixit_commands=True)

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

result = subject.get_slice(cursor=-3, length=10, all_commands=True)
result = subject.get_slice(cursor=-3, length=10, include_fixit_commands=True)

assert result == CommandSlice(
commands=[command_1, command_2, command_3, command_4],
Expand All @@ -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, all_commands=True)
result = subject.get_slice(cursor=None, length=3, include_fixit_commands=True)

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, all_commands=True)
result = subject.get_slice(cursor=None, length=3, include_fixit_commands=True)

assert result == CommandSlice(
commands=[command_3, command_4],
Expand All @@ -997,7 +997,7 @@ def test_get_slice_default_cursor_running() -> None:
running_command_id="command-id-3",
)

result = subject.get_slice(cursor=None, length=2, all_commands=True)
result = subject.get_slice(cursor=None, length=2, include_fixit_commands=True)

assert result == CommandSlice(
commands=[command_3, command_4],
Expand Down
4 changes: 3 additions & 1 deletion robot-server/robot_server/commands/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ async def get_commands_list(
cursor: Cursor index for the collection response.
pageLength: Maximum number of items to return.
"""
cmd_slice = orchestrator.get_command_slice(cursor=cursor, length=pageLength)
cmd_slice = orchestrator.get_command_slice(
cursor=cursor, length=pageLength, include_fixit_commands=True
)
commands = cast(List[StatelessCommand], cmd_slice.commands)
meta = MultiBodyMeta(cursor=cmd_slice.cursor, totalLength=cmd_slice.total_length)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ def get_command_slice(
cursor: Requested index of first command in the returned slice.
length: Length of slice to return.
"""
return self.run_orchestrator.get_command_slice(cursor=cursor, length=length)
return self.run_orchestrator.get_command_slice(
cursor=cursor, length=length, include_fixit_commands=False
)

def get_current_command(self) -> Optional[CommandPointer]:
"""Get the "current" command, if any."""
Expand Down
8 changes: 8 additions & 0 deletions robot-server/robot_server/runs/router/commands_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@ async def get_run_commands(
description="The maximum number of commands in the list to return.",
),
] = _DEFAULT_COMMAND_LIST_LENGTH,
includeFixitCommands: bool = Query(
True,
description="If `true`, return all commands (protocol, setup, fixit)."
" If `false`, only return safe commands (protocol, setup).",
),
) -> PydanticResponse[MultiBody[RunCommandSummary, CommandCollectionLinks]]:
"""Get a summary of a set of commands in a run.
Expand All @@ -293,12 +298,15 @@ async def get_run_commands(
cursor: Cursor index for the collection response.
pageLength: Maximum number of items to return.
run_data_manager: Run data retrieval interface.
includeFixitCommands: If `true`, return all commands."
" If `false`, only return safe commands.
"""
try:
command_slice = run_data_manager.get_commands_slice(
run_id=runId,
cursor=cursor,
length=pageLength,
include_fixit_commands=includeFixitCommands,
)
except RunNotFoundError as e:
raise RunNotFound.from_exc(e).as_error(status.HTTP_404_NOT_FOUND) from e
Expand Down
6 changes: 5 additions & 1 deletion robot-server/robot_server/runs/run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,20 +373,24 @@ def get_commands_slice(
run_id: str,
cursor: Optional[int],
length: int,
include_fixit_commands: bool,
) -> CommandSlice:
"""Get a slice of run commands.
Args:
run_id: ID of the run.
cursor: Requested index of first command in the returned slice.
length: Length of slice to return.
include_fixit_commands: Include fixit commands.
Raises:
RunNotFoundError: The given run identifier was not found in the database.
"""
if run_id == self._run_orchestrator_store.current_run_id:
return self._run_orchestrator_store.get_command_slice(
cursor=cursor, length=length
cursor=cursor,
length=length,
include_fixit_commands=include_fixit_commands,
)

# Let exception propagate
Expand Down
9 changes: 5 additions & 4 deletions robot-server/robot_server/runs/run_orchestrator_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,17 +330,18 @@ def get_current_command(self) -> Optional[CommandPointer]:
return self.run_orchestrator.get_current_command()

def get_command_slice(
self,
cursor: Optional[int],
length: int,
self, cursor: Optional[int], length: int, include_fixit_commands: bool
) -> CommandSlice:
"""Get a slice of run commands.
Args:
cursor: Requested index of first command in the returned slice.
length: Length of slice to return.
include_fixit_commands: Include fixit commands.
"""
return self.run_orchestrator.get_command_slice(cursor=cursor, length=length)
return self.run_orchestrator.get_command_slice(
cursor=cursor, length=length, include_fixit_commands=include_fixit_commands
)

def get_command_error_slice(
self,
Expand Down
6 changes: 5 additions & 1 deletion robot-server/tests/commands/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,11 @@ async def test_get_commands_list(
index=0,
)
)
decoy.when(run_orchestrator.get_command_slice(cursor=1337, length=42)).then_return(
decoy.when(
run_orchestrator.get_command_slice(
cursor=1337, length=42, include_fixit_commands=True
)
).then_return(
CommandSlice(commands=[command_1, command_2], cursor=0, total_length=2)
)

Expand Down
12 changes: 7 additions & 5 deletions robot-server/tests/runs/router/test_commands_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,7 @@ async def test_get_run_commands(
)
decoy.when(
mock_run_data_manager.get_commands_slice(
run_id="run-id",
cursor=None,
length=42,
run_id="run-id", cursor=None, length=42, include_fixit_commands=True
)
).then_return(CommandSlice(commands=[command], cursor=1, total_length=3))

Expand Down Expand Up @@ -412,7 +410,9 @@ async def test_get_run_commands_empty(
"""It should return an empty commands list if no commands."""
decoy.when(mock_run_data_manager.get_current_command("run-id")).then_return(None)
decoy.when(
mock_run_data_manager.get_commands_slice(run_id="run-id", cursor=21, length=42)
mock_run_data_manager.get_commands_slice(
run_id="run-id", cursor=21, length=42, include_fixit_commands=True
)
).then_return(CommandSlice(commands=[], cursor=0, total_length=0))

result = await get_run_commands(
Expand All @@ -436,7 +436,9 @@ async def test_get_run_commands_not_found(
not_found_error = RunNotFoundError("oh no")

decoy.when(
mock_run_data_manager.get_commands_slice(run_id="run-id", cursor=21, length=42)
mock_run_data_manager.get_commands_slice(
run_id="run-id", cursor=21, length=42, include_fixit_commands=True
)
).then_raise(not_found_error)
decoy.when(mock_run_data_manager.get_current_command(run_id="run-id")).then_raise(
not_found_error
Expand Down
12 changes: 8 additions & 4 deletions robot-server/tests/runs/test_run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,9 @@ def test_get_commands_slice_from_db(
decoy.when(
mock_run_store.get_commands_slice(run_id="run_id", cursor=1, length=2)
).then_return(expected_command_slice)
result = subject.get_commands_slice(run_id="run_id", cursor=1, length=2)
result = subject.get_commands_slice(
run_id="run_id", cursor=1, length=2, include_fixit_commands=True
)

assert expected_command_slice == result

Expand All @@ -875,11 +877,11 @@ def test_get_commands_slice_current_run(
commands=expected_commands_result, cursor=1, total_length=3
)
decoy.when(mock_run_orchestrator_store.current_run_id).then_return("run-id")
decoy.when(mock_run_orchestrator_store.get_command_slice(1, 2)).then_return(
decoy.when(mock_run_orchestrator_store.get_command_slice(1, 2, True)).then_return(
expected_command_slice
)

result = subject.get_commands_slice("run-id", 1, 2)
result = subject.get_commands_slice("run-id", 1, 2, include_fixit_commands=True)

assert expected_command_slice == result

Expand Down Expand Up @@ -929,7 +931,9 @@ def test_get_commands_slice_from_db_run_not_found(
mock_run_store.get_commands_slice(run_id="run-id", cursor=1, length=2)
).then_raise(RunNotFoundError(run_id="run-id"))
with pytest.raises(RunNotFoundError):
subject.get_commands_slice(run_id="run-id", cursor=1, length=2)
subject.get_commands_slice(
run_id="run-id", cursor=1, length=2, include_fixit_commands=True
)


def test_get_current_command(
Expand Down

0 comments on commit be7bd0b

Please sign in to comment.