DM-40303: Fix pydantic v2 warnings #874
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #874 +/- ##
==========================================
- Coverage 87.82% 87.72% -0.10%
==========================================
Files 274 274
Lines 36052 36096 +44
Branches 7543 7549 +6
==========================================
+ Hits 31662 31665 +3
- Misses 3223 3259 +36
- Partials 1167 1172 +5
☔ View full report in Codecov by Sentry. |
timj
force-pushed
the
tickets/DM-40303
branch
3 times, most recently
from
e258571
to
87a62cb
Compare
| @@ -1064,7 +1064,7 @@ def transfer_from( | |||
| # ask the receiving datastore to copy it when it doesn't exist | |||
| # so we have to filter again based on what the source datastore | |||
| # understands. | |||
| known_to_source = source_child.knows_these([ref for ref in refs]) | |||
| known_to_source = source_child.knows_these(list(refs)) | |||
There was a problem hiding this comment.
Not sure you need list here, knows_these says it takes Iterable, and refs is Iterable here too. I think this code already assumes that you can iterate many times over refs, which is probably not true for Iterable in general.
There was a problem hiding this comment.
I think I was forcing it to a list originally so that we could go through it multiple times, which probably means the annotation is wrong. Should it be Sequence?
There was a problem hiding this comment.
We can keep Iterable but then make sure that code can handle single-pass iterables. This code in chainedDatastore cannot, it calls set(refs) few lines above. fileDatastore also doing similar thing in knows_these. Or alternatively we should use Sequence everywhere.
Now use a single model validator that checks everything because we are checking multiple keys.
Only v2 models and v1 models with the compatibility class support model_dump_json().
The pydantic models that use the v1 compatibility class work fine with model_validate but there are some v1 models that do not inherit from that and also some classes like ScarletModelData that emulate the pydantic v1 methods but are not using pydantic. Therefore we need to still check parse_obj.
Checklist
doc/changes