fix(api): make frontRightNozzle and backLeftNozzle required for Quadrant config #16025

sanni-t · 2024-08-15T20:18:04Z

Overview

The QuadrantNozzleLayoutConfiguration has two optional fields- frontRightNozzle and backLeftNozzle, but they didn't have a default value assigned. So when the app tried to fetch the run's data by sending a GET /runs/{run_id}/commands/ request, if the configureNozzleLayout command was saved as the following valid command in the database-

{
  "id": "44c2ebda-4e81-42f4-bbd7-91710ca30242",
  "createdAt": "2024-08-13T20:00:19.662654+00:00",
  "commandType": "configureNozzleLayout",
  "key": "2a57aa0e5f1e286b479c11d95e2cce35",
  "status": "succeeded",
  "params": {
    "pipetteId": "984f1433-acfd-48e7-b213-3c6fdfd85321",
    "configurationParams": {
      "style": "QUADRANT",
      "primaryNozzle": "H1",
      "backLeftNozzle": "B1"
    }
  },
  "result": {},
  "startedAt": "2024-08-13T20:00:19.664360+00:00",
  "completedAt": "2024-08-13T20:00:19.676488+00:00",
  "notes": []
}

..then pydantic ran into a validation error since it didn't know how to parse configurationParams with a missing frontRightNozzle.

This was resulting in the 'Download Run Log' option to give a 500 error, which this PR fixes by making the fields no longer optional.

Changelog

We could have made the default values None but that would require a change in the command schema. So in order to not change the schema, and also make the API a bit more strict while doing it, we decided to make the fields non-optional.
This PR makes this change and updates the configure_nozzle_layout API method so that a Quadrant configuration (used by the API-side PARTIAL_COLUMN & QUADRANT configurations) will always set its front_right & back_left nozzles.
While making this change, I discovered a missing check on the end param- that it should be only allowed for PARTIAL_COLUMN configuration. While adding that I ended up refactoring the validation check since it was bloating up even further.
Similarly, adding more test cases was making the tests quite wordy and taking up a lot of space in the 1k+ lines file. So I moved all the partial tip test setup stuff to its own file and added more test cases

Test Plan and Hands on Testing

Run the below protocol on a robot
Close the run (this is important)
Download run log from app. This should succeed

Protocol for testing:

from opentrons.protocol_api import PARTIAL_COLUMN

metadata = {
    "protocolName": "8 Channel SINGLE Happy Path A1 or H1",
    "description": "Tip Rack South Clearance for the 8 Channel pipette and a SINGLE partial tip configuration.",
}

requirements = {
    "robotType": "Flex",
    "apiLevel": "2.20",
}

def run(protocol):
    trash = protocol.load_trash_bin("A3")  # must load trash bin
    thermocycler = protocol.load_module("thermocycler module gen2")
    partial_tip_rack = protocol.load_labware(
        load_name="opentrons_flex_96_tiprack_50ul",
        label="Partial Tip Rack",
        location="B3",
    )
    pipette = protocol.load_instrument(instrument_name="flex_8channel_50", mount="right")
    # mount on the right and you will get an error.
    pipette.configure_nozzle_layout(
        style=PARTIAL_COLUMN,
        start="H1",
        end="B1",  # 2 Tips
        tip_racks=[partial_tip_rack],
    )
    source_labware_B2 = protocol.load_labware(
        load_name="nest_96_wellplate_100ul_pcr_full_skirt",
        label="B2 Source Labware",
        location="B2",
    )
    destination_labware_C2 = protocol.load_labware(
        load_name="nest_96_wellplate_100ul_pcr_full_skirt",
        label="C2 Destination Labware",
        location="C2",
    )
    volume = 10  # Default volume for actions that require it
    pipette.transfer(volume, source_labware_B2["A6"], destination_labware_C2["A6"])

Review requests

Make sure I have accounted for all the existing validation errors on configure_nozzle_layout
I think this will not fix the issues with runs that were already run and saved on the DB with the optional fields. But there should not be any robots in the field that would have this problem because Quadrant configuration was not available in the public API until now. So I think this is okay but let me know if I am missing any other problem with this approach.

Risk assessment

Low. A bug fix and mostly refactoring of rest of the code.

…, added more errors to configure_nozzle_layout, updated and added tests

SyntaxColoring

Makes sense.

I think we could have gotten away with changing the command schema if you wanted, but I don't think that approach would have been any better than this one. And I'm certainly glad to make the Protocol Engine API stricter.

CaseyBatten

These fixes and slight refactors look great, and nicely cleaned up our growing nozzle layout api method. Thanks Sanniti!

sanni-t · 2024-08-16T15:27:34Z

api/src/opentrons/protocol_api/instrument_context.py

+    if any([end, front_right, back_left]):
+        raise ValueError(
+            f"Parameters 'end', 'front_right' and 'back_left' cannot be used with "
+            f"the {style.name} nozzle configuration."
+        )


New addition to this existing check- including 'end' in this validation

sanni-t · 2024-08-16T15:29:37Z

api/src/opentrons/protocol_api/instrument_context.py

