Skip to content

Commit

Permalink
refactor(api,shared-data): fix v10 command parsing (#16420)
Browse files Browse the repository at this point in the history
We actually missed some stuff when adding command schema v10, and we
missed it in part because it's all over the place.

This commit instead removes some of the duplicative json protocol
parsing alongside adding better support for v10 protocols:

- Loosen the definition of commands in protocol_schema_v8, including
entirely removing the weird half-measure all-optional param parsing, so
you don't have to add new things there when you add a new command
parameter kind
- Improve the formatting of the errors raised by json protocols that
aren't valid because they have bad command, labware, or liquid schema
IDs (because those are the ones we actually parse in python)

Closes EXEC-753
  • Loading branch information
sfoster1 authored Oct 4, 2024
1 parent 12ebf27 commit 1b35652
Show file tree
Hide file tree
Showing 10 changed files with 3,256 additions and 194 deletions.
98 changes: 83 additions & 15 deletions api/src/opentrons/protocol_reader/file_format_validator.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""File format validation interface."""

from __future__ import annotations

from typing import Iterable

Expand Down Expand Up @@ -29,6 +29,16 @@
class FileFormatValidationError(ProtocolFilesInvalidError):
"""Raised when a file does not conform to the format it's supposed to."""

@classmethod
def _generic_json_failure(
cls, info: IdentifiedJsonMain, exc: Exception
) -> FileFormatValidationError:
return cls(
message=f"{info.original_file.name} could not be read as a JSON protocol.",
detail={"kind": "bad-json-protocol"},
wrapping=[PythonException(exc)],
)


class FileFormatValidator:
"""File format validation interface."""
Expand Down Expand Up @@ -61,22 +71,80 @@ def validate_sync() -> None:
await anyio.to_thread.run_sync(validate_sync)


def _handle_v8_json_protocol_validation_error(
info: IdentifiedJsonMain, pve: PydanticValidationError
) -> None:
for error in pve.errors():
if error["loc"] == ("commandSchemaId",) and error["type"] == "type_error.enum":
# type_error.enum is for "this entry is not in this enum" and happens if you constrain a field by
# annotating it with Enum, as we now do for command schema IDs
raise FileFormatValidationError(
message=(
f"{info.original_file.name} could not be read as a JSON protocol, in part because its command schema "
"id is unknown. This protocol may have been exported from a future version of authorship software. "
"Updating your Opentrons software may help."
),
detail={
"kind": "bad-command-schema-id",
"command-schema-id": info.unvalidated_json.get(
"commandSchemaId", "<unknown>"
),
},
wrapping=[PythonException(pve)],
) from pve
if (
error["loc"] == ("labwareDefinitionSchemaId",)
and error["type"] == "value_error.const"
):
# value_error.const is for "this entry is not one of these const values", which is different from type_error.enum
# for I'm sure a very good reason, and happens if you constrain a field by annotating it with a Literal
raise FileFormatValidationError(
message=(
f"{info.original_file.name} could not be read as a JSON protocol, in part because its labware schema "
"id is unknown. This protocol may have been exported from a future version of authorship software. "
"Updating your Opentrons software may help."
),
detail={
"kind": "bad-labware-schema-id",
"labware-schema-id": info.unvalidated_json.get(
"labwareDefinitionSchemaId", "<unknown>"
),
},
)
if error["loc"] == ("liquidSchemaId",) and error["type"] == "value_error.const":
raise FileFormatValidationError(
message=(
f"{info.original_file.name} could not be read as a JSON protocol, in part because its liquid schema "
"id is unknown. This protocol may have been exported from a future version of authorship software. "
"Updating your Opentrons software may help."
),
detail={
"kind": "bad-liquid-schema-id",
"liquid-schema-id": info.unvalidated_json.get(
"liquidSchemaId", "<unknown>"
),
},
)
else:
raise FileFormatValidationError._generic_json_failure(info, pve) from pve


async def _validate_json_protocol(info: IdentifiedJsonMain) -> None:
def validate_sync() -> None:
try:
if info.schema_version == 8:
if info.schema_version == 8:
try:
JsonProtocolV8.parse_obj(info.unvalidated_json)
elif info.schema_version == 7:
JsonProtocolV7.parse_obj(info.unvalidated_json)
elif info.schema_version == 6:
JsonProtocolV6.parse_obj(info.unvalidated_json)
else:
JsonProtocolUpToV5.parse_obj(info.unvalidated_json)
except PydanticValidationError as e:
raise FileFormatValidationError(
message=f"{info.original_file.name} could not be read as a JSON protocol.",
detail={"kind": "bad-json-protocol"},
wrapping=[PythonException(e)],
) from e
except PydanticValidationError as pve:
_handle_v8_json_protocol_validation_error(info, pve)
else:
try:
if info.schema_version == 7:
JsonProtocolV7.parse_obj(info.unvalidated_json)
elif info.schema_version == 6:
JsonProtocolV6.parse_obj(info.unvalidated_json)
else:
JsonProtocolUpToV5.parse_obj(info.unvalidated_json)
except PydanticValidationError as e:
raise FileFormatValidationError._generic_json_failure(info, e) from e

await anyio.to_thread.run_sync(validate_sync)
26 changes: 21 additions & 5 deletions api/src/opentrons/protocol_runner/json_translator.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Translation of JSON protocol commands into ProtocolEngine commands."""
from typing import cast, List, Union
from pydantic import parse_obj_as
from typing import cast, List, Union, Iterator
from pydantic import parse_obj_as, ValidationError as PydanticValidationError

from opentrons_shared_data.pipette.types import PipetteNameType
from opentrons_shared_data.protocol.models import (
Expand All @@ -12,6 +12,7 @@
protocol_schema_v8,
)
from opentrons_shared_data import command as command_schema
from opentrons_shared_data.errors.exceptions import InvalidProtocolData, PythonException

from opentrons.types import MountType
from opentrons.protocol_engine import (
Expand Down Expand Up @@ -196,7 +197,7 @@ class JsonTranslator:
"""Class that translates commands/liquids from PD/JSON to ProtocolEngine."""

def translate_liquids(
self, protocol: Union[ProtocolSchemaV6, ProtocolSchemaV7]
self, protocol: Union[ProtocolSchemaV6, ProtocolSchemaV7, ProtocolSchemaV8]
) -> List[Liquid]:
"""Takes json protocol v6 and translates liquids->protocol engine liquids."""
protocol_liquids = protocol.liquids or {}
Expand Down Expand Up @@ -258,7 +259,8 @@ def _translate_v8_commands(
self, protocol: ProtocolSchemaV8
) -> List[pe_commands.CommandCreate]:
"""Translate commands in json protocol schema v8, which might be of different command schemas."""
command_schema_ref = protocol.commandSchemaId
command_schema_ref = protocol.commandSchemaId.value

# these calls will raise if the command schema version is invalid or unknown
command_schema_version = command_schema.schema_version_from_ref(
command_schema_ref
Expand All @@ -267,4 +269,18 @@ def _translate_v8_commands(
command_schema_version
)

return [_translate_simple_command(command) for command in protocol.commands]
def translate_all_commands() -> Iterator[pe_commands.CommandCreate]:
for command in protocol.commands:
try:
yield _translate_simple_command(command)
except PydanticValidationError as pve:
raise InvalidProtocolData(
message=(
"The protocol is invalid because it contains an unknown or malformed command, "
f'"{command.commandType}".'
),
detail={"kind": "invalid-command"},
wrapping=[PythonException(pve)],
)

return list(translate_all_commands())
Loading

0 comments on commit 1b35652

Please sign in to comment.