diff --git a/api/src/opentrons/protocol_api/_parameter_context.py b/api/src/opentrons/protocol_api/_parameter_context.py index 8ca2bdb2c2a..2e0e0096f44 100644 --- a/api/src/opentrons/protocol_api/_parameter_context.py +++ b/api/src/opentrons/protocol_api/_parameter_context.py @@ -15,6 +15,7 @@ from opentrons.protocols.parameters.exceptions import ( ParameterDefinitionError, ParameterValueError, + IncompatibleParameterError, ) from opentrons.protocol_engine.types import ( RunTimeParameter, @@ -183,6 +184,14 @@ def add_csv_file( variable_name: The variable name the CSV parameter will be referred to in the run context. description: A description of the parameter as it will show up on the frontend. """ + if any( + isinstance(parameter, csv_parameter_definition.CSVParameterDefinition) + for parameter in self._parameters.values() + ): + raise IncompatibleParameterError( + "Only one CSV File parameter can be defined per protocol." + ) + validation.validate_variable_name_unique(variable_name, set(self._parameters)) parameter = csv_parameter_definition.create_csv_parameter( display_name=display_name, @@ -246,8 +255,11 @@ def initialize_csv_files( file_id = file_path.parent.name file_name = file_path.name + with file_path.open("rb") as fh: + contents = fh.read() + parameter.file_info = FileInfo(id=file_id, name=file_name) - parameter.value = file_path + parameter.value = contents def export_parameters_for_analysis(self) -> List[RunTimeParameter]: """Exports all parameters into a protocol engine models for reporting in analysis. diff --git a/api/src/opentrons/protocols/execution/execute.py b/api/src/opentrons/protocols/execution/execute.py index ff52b3b1dc5..6dd74bcb778 100644 --- a/api/src/opentrons/protocols/execution/execute.py +++ b/api/src/opentrons/protocols/execution/execute.py @@ -16,7 +16,6 @@ from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.parameters.csv_parameter_interface import CSVParameter -from opentrons.protocols.parameters.exceptions import RuntimeParameterRequired MODULE_LOG = logging.getLogger(__name__) @@ -51,13 +50,8 @@ def run_protocol( finally: if protocol.api_level >= APIVersion(2, 18): for parameter in context.params.get_all().values(): - if isinstance(parameter, CSVParameter): - try: - parameter.file.close() - # This will be raised if the csv file wasn't set, which means it was never opened, - # so we can safely skip this. - except RuntimeParameterRequired: - pass + if isinstance(parameter, CSVParameter) and parameter.file_opened: + parameter.file.close() else: if protocol.contents["schemaVersion"] == 3: ins = execute_json_v3.load_pipettes_from_json(context, protocol.contents) diff --git a/api/src/opentrons/protocols/parameters/csv_parameter_definition.py b/api/src/opentrons/protocols/parameters/csv_parameter_definition.py index d23b7d70f0b..c48356d01d4 100644 --- a/api/src/opentrons/protocols/parameters/csv_parameter_definition.py +++ b/api/src/opentrons/protocols/parameters/csv_parameter_definition.py @@ -1,5 +1,4 @@ """CSV Parameter definition and associated classes/functions.""" -from pathlib import Path from typing import Optional from opentrons.protocol_engine.types import ( @@ -14,7 +13,7 @@ from .csv_parameter_interface import CSVParameter -class CSVParameterDefinition(AbstractParameterDefinition[Optional[Path]]): +class CSVParameterDefinition(AbstractParameterDefinition[Optional[bytes]]): """The definition for a user defined CSV file parameter.""" def __init__( @@ -30,7 +29,7 @@ def __init__( self._display_name = validation.ensure_display_name(display_name) self._variable_name = validation.ensure_variable_name(variable_name) self._description = validation.ensure_description(description) - self._value: Optional[Path] = None + self._value: Optional[bytes] = None self._file_info: Optional[FileInfo] = None @property @@ -39,13 +38,13 @@ def variable_name(self) -> str: return self._variable_name @property - def value(self) -> Optional[Path]: + def value(self) -> Optional[bytes]: """The current set file for the CSV parameter. Defaults to None on definition creation.""" return self._value @value.setter - def value(self, new_path: Path) -> None: - self._value = new_path + def value(self, contents: bytes) -> None: + self._value = contents @property def file_info(self) -> Optional[FileInfo]: @@ -56,7 +55,7 @@ def file_info(self, file_info: FileInfo) -> None: self._file_info = file_info def as_csv_parameter_interface(self, api_version: APIVersion) -> CSVParameter: - return CSVParameter(csv_path=self._value, api_version=api_version) + return CSVParameter(contents=self._value, api_version=api_version) def as_protocol_engine_type(self) -> RunTimeParameter: """Returns CSV parameter as a Protocol Engine type to send to client.""" diff --git a/api/src/opentrons/protocols/parameters/csv_parameter_interface.py b/api/src/opentrons/protocols/parameters/csv_parameter_interface.py index 40a099558d4..20627322547 100644 --- a/api/src/opentrons/protocols/parameters/csv_parameter_interface.py +++ b/api/src/opentrons/protocols/parameters/csv_parameter_interface.py @@ -1,35 +1,46 @@ import csv -from pathlib import Path +from tempfile import NamedTemporaryFile from typing import Optional, TextIO, Any, List from opentrons.protocols.api_support.types import APIVersion -from . import parameter_file_reader -from .exceptions import ParameterValueError +from .exceptions import ParameterValueError, RuntimeParameterRequired # TODO(jbl 2024-08-02) This is a public facing class and as such should be moved to the protocol_api folder class CSVParameter: - def __init__(self, csv_path: Optional[Path], api_version: APIVersion) -> None: - self._path = csv_path - self._file: Optional[TextIO] = None - self._contents: Optional[str] = None + def __init__(self, contents: Optional[bytes], api_version: APIVersion) -> None: + self._contents = contents self._api_version = api_version + self._file: Optional[TextIO] = None @property def file(self) -> TextIO: """Returns the file handler for the CSV file.""" if self._file is None: - self._file = parameter_file_reader.open_file_path(self._path) + text = self.contents + temporary_file = NamedTemporaryFile("r+") + temporary_file.write(text) + temporary_file.flush() + + # Open a new file handler for the temporary file with read-only permissions and close the other + self._file = open(temporary_file.name, "r") + temporary_file.close() return self._file + @property + def file_opened(self) -> bool: + """Return if a file handler has been opened for the CSV parameter.""" + return self._file is not None + @property def contents(self) -> str: """Returns the full contents of the CSV file as a single string.""" if self._contents is None: - self.file.seek(0) - self._contents = self.file.read() - return self._contents + raise RuntimeParameterRequired( + "CSV parameter needs to be set to a file for full analysis or run." + ) + return self._contents.decode("utf-8") def parse_as_csv( self, detect_dialect: bool = True, **kwargs: Any @@ -42,23 +53,20 @@ def parse_as_csv( rows: List[List[str]] = [] if detect_dialect: try: - self.file.seek(0) - dialect = csv.Sniffer().sniff(self.file.read(1024)) - self.file.seek(0) - reader = csv.reader(self.file, dialect, **kwargs) + dialect = csv.Sniffer().sniff(self.contents[:1024]) + reader = csv.reader(self.contents.split("\n"), dialect, **kwargs) except (UnicodeDecodeError, csv.Error): raise ParameterValueError( - "Cannot parse dialect or contents from provided CSV file." + "Cannot parse dialect or contents from provided CSV contents." ) else: try: - reader = csv.reader(self.file, **kwargs) + reader = csv.reader(self.contents.split("\n"), **kwargs) except (UnicodeDecodeError, csv.Error): - raise ParameterValueError("Cannot parse provided CSV file.") + raise ParameterValueError("Cannot parse provided CSV contents.") try: for row in reader: rows.append(row) except (UnicodeDecodeError, csv.Error): - raise ParameterValueError("Cannot parse provided CSV file.") - self.file.seek(0) + raise ParameterValueError("Cannot parse provided CSV contents.") return rows diff --git a/api/src/opentrons/protocols/parameters/exceptions.py b/api/src/opentrons/protocols/parameters/exceptions.py index 9f7bcee009c..7f1b8a933cb 100644 --- a/api/src/opentrons/protocols/parameters/exceptions.py +++ b/api/src/opentrons/protocols/parameters/exceptions.py @@ -28,3 +28,7 @@ class ParameterDefinitionError(ValueError): class ParameterNameError(ValueError): """An error raised when a parameter name or description is not valid.""" + + +class IncompatibleParameterError(ValueError): + """An error raised when a parameter conflicts with another parameter.""" diff --git a/api/src/opentrons/protocols/parameters/parameter_file_reader.py b/api/src/opentrons/protocols/parameters/parameter_file_reader.py deleted file mode 100644 index 9a39c2fa0dc..00000000000 --- a/api/src/opentrons/protocols/parameters/parameter_file_reader.py +++ /dev/null @@ -1,26 +0,0 @@ -from pathlib import Path -from tempfile import NamedTemporaryFile -from typing import Optional, TextIO - -from .exceptions import RuntimeParameterRequired - - -def open_file_path(file_path: Optional[Path]) -> TextIO: - """Ensure file path is set and open up the file in a safe read-only temporary file.""" - if file_path is None: - raise RuntimeParameterRequired( - "CSV parameter needs to be set to a file for full analysis or run." - ) - # Read the contents of the actual file - with file_path.open() as fh: - contents = fh.read() - - # Open a temporary file with write permissions and write contents to that - temporary_file = NamedTemporaryFile("r+") - temporary_file.write(contents) - temporary_file.flush() - - # Open a new file handler for the temporary file with read-only permissions and close the other - read_only_temp_file = open(temporary_file.name, "r") - temporary_file.close() - return read_only_temp_file diff --git a/api/src/opentrons/protocols/parameters/types.py b/api/src/opentrons/protocols/parameters/types.py index 631d686b7e7..0e248d8e1c0 100644 --- a/api/src/opentrons/protocols/parameters/types.py +++ b/api/src/opentrons/protocols/parameters/types.py @@ -1,11 +1,10 @@ -from pathlib import Path from typing import TypeVar, Union, TypedDict from .csv_parameter_interface import CSVParameter PrimitiveAllowedTypes = Union[str, int, float, bool] -AllAllowedTypes = Union[str, int, float, bool, Path, None] +AllAllowedTypes = Union[str, int, float, bool, bytes, None] UserFacingTypes = Union[str, int, float, bool, CSVParameter] ParamType = TypeVar("ParamType", bound=AllAllowedTypes) diff --git a/api/src/opentrons/protocols/parameters/validation.py b/api/src/opentrons/protocols/parameters/validation.py index 68a1d93ec97..d111e1cc8a8 100644 --- a/api/src/opentrons/protocols/parameters/validation.py +++ b/api/src/opentrons/protocols/parameters/validation.py @@ -237,7 +237,7 @@ def validate_type(value: ParamType, parameter_type: type) -> None: """Validate parameter value is the correct type.""" if not isinstance(value, parameter_type): raise ParameterValueError( - f"Parameter value {value} has type '{type(value).__name__}'," + f"Parameter value {value!r} has type '{type(value).__name__}'," f" but must be of type '{parameter_type.__name__}'." ) @@ -252,7 +252,7 @@ def validate_options( """Validate default values and all possible constraints for a valid parameter definition.""" if not isinstance(default, parameter_type): raise ParameterValueError( - f"Parameter default {default} has type '{type(default).__name__}'," + f"Parameter default {default!r} has type '{type(default).__name__}'," f" but must be of type '{parameter_type.__name__}'." ) diff --git a/api/tests/opentrons/protocol_api/test_parameter_context.py b/api/tests/opentrons/protocol_api/test_parameter_context.py index e2004a4b9b7..f59521c78a9 100644 --- a/api/tests/opentrons/protocol_api/test_parameter_context.py +++ b/api/tests/opentrons/protocol_api/test_parameter_context.py @@ -13,7 +13,10 @@ parameter_definition as mock_parameter_definition, validation as mock_validation, ) -from opentrons.protocols.parameters.exceptions import ParameterDefinitionError +from opentrons.protocols.parameters.exceptions import ( + ParameterDefinitionError, + IncompatibleParameterError, +) from opentrons.protocol_engine.types import BooleanParameter from opentrons.protocol_api._parameter_context import ParameterContext @@ -196,7 +199,7 @@ def test_add_string(decoy: Decoy, subject: ParameterContext) -> None: def test_add_csv(decoy: Decoy, subject: ParameterContext) -> None: """It should create and add a CSV parameter definition.""" subject._parameters["other_param"] = decoy.mock( - cls=mock_csv_parameter_definition.CSVParameterDefinition + cls=mock_parameter_definition.ParameterDefinition ) param_def = decoy.mock(cls=mock_csv_parameter_definition.CSVParameterDefinition) decoy.when(param_def.variable_name).then_return("my potentially cool variable") @@ -220,6 +223,22 @@ def test_add_csv(decoy: Decoy, subject: ParameterContext) -> None: ) +def test_add_csv_raises_for_multiple_csvs( + decoy: Decoy, subject: ParameterContext +) -> None: + """It should raise with a IncompatibleParameterError if there's already a CSV parameter..""" + subject._parameters["other_param"] = decoy.mock( + cls=mock_csv_parameter_definition.CSVParameterDefinition + ) + + with pytest.raises(IncompatibleParameterError): + subject.add_csv_file( + display_name="jkl", + variable_name="qwerty", + description="fee foo fum", + ) + + def test_set_parameters(decoy: Decoy, subject: ParameterContext) -> None: """It should set the parameter values.""" param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition) diff --git a/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py b/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py index 04dee0512b5..ac8da823edb 100644 --- a/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py +++ b/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py @@ -1,6 +1,5 @@ """Tests for the CSV Parameter Definitions.""" import inspect -from pathlib import Path import pytest from decoy import Decoy @@ -62,9 +61,9 @@ def test_create_csv_parameter(decoy: Decoy) -> None: def test_set_csv_value( decoy: Decoy, csv_parameter_subject: CSVParameterDefinition ) -> None: - """It should set the CSV parameter value to a path.""" - csv_parameter_subject.value = Path("123") - assert csv_parameter_subject.value == Path("123") + """It should set the CSV parameter value to a byte string.""" + csv_parameter_subject.value = b"123" + assert csv_parameter_subject.value == b"123" def test_csv_parameter_as_protocol_engine_type( @@ -79,13 +78,13 @@ def test_csv_parameter_as_protocol_engine_type( file=None, ) - csv_parameter_subject.file_info = FileInfo(id="123abc", name="") + csv_parameter_subject.file_info = FileInfo(id="123", name="abc") result = csv_parameter_subject.as_protocol_engine_type() assert result == CSVParameter( displayName="My cool CSV", variableName="my_cool_csv", description="Comma Separated Value", - file=FileInfo(id="123abc", name=""), + file=FileInfo(id="123", name="abc"), ) @@ -98,6 +97,6 @@ def test_csv_parameter_as_csv_parameter_interface( with pytest.raises(RuntimeParameterRequired): result.file - csv_parameter_subject.value = Path("abc") + csv_parameter_subject.value = b"abc" result = csv_parameter_subject.as_csv_parameter_interface(api_version) - assert result._path == Path("abc") + assert result.contents == "abc" diff --git a/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py b/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py index 4cd9e649b63..81bffd0028e 100644 --- a/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py +++ b/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py @@ -1,26 +1,13 @@ import pytest -import inspect +import platform from decoy import Decoy from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] -import tempfile -from pathlib import Path -from typing import TextIO, Generator - from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.api_support.definitions import MAX_SUPPORTED_VERSION -from opentrons.protocols.parameters import ( - parameter_file_reader as mock_param_file_reader, -) from opentrons.protocols.parameters.csv_parameter_interface import CSVParameter -@pytest.fixture(autouse=True) -def _patch_parameter_file_reader(decoy: Decoy, monkeypatch: pytest.MonkeyPatch) -> None: - for name, func in inspect.getmembers(mock_param_file_reader, inspect.isfunction): - monkeypatch.setattr(mock_param_file_reader, name, decoy.mock(func=func)) - - @pytest.fixture def api_version() -> APIVersion: """The API version under test.""" @@ -28,70 +15,54 @@ def api_version() -> APIVersion: @pytest.fixture -def csv_file_basic() -> Generator[TextIO, None, None]: +def csv_file_basic() -> bytes: """A basic CSV file with quotes around strings.""" - with tempfile.TemporaryFile("r+") as temp_file: - contents = '"x","y","z"\n"a",1,2\n"b",3,4\n"c",5,6' - temp_file.write(contents) - temp_file.seek(0) - yield temp_file + return b'"x","y","z"\n"a",1,2\n"b",3,4\n"c",5,6' @pytest.fixture -def csv_file_no_quotes() -> Generator[TextIO, None, None]: +def csv_file_no_quotes() -> bytes: """A basic CSV file with no quotes around strings.""" - with tempfile.TemporaryFile("r+") as temp_file: - contents = "x,y,z\na,1,2\nb,3,4\nc,5,6" - temp_file.write(contents) - temp_file.seek(0) - yield temp_file + return b"x,y,z\na,1,2\nb,3,4\nc,5,6" @pytest.fixture -def csv_file_preceding_spaces() -> Generator[TextIO, None, None]: +def csv_file_preceding_spaces() -> bytes: """A basic CSV file with quotes around strings and spaces preceding non-initial columns.""" - with tempfile.TemporaryFile("r+") as temp_file: - contents = '"x", "y", "z"\n"a", 1, 2\n"b", 3, 4\n"c", 5, 6' - temp_file.write(contents) - temp_file.seek(0) - yield temp_file + return b'"x", "y", "z"\n"a", 1, 2\n"b", 3, 4\n"c", 5, 6' @pytest.fixture -def csv_file_mixed_quotes() -> Generator[TextIO, None, None]: +def csv_file_mixed_quotes() -> bytes: """A basic CSV file with both string quotes and escaped quotes.""" - with tempfile.TemporaryFile("r+") as temp_file: - contents = 'head,er\n"a,b,c",def\n"""ghi""","jkl"' - temp_file.write(contents) - temp_file.seek(0) - yield temp_file + return b'head,er\n"a,b,c",def\n"""ghi""","jkl"' @pytest.fixture -def csv_file_different_delimiter() -> Generator[TextIO, None, None]: +def csv_file_different_delimiter() -> bytes: """A basic CSV file with a non-comma delimiter.""" - with tempfile.TemporaryFile("r+") as temp_file: - contents = "x:y:z\na,:1,:2\nb,:3,:4\nc,:5,:6" - temp_file.write(contents) - temp_file.seek(0) - yield temp_file - - -@pytest.fixture -def subject(api_version: APIVersion) -> CSVParameter: - """Return a CSVParameter interface subject.""" - return CSVParameter(csv_path=Path("abc"), api_version=api_version) + return b"x:y:z\na,:1,:2\nb,:3,:4\nc,:5,:6" def test_csv_parameter( - decoy: Decoy, csv_file_basic: TextIO, subject: CSVParameter + decoy: Decoy, api_version: APIVersion, csv_file_basic: bytes ) -> None: """It should load the CSV parameter and provide access to the file, contents, and rows.""" - decoy.when(mock_param_file_reader.open_file_path(Path("abc"))).then_return( - csv_file_basic - ) - assert subject.file is csv_file_basic + subject = CSVParameter(csv_file_basic, api_version) + + # On Windows, you can't open a NamedTemporaryFile a second time, which breaks the code under test. + # Because of the way CSV analysis works this code will only ever be run on the actual OT-2/Flex hardware, + # so we skip testing and instead assert that we get a PermissionError on Windows (to ensure this + # test gets fixed in case we ever refactor the file opening.) + if platform.system() != "Windows": + assert subject.file.readable() + assert not subject.file.writable() + assert subject.file.read() == '"x","y","z"\n"a",1,2\n"b",3,4\n"c",5,6' + else: + with pytest.raises(PermissionError): + subject.file assert subject.contents == '"x","y","z"\n"a",1,2\n"b",3,4\n"c",5,6' + assert subject.parse_as_csv()[0] == ["x", "y", "z"] @pytest.mark.parametrize( @@ -103,22 +74,26 @@ def test_csv_parameter( ], ) def test_csv_parameter_rows( - decoy: Decoy, csv_file: TextIO, subject: CSVParameter + decoy: Decoy, + api_version: APIVersion, + csv_file: bytes, ) -> None: """It should load the rows as all strings even with no quotes or leading spaces.""" - decoy.when(mock_param_file_reader.open_file_path(Path("abc"))).then_return(csv_file) + subject = CSVParameter(csv_file, api_version) + assert len(subject.parse_as_csv()) == 4 assert subject.parse_as_csv()[0] == ["x", "y", "z"] assert subject.parse_as_csv()[1] == ["a", "1", "2"] def test_csv_parameter_mixed_quotes( - decoy: Decoy, csv_file_mixed_quotes: TextIO, subject: CSVParameter + decoy: Decoy, + api_version: APIVersion, + csv_file_mixed_quotes: bytes, ) -> None: """It should load the rows with no quotes, quotes and escaped quotes with double quotes.""" - decoy.when(mock_param_file_reader.open_file_path(Path("abc"))).then_return( - csv_file_mixed_quotes - ) + subject = CSVParameter(csv_file_mixed_quotes, api_version) + assert len(subject.parse_as_csv()) == 3 assert subject.parse_as_csv()[0] == ["head", "er"] assert subject.parse_as_csv()[1] == ["a,b,c", "def"] @@ -126,25 +101,27 @@ def test_csv_parameter_mixed_quotes( def test_csv_parameter_additional_kwargs( - decoy: Decoy, csv_file_different_delimiter: TextIO, subject: CSVParameter + decoy: Decoy, + api_version: APIVersion, + csv_file_different_delimiter: bytes, ) -> None: """It should load the rows with a different delimiter.""" - decoy.when(mock_param_file_reader.open_file_path(Path("abc"))).then_return( - csv_file_different_delimiter - ) + subject = CSVParameter(csv_file_different_delimiter, api_version) rows = subject.parse_as_csv(delimiter=":") + assert len(rows) == 4 assert rows[0] == ["x", "y", "z"] assert rows[1] == ["a,", "1,", "2"] def test_csv_parameter_dont_detect_dialect( - decoy: Decoy, csv_file_preceding_spaces: TextIO, subject: CSVParameter + decoy: Decoy, + api_version: APIVersion, + csv_file_preceding_spaces: bytes, ) -> None: """It should load the rows without trying to detect the dialect.""" - decoy.when(mock_param_file_reader.open_file_path(Path("abc"))).then_return( - csv_file_preceding_spaces - ) + subject = CSVParameter(csv_file_preceding_spaces, api_version) rows = subject.parse_as_csv(detect_dialect=False) + assert rows[0] == ["x", ' "y"', ' "z"'] assert rows[1] == ["a", " 1", " 2"] diff --git a/api/tests/opentrons/protocols/parameters/test_parameter_file_reader.py b/api/tests/opentrons/protocols/parameters/test_parameter_file_reader.py deleted file mode 100644 index d469c827d08..00000000000 --- a/api/tests/opentrons/protocols/parameters/test_parameter_file_reader.py +++ /dev/null @@ -1,34 +0,0 @@ -import pytest -import platform - -from opentrons_shared_data import get_shared_data_root, load_shared_data - -from opentrons.protocols.parameters.exceptions import RuntimeParameterRequired -from opentrons.protocols.parameters import parameter_file_reader as subject - - -def test_open_file_path() -> None: - """It should open a temporary file handler given a path.""" - contents = load_shared_data("protocol/fixtures/7/simpleV7.json") - shared_data_path = get_shared_data_root() / "protocol/fixtures/7/simpleV7.json" - - # On Windows, you can't open a NamedTemporaryFile a second time, which breaks the code under test. - # Because of the way CSV analysis works this code will only ever be run on the actual OT-2/Flex hardware, - # so we skip testing and instead assert that we get a PermissionError on Windows (to ensure this - # test gets fixed in case we ever refactor the file opening.) - if platform.system() != "Windows": - result = subject.open_file_path(shared_data_path) - - assert result.readable() - assert not result.writable() - assert result.read() == contents.decode("utf-8") - result.close() - else: - with pytest.raises(PermissionError): - subject.open_file_path(shared_data_path) - - -def test_open_file_path_raises() -> None: - """It should raise of no file path is provided.""" - with pytest.raises(RuntimeParameterRequired): - subject.open_file_path(None) diff --git a/robot-server/robot_server/protocols/router.py b/robot-server/robot_server/protocols/router.py index 2ea216f9f29..609b1d8e978 100644 --- a/robot-server/robot_server/protocols/router.py +++ b/robot-server/robot_server/protocols/router.py @@ -196,9 +196,10 @@ class ProtocolLinks(BaseModel): responses={ status.HTTP_200_OK: {"model": SimpleBody[Protocol]}, status.HTTP_201_CREATED: {"model": SimpleBody[Protocol]}, - status.HTTP_400_BAD_REQUEST: {"model": ErrorBody[FileIdNotFound]}, status.HTTP_422_UNPROCESSABLE_ENTITY: { - "model": ErrorBody[Union[ProtocolFilesInvalid, ProtocolRobotTypeMismatch]] + "model": ErrorBody[ + Union[ProtocolFilesInvalid, ProtocolRobotTypeMismatch, FileIdNotFound] + ] }, status.HTTP_503_SERVICE_UNAVAILABLE: {"model": ErrorBody[LastAnalysisPending]}, }, @@ -330,7 +331,9 @@ async def create_protocol( # noqa: C901 for name, file_id in parsed_rtp_files.items() } except FileIdNotFoundError as e: - raise FileIdNotFound(detail=str(e)).as_error(status.HTTP_400_BAD_REQUEST) + raise FileIdNotFound(detail=str(e)).as_error( + status.HTTP_422_UNPROCESSABLE_ENTITY + ) content_hash = await file_hasher.hash(buffered_files) cached_protocol_id = protocol_store.get_id_by_hash(content_hash) @@ -702,8 +705,8 @@ async def delete_protocol_by_id( responses={ status.HTTP_200_OK: {"model": SimpleMultiBody[AnalysisSummary]}, status.HTTP_201_CREATED: {"model": SimpleMultiBody[AnalysisSummary]}, - status.HTTP_400_BAD_REQUEST: {"model": ErrorBody[FileIdNotFound]}, status.HTTP_404_NOT_FOUND: {"model": ErrorBody[ProtocolNotFound]}, + status.HTTP_422_UNPROCESSABLE_ENTITY: {"model": ErrorBody[FileIdNotFound]}, status.HTTP_503_SERVICE_UNAVAILABLE: {"model": ErrorBody[LastAnalysisPending]}, }, ) @@ -743,7 +746,9 @@ async def create_protocol_analysis( for name, file_id in rtp_files.items() } except FileIdNotFoundError as e: - raise FileIdNotFound(detail=str(e)).as_error(status.HTTP_400_BAD_REQUEST) + raise FileIdNotFound(detail=str(e)).as_error( + status.HTTP_422_UNPROCESSABLE_ENTITY + ) try: ( diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index 7000882b965..1d101c978f9 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -149,8 +149,8 @@ async def get_run_data_from_url( status_code=status.HTTP_201_CREATED, responses={ status.HTTP_201_CREATED: {"model": SimpleBody[Run]}, - status.HTTP_400_BAD_REQUEST: {"model": ErrorBody[FileIdNotFound]}, status.HTTP_404_NOT_FOUND: {"model": ErrorBody[ProtocolNotFound]}, + status.HTTP_422_UNPROCESSABLE_ENTITY: {"model": ErrorBody[FileIdNotFound]}, status.HTTP_409_CONFLICT: {"model": ErrorBody[RunAlreadyActive]}, }, ) @@ -209,7 +209,9 @@ async def create_run( # noqa: C901 for name, file_id in rtp_files.items() } except FileIdNotFoundError as e: - raise FileIdNotFound(detail=str(e)).as_error(status.HTTP_400_BAD_REQUEST) + raise FileIdNotFound(detail=str(e)).as_error( + status.HTTP_422_UNPROCESSABLE_ENTITY + ) protocol_resource = None