-
Notifications
You must be signed in to change notification settings - Fork 183
Merge back 'chore_release-8.5.0' into 'chore_release-pd-8.5.0' #18648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## chore_release-pd-8.5.0 #18648 +/- ##
==========================================================
- Coverage 25.89% 23.96% -1.94%
==========================================================
Files 3256 3256
Lines 281309 281367 +58
Branches 28543 28508 -35
==========================================================
- Hits 72842 67419 -5423
- Misses 208440 213923 +5483
+ Partials 27 25 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Labware labels are passed through from the protocol api all the way to the commands and eventually the client with little actual _functionality_ dependent on their types or their values. That means that the only thing that checks them is pydantic model validation, and if you avoid that - as the legacy command mapper does by using model_construct() - then other downstream things won't be happy. In this case, OT-2 LPC starts its session by echoing commands it reads from the robot analysis back out to a maintenance run. When a client uses HTTP to create commands, pydantic model validation _does_ run, and it would fail, and this wasn't an anticipated error scenario because, well, it's echoed from the protocol! But the incorrect data made it through. Another way to handle this would be to do pydantic model validation on the protocol inputs, but that would break any old protocols that implicitly depended on this working. Compatibility! Also IMO it's silly that we enforce this if the entire system works just fine if you give it an integer. Closes RESC-423
cd341b6
to
6d9761b
Compare
fedb3b1
to
b23b9a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! @smb2268 is out today so if you want to merge it today you'll have to go without their input.
Thanks Seth. Lol, I wanted to get this in before someone changed |
b23b9a1
to
5dc6e02
Compare
This pulls in the new labware images.