addressed review comments

Opentrons · Mar 20, 2024 · 32011f8 · 32011f8
1 parent 3bc8a61
commit 32011f8
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 19 deletions.
diff --git a/robot-server/robot_server/protocols/router.py b/robot-server/robot_server/protocols/router.py
@@ -166,9 +166,13 @@ async def create_protocol(
             " protocol resources on the robot."
         ),
     ),
-    runTimeParameterValues: Optional[str] = Form(
+    run_time_parameter_values: Optional[str] = Form(
         default=None,
-        description="Key value pairs of run-time parameters defined in a protocol.",
+        description="Key-value pairs of run-time parameters defined in a protocol."
+        " Note that this is expected to be a string holding a JSON object."
+        " Also, if this data is included in the request, the server will"
+        " always trigger an analysis (for now).",
+        alias="runTimeParameterValues",
     ),
     protocol_directory: Path = Depends(get_protocol_directory),
     protocol_store: ProtocolStore = Depends(get_protocol_store),
@@ -189,7 +193,7 @@ async def create_protocol(
     Arguments:
         files: List of uploaded files, from form-data.
         key: Optional key for client-side tracking
-        runTimeParameterValues: Key value pairs of run-time parameters defined in a protocol.
+        run_time_parameter_values: Key value pairs of run-time parameters defined in a protocol.
         protocol_directory: Location to store uploaded files.
         protocol_store: In-memory database of protocol resources.
         analysis_store: In-memory database of protocol analyses.
@@ -211,11 +215,13 @@ async def create_protocol(
         assert file.filename is not None
     buffered_files = await file_reader_writer.read(files=files)  # type: ignore[arg-type]
 
-    if isinstance(runTimeParameterValues, str):
+    if isinstance(run_time_parameter_values, str):
         # We have to do this isinstance check because if `runTimeParameterValues` is
         # not specified in the request, then it gets assigned a Form(None) value
         # instead of just a None. \(O.o)/
-        parsed_rtp = json.loads(runTimeParameterValues)
+        # TODO: check if we can make our own "RTP multipart-form field" Pydantic type
+        #  so we can validate the data contents and return a better error response.
+        parsed_rtp = json.loads(run_time_parameter_values)
     else:
         parsed_rtp = None
     content_hash = await file_hasher.hash(buffered_files)

diff --git a/robot-server/tests/integration/http_api/protocols/test_analyses.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_analyses.tavern.yaml
@@ -28,7 +28,7 @@ stages:
 
   - name: Check that the analysis summary is present in /protocols/:id; retry until it says it's completed
     max_retries: 5
-    delay_after: 5
+    delay_after: 1
     request:
       url: '{ot2_server_base_url}/protocols/{protocol_id}'
     response:
@@ -95,16 +95,15 @@ stages:
       files:
         files: 'tests/integration/protocols/basic_transfer_standalone.py'
     response:
-      save:
-        json:
-          protocol_id: data.id
-          analysis_id: data.analysisSummaries[1].id
       strict:
         - json:off
       status_code: 200
       json:
         data:
+          id: '{protocol_id}'
           analyses: []
           analysisSummaries:
+            - id: '{analysis_id}'
+              status: completed
             - id: !anystr
               status: pending
diff --git a/robot-server/tests/protocols/test_protocols_router.py b/robot-server/tests/protocols/test_protocols_router.py
@@ -380,8 +380,8 @@ async def test_create_existing_protocol(
         protocol_directory=protocol_directory,
         protocol_store=protocol_store,
         analysis_store=analysis_store,
-        protocol_reader=protocol_reader,
         file_reader_writer=file_reader_writer,
+        protocol_reader=protocol_reader,
         file_hasher=file_hasher,
         protocol_analyzer=protocol_analyzer,
         task_runner=task_runner,
@@ -482,8 +482,8 @@ async def test_create_protocol(
         protocol_directory=protocol_directory,
         protocol_store=protocol_store,
         analysis_store=analysis_store,
-        protocol_reader=protocol_reader,
         file_reader_writer=file_reader_writer,
+        protocol_reader=protocol_reader,
         file_hasher=file_hasher,
         protocol_analyzer=protocol_analyzer,
         task_runner=task_runner,
@@ -592,12 +592,12 @@ async def test_create_protocol_with_run_time_params(
     await create_protocol(
         files=[protocol_file],
         key="dummy-key-111",
-        runTimeParameterValues='{"vol": 123, "dry_run": true, "mount": "left"}',
+        run_time_parameter_values='{"vol": 123, "dry_run": true, "mount": "left"}',
         protocol_directory=protocol_directory,
         protocol_store=protocol_store,
         analysis_store=analysis_store,
-        protocol_reader=protocol_reader,
         file_reader_writer=file_reader_writer,
+        protocol_reader=protocol_reader,
         file_hasher=file_hasher,
         protocol_analyzer=protocol_analyzer,
         task_runner=task_runner,
@@ -693,12 +693,12 @@ async def test_create_existing_protocol_with_run_time_params(
     result = await create_protocol(
         files=[protocol_file],
         key="dummy-key-111",
-        runTimeParameterValues='{"vol": 123, "dry_run": true, "mount": "left"}',
+        run_time_parameter_values='{"vol": 123, "dry_run": true, "mount": "left"}',
         protocol_directory=protocol_directory,
         protocol_store=protocol_store,
         analysis_store=analysis_store,
-        protocol_reader=protocol_reader,
         file_reader_writer=file_reader_writer,
+        protocol_reader=protocol_reader,
         file_hasher=file_hasher,
         protocol_analyzer=protocol_analyzer,
         task_runner=task_runner,
@@ -759,11 +759,11 @@ async def test_create_protocol_not_readable(
         await create_protocol(
             files=[],
             protocol_directory=Path("/dev/null"),
-            protocol_reader=protocol_reader,
             protocol_store=protocol_store,
-            protocol_id="protocol-id",
             file_reader_writer=file_reader_writer,
+            protocol_reader=protocol_reader,
             file_hasher=file_hasher,
+            protocol_id="protocol-id",
         )
 
     assert exc_info.value.status_code == 422
@@ -811,9 +811,9 @@ async def test_create_protocol_different_robot_type(
         await create_protocol(
             files=[],
             protocol_directory=Path("/dev/null"),
-            protocol_reader=protocol_reader,
             protocol_store=protocol_store,
             file_reader_writer=file_reader_writer,
+            protocol_reader=protocol_reader,
             file_hasher=file_hasher,
             protocol_id="protocol-id",
         )