feat(shared-data): Add "$otSharedSchema" property to labware schema 3 #17421
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #17421 +/- ##
==========================================
+ Coverage 20.09% 21.66% +1.56%
==========================================
Files 3191 3151 -40
Lines 230123 226968 -3155
Branches 8204 8390 +186
==========================================
+ Hits 46246 49163 +2917
+ Misses 183877 177803 -6074
- Partials 0 2 +2
Flags with carried forward coverage won't be shown. Click here to find out more. |
SyntaxColoring
force-pushed
the
identify_labware_defs
branch
from
890e550 to
faba464
Compare
SyntaxColoring
added
the
gen-analyses-snapshot-pr
This comment was marked as resolved.
This comment was marked as resolved.
…17423) This PR was requested on the PR #17421 Co-authored-by: SyntaxColoring <[email protected]>
| @@ -1240,6 +1240,7 @@ | |||
| }, | |||
| "result": { | |||
| "definition": { | |||
| "$otSharedSchema": "#/labware/schemas/3", | |||
There was a problem hiding this comment.
It's surprising to me that any analysis snapshots need to change. These are for PAPIv2.21 protocols. PAPIv2.21
was released in robot software v8.2.0. Are schema 3 labware already being used in the wild? Because my understanding was that schema 3 was not ready for that.
It's a bummer if that's the case because it means it'll be difficult to make breaking schema changes. And if we can't make any breaking schema changes then we may as well have stayed on schema 2 in the first place.
There was a problem hiding this comment.
Interesting. It looks like the answer to this is that these labware have only a schema v3 definition:
- evotips_flex_96_tiprack_adapter
- evotips_opentrons_96_labware
- opentrons_tough_pcr_auto_sealing_lid
- protocol_engine_lid_stack_object
- opentrons_flex_tiprack_lid
and what's in these analyses is the thermocycler lid. these haven't shipped yet, so as long as we make sure there's v2 definitions in 8.3 before it ships we're good.
| "$otSharedSchema": { | ||
| "description": "The path to a valid Opentrons shared schema relative to the shared-data directory, without its extension.", | ||
| "enum": ["#/labware/schemas/3"] | ||
| }, |
There was a problem hiding this comment.
Do we like $otSharedSchema for this, or do we want to try use something like $id instead? $id is more standard but it would be new to us.
There was a problem hiding this comment.
I would say just stick with $otSharedSchema if we're not doing a full uri ref to a schema
| class LabwareDefinition(BaseModel): | ||
| model_config = { | ||
| # `"extra": "allow"` gives us lossless preservation of the $otSharedSchema field | ||
| # (which should always be omitted in schema 2 and always be present in schema 3) | ||
| # across parse/serialize round trips. Pydantic doesn't seem to have a good way | ||
| # of doing that when the $otSharedSchema field is declared explicitly on this | ||
| # model. | ||
| # | ||
| # Splitting this model into separate ones for schemas 2 and 3 would also solve this. | ||
| "extra": "allow" | ||
| } | ||
|
|
||
| # $otSharedSchema deliberately omitted for now. See `"extra": "allow"` in model_config. | ||
| schemaVersion: Literal[1, 2, 3] = Field( | ||
| ..., description="Which schema version a labware is using" | ||
| ) |
There was a problem hiding this comment.
Is this a reasonable way to avoid Pydantic hazards, at least for now?
SyntaxColoring
removed
the
gen-analyses-snapshot-pr
|
Re-drafting while #17421 (comment) is worked out. |
Background
Labware definitions currently do not have any way of self-identifying as labware definitions. They are just JSON files, with a .json file extension, and they do not contain any field that explicitly identifies them as labware definitions. So the system has to rely on heuristics, which is a little gross:
opentrons/api/src/opentrons/protocol_reader/file_identifier.py
Lines 183 to 185 in 19a94a6
This fixes that in the yet-to-be-released labware schema 3.
Closes EXEC-1156.
Test Plan and Hands on Testing
Our automated tests do not currently validate our labware definitions against schema 3. That's an accident and we're fixing it in #17425. Meanwhile, I've verified locally that the updated definitions conform to the updated schema.
Changelog
"$otSharedSchema": "#/labware/schemas/3"top-level property to labware schema 3. This follows the pattern set by our JSON protocol schema and others.Review requests
See comments.
Risk assessment
At least medium because of Pydantic round-trip lossiness—see comments.
High if I'm also right about schema 3 already being released into the wild—see comments.