Skip to content

Commit

Permalink
return error for active run, add tests, improved docstrings
Browse files Browse the repository at this point in the history
  • Loading branch information
sanni-t committed Apr 29, 2024
1 parent bbe73d7 commit a35f811
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 11 deletions.
37 changes: 29 additions & 8 deletions robot-server/robot_server/runs/router/commands_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from robot_server.robot.control.dependencies import require_estop_in_good_state

from ..run_models import RunCommandSummary
from ..run_data_manager import RunDataManager
from ..run_data_manager import RunDataManager, PreSerializedCommandsNotAvailableError
from ..engine_store import EngineStore
from ..run_store import RunStore, CommandNotFoundError
from ..run_models import RunNotFoundError
Expand Down Expand Up @@ -72,6 +72,18 @@ class CommandNotAllowed(ErrorDetails):
title: str = "Command Not Allowed"


class PreSerializedCommandsNotAvailable(ErrorDetails):
"""An error if one tries to fetch pre-serialized commands before they are written to the database."""

id: Literal[
"PreSerializedCommandsNotAvailable"
] = "PreSerializedCommandsNotAvailable"
title: str = "Pre-Serialized commands not available."
detail: str = (
"Pre-serialized commands are only available once a run has finished running."
)


class CommandLinkMeta(BaseModel):
"""Metadata about a command resource referenced in `links`."""

