review small fixes and TODOs for follow up

api/src/opentrons/protocol_api/_parameter_context.py

@@ -240,15 +240,22 @@ def initialize_csv_files(
                     f"File Id was provided for the parameter '{variable_name}',"
                     f" but '{variable_name}' is not a CSV parameter."
                 )
+            # TODO(jbl 2024-08-02) This file opening should be moved elsewhere to provide more flexibility with files
+            #    that may be opened as non-text or non-UTF-8
+            # The parent folder in the path will be the file ID, so we can use that to resolve that here
             file_id = file_path.parent.name
             file_name = file_path.name
+
+            # Read the contents of the actual file
             with file_path.open() as csv_file:
                 contents = csv_file.read()
 
+            # Open a temporary file with write permissions and write contents to that
             temporary_file = tempfile.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
             parameter_file = open(temporary_file.name, "r")
             temporary_file.close()
 

api/src/opentrons/protocols/execution/execute.py

@@ -48,6 +48,7 @@ def run_protocol(
         except Exception:
             raise
         finally:
+            # TODO(jbl 2024-08-02) this should be more tightly bound to the opening of the csv files
             if parameter_context is not None:
                 parameter_context.close_csv_files()
     else:

robot-server/robot_server/protocols/router.py

@@ -280,7 +280,7 @@ async def create_protocol(  # noqa: C901
         quick_transfer_protocol_auto_deleter: An interface to delete old quick
             transfer resources to make room for the new protocol.
         data_files_directory: Persistence directory for data files.
-        data_files_store: In-memory database of data file resources.
+        data_files_store: Database of data file resources.
         robot_type: The type of this robot. Protocols meant for other robot types
             are rejected.
         protocol_id: Unique identifier to attach to the protocol resource.

robot-server/robot_server/runs/router/base_router.py

@@ -176,7 +176,7 @@ async def create_run(
             the new run.
         quick_transfer_run_auto_deleter: An interface to delete old quick-transfer
         data_files_directory: Persistence directory for data files.
-        data_files_store: In-memory database of data file resources.
+        data_files_store: Database of data file resources.
         resources to make room for the new run.
         check_estop: Dependency to verify the estop is in a valid state.
         deck_configuration_store: Dependency to fetch the deck configuration.
@@ -191,6 +191,7 @@ async def create_run(
         request_body.data.runTimeParameterFiles if request_body is not None else None
     )
     rtp_paths: Optional[CSVRuntimeParamPaths] = None
+    # TODO(jbl 2024-08-02) raise the proper error if file ids don't exist
     if rtp_files:
         rtp_paths = {
             name: data_files_directory / file_id / data_files_store.get(file_id).name

robot-server/robot_server/runs/run_models.py

@@ -255,7 +255,7 @@ class RunCreate(BaseModel):
     )
     runTimeParameterFiles: Optional[CSVRunTimeParamFilesType] = Field(
         default={},
-        description="Key-fileId pairs of CSV run-time parameters defined in a protocol.",
+        description="Key-fileId pairs of CSV run-time parameters defined in a run.",
     )
 
 

robot-server/robot_server/runs/run_orchestrator_store.py

@@ -192,6 +192,7 @@ async def create(
         notify_publishers: Callable[[], None],
         protocol: Optional[ProtocolResource],
         run_time_param_values: Optional[PrimitiveRunTimeParamValuesType] = None,
+        # TODO(jbl 2024-08-02) combine this with run_time_param_values now that theres no ambiguity with Paths
         run_time_param_paths: Optional[CSVRuntimeParamPaths] = None,
     ) -> StateSummary:
         """Create and store a ProtocolRunner and ProtocolEngine for a given Run.