+def _raise_if_has_end_nozzle_for_quadrant(end: Optional[str]) -> None:
+    if end is not None:
+        raise ValueError(
+            "Parameter 'end' is not supported for QUADRANT configuration."
+            " Use 'front_right' and 'back_left' arguments to specify the quadrant nozzle map instead."


New validation check

sanni-t · 2024-08-16T15:31:13Z

api/src/opentrons/protocol_engine/types.py

+    frontRightNozzle: str = Field(
        ...,
        regex=NOZZLE_NAME_REGEX,
        description="The front right nozzle in your configuration.",
    )
-    backLeftNozzle: Optional[str] = Field(
+    backLeftNozzle: str = Field(


Fixes RQA-2992

sanni-t · 2024-08-16T15:37:45Z

api/tests/opentrons/protocol_api/test_instrument_context.py

+    decoy.verify(
+        mock_instrument_core.configure_nozzle_layout(
+            style=nozzle_layout_args.style,
+            primary_nozzle=expected_core_args.primary_nozzle,
+            front_right_nozzle=expected_core_args.front_right_nozzle,
+            back_left_nozzle=expected_core_args.back_left_nozzle,
+        )
+    )


Test to make sure PARTIAL_COLUMN (the only publicly available configuration that uses Quadrant config) always populates the front_right_nozzle and back_left_nozzle

sanni-t · 2024-08-16T15:39:34Z

api/src/opentrons/protocol_api/instrument_context.py

+                    back_left_resolved = validated_end
+                    front_right_resolved = validated_start
                elif start == "A1" or start == "A12":
-                    if "H" in end:
-                        raise ValueError(
-                            f"A partial column configuration with 'start'={start} cannot have its 'end' parameter be in row H. Use `ALL` configuration to utilize all nozzles."
-                        )
-                    front_right_resolved = end
-
-            if style == NozzleLayout.QUADRANT:
-                if front_right is None and back_left is None:
-                    raise ValueError(
-                        "Cannot configure a QUADRANT layout without a front right or back left nozzle."
-                    )
-            elif not (front_right is None and back_left is None):
-                raise ValueError(
-                    f"Parameters 'front_right' and 'back_left' cannot be used with a {style.value} nozzle configuration."
+                    front_right_resolved = validated_end
+                    back_left_resolved = validated_start


Both front_right and back_left will be resolved now

sanni-t · 2024-08-16T15:39:50Z

api/src/opentrons/protocol_api/instrument_context.py

+                if front_right is None:
+                    front_right_resolved = validated_start
+                elif back_left is None:
+                    back_left_resolved = validated_start


Same- Both front_right and back_left will be resolved now

…elds_3

made frontRightNozzle and backLeftNozzle required for Quadrant config…

1c9e41f

…, added more errors to configure_nozzle_layout, updated and added tests

sanni-t requested a review from a team as a code owner August 15, 2024 20:18

sanni-t requested review from CaseyBatten and SyntaxColoring and removed request for a team August 15, 2024 20:18

SyntaxColoring approved these changes Aug 16, 2024

View reviewed changes

CaseyBatten approved these changes Aug 16, 2024

View reviewed changes

sanni-t commented Aug 16, 2024

View reviewed changes

sanni-t added 2 commits August 16, 2024 12:09

added a test for Quadrant

57919b9

Merge branch 'chore_release-8.0.0' into fix-nozzle_config_optional_fi…

0ef2fd6

…elds_3

sanni-t merged commit 99a17ae into chore_release-8.0.0 Aug 16, 2024
45 checks passed

sanni-t deleted the fix-nozzle_config_optional_fields_3 branch August 19, 2024 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(api): make frontRightNozzle and backLeftNozzle required for Quadrant config #16025

fix(api): make frontRightNozzle and backLeftNozzle required for Quadrant config #16025

sanni-t commented Aug 15, 2024 •

edited

Loading

SyntaxColoring left a comment

CaseyBatten left a comment

sanni-t Aug 16, 2024

sanni-t Aug 16, 2024

sanni-t Aug 16, 2024

sanni-t Aug 16, 2024

sanni-t Aug 16, 2024

sanni-t Aug 16, 2024

fix(api): make frontRightNozzle and backLeftNozzle required for Quadrant config #16025

fix(api): make frontRightNozzle and backLeftNozzle required for Quadrant config #16025

Conversation

sanni-t commented Aug 15, 2024 • edited Loading

Overview

Changelog

Test Plan and Hands on Testing

Review requests

Risk assessment

SyntaxColoring left a comment

Choose a reason for hiding this comment

CaseyBatten left a comment

Choose a reason for hiding this comment

sanni-t Aug 16, 2024

Choose a reason for hiding this comment

sanni-t Aug 16, 2024

Choose a reason for hiding this comment

sanni-t Aug 16, 2024

Choose a reason for hiding this comment

sanni-t Aug 16, 2024

Choose a reason for hiding this comment

sanni-t Aug 16, 2024

Choose a reason for hiding this comment

sanni-t Aug 16, 2024

Choose a reason for hiding this comment

sanni-t commented Aug 15, 2024 •

edited

Loading