feat(api, robot-server): allow setting CSV param files via protocols & analyses endpoints #15686

sanni-t · 2024-07-16T23:15:50Z

Closes AUTH-416

Overview

This PR adds the capability of setting the CSV run time parameter with a CSV file. Features added:

POST /protocols and POST /analyses now accept runTimeParameterFiles data in addition to the existing runTimeParameterValues data
Adds a table to save the CSV file IDs used in analyses
The CSV param is updated with the file ID mentioned

Test Plan

Upload the protocol in feat(api): add csv parameter definitions #15332 without supplying a csv file ID in the request. Test that:
- the protocol upload response contains the full run time parameters, including CSV parameter without any file info
- the protocol's analysis eventually completes with a status of ok and the run time parameters are present in the full completed analysis
Upload a CSV file to the /dataFiles endpoint, get its ID.
Upload the above protocol again, and provide the above CSV file ID with it using runTimeParameterFiles. Test that:
- the protocol upload response contains the full run time parameters, including CSV parameter and the file ID
- the protocol's analysis eventually completes with a status of ok and the CSV parameters with file ID are present in the full analysis
Test both points (1) & (3) using the POST /protocols/protocolId/analyses endpoint
Test that if you send the same protocol with the same RTP values and files, a new analysis is not started

Changelog

API:

updated the CSV param engine type to include file name in file property
updated PythonProtocolRunner.load & supporting functions to accept CSV file ID overrides and use them to validate and set CSV parameters.
added a function to initialize the CSV parameter with the file and file info (right now only sets the file ID)

Robot-server:

added analysis csv params DB table
updated POST /protocols and POST /analyses to accept runTimeParameterFiles in their form-data & request body, respectively.
updated analysis stores to update the csv table with CSV file IDs

Review requests

Test with the app and make sure the app requirements are fulfilled.
Usual code sanity checks.

Risk assessment

None- low. Adds a new capability to the protocols endpoints and doesn't change existing code much.

github-actions · 2024-07-18T22:18:28Z

A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/

sanni-t · 2024-07-18T22:44:13Z

api/src/opentrons/protocol_engine/types.py

+    file: Optional[FileInfo] = Field(
+        default=None,
+        description="ID of the CSV file stored on the robot; to be used for fetching the CSV file."
+        " For local analysis this will most likely be empty.",


@ncdiehl @koji : reverted this to be a nested file property and added name field to the data type.

sanni-t · 2024-07-18T22:45:23Z

...rver/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml

+                  file:
+                  - id: '{csv_file_id}'
+                    name: ''


@ncdiehl @koji see this for example CSV param in analysis response

github-actions · 2024-07-18T22:51:35Z

A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/

jbleon95 · 2024-07-22T13:52:47Z

api/src/opentrons/protocol_api/_parameter_context.py

 from opentrons.protocol_engine.types import (
    RunTimeParameter,
    PrimitiveRunTimeParamValuesType,
+    CsvRunTimeParamFilesType,


Little nitpick, I think we should always capitalize CSV if we're doing camelcase, it makes it stand out more.

jbleon95 · 2024-07-22T14:17:29Z

robot-server/robot_server/protocols/completed_analysis_store.py

+        with self._sql_engine.begin() as transaction:
+            results = transaction.execute(statement).all()
+
+        csv_rtps: Dict[str, Union[str, None]] = {}


Suggested change

csv_rtps: Dict[str, Union[str, None]] = {}

csv_rtps: Dict[str, Optional[str]] = {}

jbleon95

Some minor nitpicks with style and the like, but this is looking good!

jbleon95 · 2024-07-22T14:18:33Z

robot-server/robot_server/protocols/completed_analysis_store.py


        with self._sql_engine.begin() as transaction:
            transaction.execute(delete_primitive_rtp_statement)
+            transaction.execute(delete_csv_rtp_statement)
+            transaction.execute(delete_csv_rtp_statement)


Line repeated, unless we want to make sure the file is really deleted 😛

jbleon95 · 2024-07-22T14:19:35Z

robot-server/robot_server/protocols/rtp_resources.py

+
+    analysis_id: str
+    parameter_variable_name: str
+    file_id: Union[str, None]


Suggested change

file_id: Union[str, None]

file_id: Optional[str]

jbleon95

Besides one more teensy nitpick this looks great.

jbleon95 · 2024-07-26T19:23:20Z

robot-server/robot_server/protocols/rtp_resources.py

+
+
+@dataclass
+class CsvParameterResource:


Sorry to be nitpicky on this but for style consistency lets also have this be CSVParameterResource

…nd getters

…ts, wire it up to set the csv file id

sanni-t marked this pull request as ready for review July 18, 2024 22:41

sanni-t requested review from a team as code owners July 18, 2024 22:42

sanni-t commented Jul 18, 2024

View reviewed changes

jbleon95 reviewed Jul 22, 2024

View reviewed changes

sanni-t force-pushed the AUTH-416-accept-csv-file-ids-and-save-to-csv-table branch from 5282993 to c7fff38 Compare July 23, 2024 18:41

jbleon95 approved these changes Jul 26, 2024

View reviewed changes

sanni-t force-pushed the AUTH-416-accept-csv-file-ids-and-save-to-csv-table branch 2 times, most recently from cd06d7d to a252211 Compare July 26, 2024 21:35

sanni-t changed the base branch from AUTH-527-move-rtps-to-their-own-table to edge July 29, 2024 19:21

sanni-t added 10 commits July 29, 2024 15:44

add csv table, add csv resource and its insertion, deletion methods a…

3c0cb56

…nd getters

updated AnalysisStore to handle csv params

69e955b

add new runTimeParameterFiles field to protocols and analyses endpoin…

70c0b32

…ts, wire it up to set the csv file id

added integration tests, fixed other test errors

96f4605

revert csv file type to be nested type, add file name to it

a442240

remove todo

9696617

address pr comments

b4823cd

fix rebase oopsie

72bc133

fix tavern test indentation

74bbdc7

all caps c s v

c927ebd

sanni-t force-pushed the AUTH-416-accept-csv-file-ids-and-save-to-csv-table branch from a252211 to c927ebd Compare July 29, 2024 19:45

sanni-t merged commit 800ce4d into edge Jul 29, 2024
39 checks passed

sanni-t deleted the AUTH-416-accept-csv-file-ids-and-save-to-csv-table branch August 6, 2024 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api, robot-server): allow setting CSV param files via protocols & analyses endpoints #15686

feat(api, robot-server): allow setting CSV param files via protocols & analyses endpoints #15686

sanni-t commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 18, 2024

sanni-t Jul 18, 2024

sanni-t Jul 18, 2024

github-actions bot commented Jul 18, 2024

jbleon95 Jul 22, 2024

jbleon95 Jul 22, 2024

jbleon95 left a comment

jbleon95 Jul 22, 2024

sanni-t Jul 23, 2024

jbleon95 Jul 22, 2024

jbleon95 left a comment

jbleon95 Jul 26, 2024

	csv_rtps: Dict[str, Union[str, None]] = {}
	csv_rtps: Dict[str, Optional[str]] = {}



		@dataclass
		class CsvParameterResource:

feat(api, robot-server): allow setting CSV param files via protocols & analyses endpoints #15686

feat(api, robot-server): allow setting CSV param files via protocols & analyses endpoints #15686

Conversation

sanni-t commented Jul 16, 2024 • edited Loading

Overview

Test Plan

Changelog

Review requests

Risk assessment

github-actions bot commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbleon95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbleon95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanni-t commented Jul 16, 2024 •

edited

Loading