Expand Down Expand Up @@ -356,24 +368,30 @@ async def get_run_commands(
@PydanticResponse.wrap_route(
commands_router.get,
path="/runs/{runId}/commandsAsPreSerializedList",
summary="Get all commands as a list of pre-serialized commands",
summary="Get all commands of a completed run as a list of pre-serialized commands",
description=(
"Get all commands of a run as a list of pre-serialized commands."
"Get all commands of a completed run as a list of pre-serialized commands."
"**Warning:** This endpoint is experimental. We may change or remove it without warning."
"\n\n"
" This is a faster alternative to fetching *all* commands using `GET /runs/{runId}/commands`"
" For large protocols (10k+ commands), the above endpoint can take minutes to respond,"
" whereas this one should only take a few seconds."
"The commands list will only be available after a run has completed"
" (whether successful, failed or stopped) and its data has been committed to the database."
" If a request is received before the run is completed, it will return a 503 Unavailable error."
" This is a faster alternative to fetching the full commands list using"
" `GET /runs/{runId}/commands`. For large protocols (10k+ commands), the above"
" endpoint can take minutes to respond, whereas this one should only take a few seconds."
),
responses={
status.HTTP_404_NOT_FOUND: {"model": ErrorBody[RunNotFound]},
status.HTTP_503_SERVICE_UNAVAILABLE: {
"model": ErrorBody[PreSerializedCommandsNotAvailable]
},
},
)
async def get_run_commands_as_pre_serialized_list(
runId: str,
run_data_manager: RunDataManager = Depends(get_run_data_manager),
) -> PydanticResponse[SimpleMultiBody[str]]:
"""Get all commands in a run as a list of pre-serialized (string encoded) commands.
"""Get all commands of a completed run as a list of pre-serialized (string encoded) commands.
Arguments:
runId: Requested run ID, from the URL
Expand All @@ -383,7 +401,10 @@ async def get_run_commands_as_pre_serialized_list(
commands = run_data_manager.get_all_commands_as_preserialized_list(runId)
except RunNotFoundError as e:
raise RunNotFound.from_exc(e).as_error(status.HTTP_404_NOT_FOUND) from e

except PreSerializedCommandsNotAvailableError as e:
raise PreSerializedCommandsNotAvailable.from_exc(e).as_error(
status.HTTP_503_SERVICE_UNAVAILABLE
) from e
return await PydanticResponse.create(
content=SimpleMultiBody.construct(
data=commands, meta=MultiBodyMeta(cursor=0, totalLength=len(commands))
Expand Down
13 changes: 12 additions & 1 deletion robot-server/robot_server/runs/run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ class RunNotCurrentError(ValueError):
"""Error raised when a requested run is not the current run."""


class PreSerializedCommandsNotAvailableError(LookupError):
"""Error raised when a run's commands are not available as pre-serialized list of commands."""


class RunDataManager:
"""Collaborator to manage current and historical run data.
Expand Down Expand Up @@ -388,7 +392,14 @@ def get_command(self, run_id: str, command_id: str) -> Command:
return self._run_store.get_command(run_id=run_id, command_id=command_id)

def get_all_commands_as_preserialized_list(self, run_id: str) -> List[str]:
"""Get all commands of a run in a stringified json doc."""
"""Get all commands of a run in a serialized json list."""
if (
run_id == self._engine_store.current_run_id
and not self._engine_store.engine.state_view.commands.get_is_terminal()
):
raise PreSerializedCommandsNotAvailableError(
"Pre-serialized commands are only available after a run has ended."
)
return self._run_store.get_all_commands_as_preserialized_list(run_id)

def _get_state_summary(self, run_id: str) -> Union[StateSummary, BadStateSummary]:
Expand Down
1 change: 0 additions & 1 deletion robot-server/robot_server/runs/run_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ def get_commands_slice(
)
.order_by(run_command_table.c.index_in_run)
)
# select_all_cmds_as_doc = sqlalchemy.select(sqlalchemy.func.json_array_elements)
slice_result = transaction.execute(select_slice).all()

sliced_commands: List[Command] = [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
from copy import deepcopy
from datetime import datetime
from typing import Any, AsyncGenerator, Dict, NamedTuple, cast
Expand Down Expand Up @@ -250,6 +251,9 @@ async def test_run_commands_persist(client_and_server: ClientServerFixture) -> N
get_persisted_command_response = await client.get_run_command(
run_id=run_id, command_id=command_id
)
get_preserialized_commands_response = await client.get_preserialized_commands(
run_id=run_id
)

# ensure the persisted commands still match the original ones
assert get_all_persisted_commands_response.json()["data"] == [
Expand All @@ -259,6 +263,11 @@ async def test_run_commands_persist(client_and_server: ClientServerFixture) -> N
]
assert get_persisted_command_response.json()["data"] == expected_command

json_converted_command = json.loads(
get_preserialized_commands_response.json()["data"][0]
)
assert json_converted_command == expected_command


async def test_runs_completed_started_at_persist_via_actions_router(
client_and_server: ClientServerFixture,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,19 @@ stages:
createdAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$"
startedAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$"
completedAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$"

- name: Get all the commands in the run as a pre-serialized list
request:
url: '{ot2_server_base_url}/runs/{run_id}/commandsAsPreSerializedList'
method: GET
response:
status_code: 200
json:
data:
- !anystr
- !anystr
- !anystr
- !anystr
meta:
cursor: 0
totalLength: 4
8 changes: 8 additions & 0 deletions robot-server/tests/integration/robot_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,14 @@ async def get_run_command(self, run_id: str, command_id: str) -> Response:
response.raise_for_status()
return response

async def get_preserialized_commands(self, run_id: str) -> Response:
"""GET /runs/:run_id/commandsAsPreSerializedList."""
response = await self.httpx_client.get(
url=f"{self.base_url}/runs/{run_id}/commandsAsPreSerializedList",
)
response.raise_for_status()
return response

async def post_labware_offset(
self,
run_id: str,
Expand Down
38 changes: 37 additions & 1 deletion robot-server/tests/runs/test_run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@

from robot_server.protocols.protocol_store import ProtocolResource
from robot_server.runs.engine_store import EngineStore, EngineConflictError
from robot_server.runs.run_data_manager import RunDataManager, RunNotCurrentError
from robot_server.runs.run_data_manager import (
RunDataManager,
RunNotCurrentError,
PreSerializedCommandsNotAvailableError,
)
from robot_server.runs.run_models import Run, BadRun, RunNotFoundError, RunDataError
from robot_server.runs.run_store import (
RunStore,
Expand Down Expand Up @@ -932,6 +936,38 @@ def test_get_command_from_db_command_not_found(
subject.get_command("run-id", "command-id")


def test_get_all_commands_as_preserialized_list(
decoy: Decoy,
subject: RunDataManager,
mock_run_store: RunStore,
mock_engine_store: EngineStore,
) -> None:
"""It should return the pre-serialized commands list."""
decoy.when(mock_engine_store.current_run_id).then_return(None)
decoy.when(
mock_run_store.get_all_commands_as_preserialized_list("run-id")
).then_return(['{"id": command-1}', '{"id": command-2}'])
assert subject.get_all_commands_as_preserialized_list("run-id") == [
'{"id": command-1}',
'{"id": command-2}',
]


def test_get_all_commands_as_preserialized_list_errors_for_active_runs(
decoy: Decoy,
subject: RunDataManager,
mock_run_store: RunStore,
mock_engine_store: EngineStore,
) -> None:
"""It should raise an error when fetching pre-serialized commands list while run is active."""
decoy.when(mock_engine_store.current_run_id).then_return("current-run-id")
decoy.when(
mock_engine_store.engine.state_view.commands.get_is_terminal()
).then_return(False)
with pytest.raises(PreSerializedCommandsNotAvailableError):
subject.get_all_commands_as_preserialized_list("current-run-id")


async def test_get_current_run_labware_definition(
decoy: Decoy,
mock_engine_store: EngineStore,
Expand Down
28 changes: 28 additions & 0 deletions robot-server/tests/runs/test_run_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,3 +734,31 @@ def test_get_commands_slice_run_not_found(subject: RunStore) -> None:
)
with pytest.raises(RunNotFoundError):
subject.get_commands_slice(run_id="not-run-id", cursor=1, length=3)


def test_get_all_commands_as_preserialized_list(
subject: RunStore,
protocol_commands: List[pe_commands.Command],
state_summary: StateSummary,
) -> None:
"""It should get all commands stored in DB as a pre-serialized list."""
subject.insert(
run_id="run-id",
protocol_id=None,
created_at=datetime(year=2021, month=1, day=1, tzinfo=timezone.utc),
)
subject.update_run_state(
run_id="run-id",
summary=state_summary,
commands=protocol_commands,
run_time_parameters=[],
)
result = subject.get_all_commands_as_preserialized_list(run_id="run-id")
assert result == [
'{"id": "pause-1", "createdAt": "2021-01-01T00:00:00", "commandType": "waitForResume",'
' "key": "command-key", "status": "succeeded", "params": {"message": "hello world"}, "result": {}}',
'{"id": "pause-2", "createdAt": "2022-02-02T00:00:00", "commandType": "waitForResume",'
' "key": "command-key", "status": "succeeded", "params": {"message": "hey world"}, "result": {}}',
'{"id": "pause-3", "createdAt": "2023-03-03T00:00:00", "commandType": "waitForResume",'
' "key": "command-key", "status": "succeeded", "params": {"message": "sup world"}, "result": {}}',
]

0 comments on commit a35f811

Please sign in to comment.