-
Notifications
You must be signed in to change notification settings - Fork 27
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
⬆️ pydantic migration catalog #6629
Changes from all commits
f460240
8bb2c77
eb1a27f
b4626ff
5a395d5
1de0edc
726f452
7ea5c71
bc1a2a4
79fcc83
e48a25b
8680e2f
c4aa75c
43a8732
5b021ae
f935de8
21e13d6
ac49962
9640081
5efe92e
e1d9437
a237671
1e4dc4d
4d3ebfc
972d87f
4e4f94c
a6530dc
fdc5bc0
afd3bbe
be23200
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,13 +33,14 @@ | |
CPU_100_PERCENT: Final[int] = int(1 * GIGA) | ||
|
||
|
||
class ResourceValue(BaseModel): | ||
class ResourceValue(BaseModel, validate_assignment=True): | ||
limit: StrictInt | StrictFloat | str | ||
reservation: StrictInt | StrictFloat | str | ||
|
||
@model_validator(mode="before") | ||
@classmethod | ||
def _ensure_limits_are_equal_or_above_reservations(cls, values): | ||
# WARNING: this does not validate ON-ASSIGNMENT! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so how does it work now? what does validate_assignment=True do? nothing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So on assignment it does not work. What I did is recreating everytime the model. It has only two values so it is not that heavy |
||
if isinstance(values["reservation"], str): | ||
# in case of string, the limit is the same as the reservation | ||
values["limit"] = values["reservation"] | ||
|
@@ -56,10 +57,8 @@ def set_reservation_same_as_limit(self) -> None: | |
def set_value(self, value: StrictInt | StrictFloat | str) -> None: | ||
self.limit = self.reservation = value | ||
|
||
model_config = ConfigDict(validate_assignment=True) | ||
|
||
|
||
ResourcesDict = dict[ResourceName, ResourceValue] | ||
ResourcesDict: TypeAlias = dict[ResourceName, ResourceValue] | ||
|
||
|
||
class BootMode(StrAutoEnum): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import pytest | ||
from models_library.services_resources import ResourceValue | ||
|
||
|
||
@pytest.mark.xfail() | ||
def test_reservation_is_cap_by_limit_on_assigment_pydantic_2_bug(): | ||
|
||
res = ResourceValue(limit=10, reservation=30) | ||
assert res.limit == 10 | ||
assert res.reservation == 10 | ||
|
||
# https://docs.pydantic.dev/latest/api/config/#pydantic.config.ConfigDict.validate_assignment | ||
# before-validators DO NOT work on Assignment!!! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then why we don't change to an AFTER validator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should have tried that... yes. I will do that |
||
# SEE https://github.com/pydantic/pydantic/issues/7105 | ||
res.reservation = 30 | ||
assert res.reservation == 10 | ||
|
||
# update here is not validated neither | ||
# | ||
# res.model_copy(update={"reservation": 30}) | ||
# |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
0.5.0 | ||
0.6.0 |
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.
@pcrespov question, is this necessary? is this used in a json schema? well I guess it does not hurt
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.
Yes, you are right. It is only necessary for the models in the OAS. I was writing those systematically and missed this.