-
Notifications
You must be signed in to change notification settings - Fork 180
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
feat: Upgrade pydantic to v2 #13983
Comments
Yeah, we definitely want to upgrade Pydantic. (Internal reference: RSS-227) Unfortunately we haven't started that work yet, so there's no branch you could use. We don't have a timeline for it, since we're focusing on the new Opentrons Flex.
This is helpful. Thank you! |
Hello @SyntaxColoring is there any proposed plan for this? |
Sorry for the lack of updates here. Yes, we're working on this in PR #14871. It's not our first priority, though, so I can't promise it will be completed soon. |
Hello @SyntaxColoring, any updates on this? |
Latest update: #14871 (comment) |
please please please prioritize this. Pydantic should have fixed many import perf issues. It's a major pain that we can't easily interoperate between core models in parts of our codebase. |
# Overview This updates our robot-stack Python projects from Pydantic v1 to v2. Closes PLAT-326 and GitHub issue #13983. Affected projects that run on the robot: - api - robot-server - shared-data - hardware - server-utils - system-server - performance-metrics (re-lock only, depends on shared-data) Affected projects that only run in CI and on our laptops: - g-code-testing - hardware-testing - abr-testing # Test Plan When the https://github.com/Opentrons/buildroot and https://github.com/Opentrons/oe-core changes are ready: * [x] OT-2 * [x] Make sure all servers still boot without errors (`systemctl status`) * [ ] No unexpected mismatches reported by `pip check` * I get "robot-server 8.2.0 has requirement pydantic==2.9.0, but you have pydantic 2.9.2." I think we can fix this in a follow-up. * [x] Try running a protocol or something and make sure the server doesn't return an error or take an obscenely long time to respond. * [x] Flex * [x] Make sure all servers still boot on a Flex without errors (`systemctl status`) * [x] No unexpected mismatches reported by `pip check` * [x] Try running a protocol or something and make sure the server doesn't return an error or take an obscenely long time to respond. Beyond that, this PR touches a million little things in a million little ways, so it's difficult to test. We should try to merge it early in the release cycle to give us time to shake things out. # Changelog See https://docs.pydantic.dev/latest/migration/#migration-guide for everything that has changed from Pydantic v1 to v2. The basic methodology of this PR is: * Update `setup.py`, `Pipfile`, and `Pipfile.lock` files to a new Pydantic version, trying to follow Unfortunately, FastAPI is tightly coupled to Pydantic, so we need to update it too. The FastAPI bump is kept minimal. * Run [bump-pydantic](https://github.com/pydantic/bump-pydantic) on all projects. This automatically does a lot of the grunt work, but it does need manual follow-up. * Manually fix up lots of little things * Do global find+replaces for some [trivial renames](https://docs.pydantic.dev/latest/migration/#changes-to-pydanticbasemodel). I moved some of this to [a separate PR, #17123](#17123), because the GitHub web UI was struggling with the big diff. # Review requests - Do the setup.py, Pipfile, and Pipfile.lock files look good? We're trying to align on a definition of "good" [here](https://opentrons.atlassian.net/wiki/spaces/RPDO/pages/4671602797/Python+dependency+management). - Do all Pydantic migrations look correct? Reference: https://docs.pydantic.dev/latest/migration/#migration-guide - Do the rewritten validators look correct? Some of these needed manual intervention. - Do the `= None` additions look correct? `bump-pydantic` added these automatically to match prior parse behavior—see https://docs.pydantic.dev/latest/migration/#required-optional-and-nullable-fields. As far as I can tell, these additions are always safe. But defaulting to `None` may not be what we actually want, e.g. it may not match the underlying JSON schema. - This was a long-lived PR that changed hands several times, so there are definitely vestigial things left over from earlier attempts. If something looks unexplained or out of place to you, please speak up. # Risk assessment ## Performance This *will,* at least in the short term, make robot-server take much longer to start up, and make the tests much slower. This is a known Pydantic v2 problem (pydantic/pydantic#6768 etc.). Earlier testing on a Flex found it slowed from 46s to 1m54s (2.5x). I don't think we'll be able to get it back down to Pydantic v1 times, but some proofs of concept suggest that we can mitigate it to only ~1.6x slower. There are some ideas in EXEC-1060. ## Correctness High-risk due to the breadth of changes: storage reads and writes, HTTP requests and responses, communication between the `opentrons.protocol_api` and `opentrons.protocol_engine`, ... * [Pydantic's type coercion behavior has changed](https://docs.pydantic.dev/latest/migration/#validator-behavior-changes), trending in the direction of doing less type coercion. This is a good direction, but it is basically impossible to audit affected one Python protocol in the snapshot tests—see the inline review comments below. --------- Co-authored-by: Max Marrone <[email protected]> Co-authored-by: Seth Foster <[email protected]>
Done in PR #14871. |
Overview
Hi team,
Do you have any plans to upgrade pydantic to v2 from v1.8.2? Is there maybe any branch that I could use if such transition is happening? It becomes a problem when you want to use opentrons library in a project where packages dependencies requires pydantic v2. I couldn't find any issue that address that.
Thanks!
Implementation details
I haven't tested that, but this could be a simple fix by using v1 apis from within v2 version, see below:
from pydantic.v1 import BaseModel
There is also a helper package
bump-pydantic
which may help with that. Here is also a migration guide.Design
No response
Acceptance criteria
Lift the dependency from pydantic 1.8.2 to pydantic >=2.
The text was updated successfully, but these errors were encountered: