refactor(robot-server): implement the load labware function to accept tipracks from the client #7131
Conversation
Laura-Danielle
added
ready for review
robot server
Codecov Report
@@ Coverage Diff @@
## edge #7131 +/- ##
==========================================
- Coverage 87.21% 87.16% -0.06%
==========================================
Files 99 99
Lines 4489 4588 +99
==========================================
+ Hits 3915 3999 +84
- Misses 574 589 +15
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
We cannot require all loadLabware commands to contain a labware. The app in 4.0 sends this command for all the user flows, and this would therefore be a breaking API change.
We have to accept loadLabware commands without a definition element in all cases. In cases where it previously did nothing, it should continue to do nothing; in the cases we're adding, not sending a labware def should make it continue to do nothing, and stick with the labware def sent at session establish. I'm not sure what the best way to do that is - @amitlissack any ideas about the particulars of how to get this to work with pydantic?
| @@ -81,6 +81,12 @@ class SetHasCalibrationBlockRequest(BaseModel): | |||
| description="whether or not there is a calibration block present") | |||
|
|
|||
|
|
|||
| class UpdateTiprackRequest(BaseModel): | |||
| tiprackDefinition: dict = Field( | |||
There was a problem hiding this comment.
This may want a richer type like at least Dict[str, Any]
| pass | ||
| async def load_labware(self, tiprackDefinition: dict): | ||
| existing_offset_calibration = self._get_stored_pipette_offset_cal() | ||
| self._load_tip_rack( |
There was a problem hiding this comment.
let's validate this def against the schema and make sure it's a tiprack before passing it to _load_tiprack. _load_tiprack taking a LabwareDefinition means that the def should be checked for validity.
There was a problem hiding this comment.
We don't validate that anywhere else. Should I add that in to the create params as well for all of the flows?
There was a problem hiding this comment.
Yeah probably should! Should return a 4xx if it fails
| def _determine_state_machine( | ||
| self, perform_tip_length: bool | ||
| ) -> Tuple[PipetteOffsetStateMachine, GenericState]: | ||
| state_machine: PipetteOffsetStateMachine =\ |
There was a problem hiding this comment.
Any way we can get rid of these type ignores?
Laura-Danielle
force-pushed
the
allow_tiprack_selection_robot_server
branch
from
8c5e640 to
4d06203
Compare
|
Just a heads up: The |
… tipracks from the client closes #7091
… very not specific.
Laura-Danielle
force-pushed
the
allow_tiprack_selection_robot_server
branch
from
cff6aa3 to
67fabb5
Compare
Overview
This PR supports #7901 on the backend.
Changelog
Review requests
Due to pydantic union issues (again) I had to make the tiprack definition required in load labware. Should we try to do a refactor of request models to avoid using unions again here, or is this fine for now?
Risk assessment
Medium. We are changing how load labware works on the backend, and forcing the front end to pass in a tiprack definition for all of the user flows to this command.
We should test all of the user flows once #7901 is up.