DM-42302: drop support for Pydantic v1 #929
Conversation
TallJimbo
force-pushed
the
tickets/DM-42302
branch
from
0642994 to
f2714e9
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #929 +/- ##
==========================================
+ Coverage 88.08% 88.31% +0.23%
==========================================
Files 307 306 -1
Lines 39365 39226 -139
Branches 8295 8256 -39
==========================================
- Hits 34673 34641 -32
+ Misses 3484 3393 -91
+ Partials 1208 1192 -16 ☔ View full report in Codecov by Sentry. |
TallJimbo
force-pushed
the
tickets/DM-42302
branch
from
20cd088 to
3a15d40
Compare
TallJimbo
force-pushed
the
tickets/DM-42302
branch
from
3ef1624 to
c9b3513
Compare
| them or want to support them without the deprecation message. | ||
| """ | ||
|
|
||
| def json( |
There was a problem hiding this comment.
I wasn't really sure if we were going to break people by dropping the json method here. I think that there might be people who use SerializedDatasetRef outside of our middleware but maybe only to_simple and from_simple rather than calling .json and .parse_obj. Maybe @mfisherlevine ?
There was a problem hiding this comment.
I've been thinking of all the Serialized* as middleware-internal, but I guess that's not quite true since those *_simple methods are public.
I'd really rather not go through a deprecation period for an interface we just inherit from a third-party package, but I'm happy to stall merging this for a few days to see if Merlin chimes in.
There was a problem hiding this comment.
The compromise is to merge this tomorrow after the next weekly and then you have a week to find out from people if we need to put a shim back in specifically for SerializedDatasetRef.
| @@ -46,16 +46,10 @@ | |||
| if TYPE_CHECKING: | |||
| from ..registry import Registry | |||
|
|
|||
| # Pydantic 2 requires we be explicit about the types that are used in | |||
There was a problem hiding this comment.
Would this comment still be helpful?
There was a problem hiding this comment.
Maybe; the interesting bit is why UUID doesn't work with Any, and I'm guessing it's because there's nothing to distinguish it from str, and it doesn't see if a str is UUID-like unless you explicitly tell it that UUID is a possibility. But that's pure speculation.
There was a problem hiding this comment.
I think this was something that became necessary after some pydantic 2 releases had already been made.
There was a problem hiding this comment.
I'll restore the comment, or something like it.
TallJimbo
force-pushed
the
tickets/DM-42302
branch
2 times, most recently
from
e99c0fd to
c8a2993
Compare
This was by no means a comprehensive attempt to find cases like this, just a few I happened to spot.
That's what's in rubin-env 8.0.0 (for me), so if it required pydantic v1 that'd be strange.
This should have happened on DM-41113, but it's only checked when running GitHub Actions on downstream middleware packages, and there weren't any branches downstream on that ticket.
TallJimbo
force-pushed
the
tickets/DM-42302
branch
from
c8a2993 to
44d9491
Compare
Checklist
doc/changes