Skip to content
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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,10 @@ class ServiceGetV2(BaseModel):
quality: dict[str, Any] = {}

history: list[ServiceRelease] = Field(
default=[],
default_factory=list,
description="history of releases for this service at this point in time, starting from the newest to the oldest."
" It includes current release.",
json_schema_extra={"default": []},
)

model_config = ConfigDict(
Expand Down
7 changes: 5 additions & 2 deletions packages/models-library/src/models_library/app_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ class AppStatusCheck(BaseModel):
app_name: str = Field(..., description="Application name")
version: str = Field(..., description="Application's version")
services: dict[str, Any] = Field(
default={}, description="Other backend services connected from this service"
default_factory=dict,
description="Other backend services connected from this service",
json_schema_extra={"default": {}},
)

sessions: dict[str, Any] | None = Field(
default={},
default_factory=dict,
description="Client sessions info. If single session per app, then is denoted as main",
json_schema_extra={"default": {}},
)

url: AnyUrl | None = Field(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ class ServiceMetaDataEditable(ServiceBaseDisplay):
"If now>=deprecated, the service is retired",
)
classifiers: list[str] | None
quality: dict[str, Any] = {}
quality: dict[str, Any] = Field(
default_factory=dict, json_schema_extra={"default": {}}
Copy link
Member

@sanderegg sanderegg Nov 1, 2024

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

Copy link
Member Author

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.

)

model_config = ConfigDict(
json_schema_extra={
Expand All @@ -60,6 +62,7 @@ class ServiceMetaDataEditable(ServiceBaseDisplay):
for n in range(1, 11)
},
},
"classifiers": [],
}
}
)
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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"]
Expand All @@ -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):
Expand Down
21 changes: 21 additions & 0 deletions packages/models-library/tests/test_services_resources.py
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!!!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then why we don't change to an AFTER validator?

Copy link
Member Author

Choose a reason for hiding this comment

The 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,7 +1,7 @@
from pydantic import Field, PositiveInt

from .base import BaseCustomSettings
from .basic_types import BuildTargetEnum
from .basic_types import BootMode, BuildTargetEnum


class BaseApplicationSettings(BaseCustomSettings):
Expand All @@ -16,6 +16,7 @@ class BaseApplicationSettings(BaseCustomSettings):
SC_VCS_URL: str | None = None

# @Dockerfile
SC_BOOT_MODE: BootMode | None = None
SC_BOOT_TARGET: BuildTargetEnum | None = None
SC_HEALTHCHECK_TIMEOUT: PositiveInt | None = Field(
default=None,
Expand Down
4 changes: 2 additions & 2 deletions scripts/common-service.Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ _assert_target_defined:


# specification of the used openapi-generator-cli (see also https://github.com/ITISFoundation/openapi-generator)
OPENAPI_GENERATOR_NAME := itisfoundation/openapi-generator-cli-openapi-generator-v4.2.3
OPENAPI_GENERATOR_TAG := v0
OPENAPI_GENERATOR_NAME := openapitools/openapi-generator-cli
OPENAPI_GENERATOR_TAG := latest
OPENAPI_GENERATOR_IMAGE := $(OPENAPI_GENERATOR_NAME):$(OPENAPI_GENERATOR_TAG)

define validate_openapi_specs
Expand Down
5 changes: 0 additions & 5 deletions services/api-server/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ reqs: ## compiles pip requirements (.in -> .txt)
cp .env-devel $@


# specification of the used openapi-generator-cli (see also https://github.com/ITISFoundation/openapi-generator)
OPENAPI_GENERATOR_NAME := openapitools/openapi-generator-cli
OPENAPI_GENERATOR_TAG := latest
OPENAPI_GENERATOR_IMAGE := $(OPENAPI_GENERATOR_NAME):$(OPENAPI_GENERATOR_TAG)

define _create_and_validate_openapi
# generating openapi specs file under $< (NOTE: Skips DEV FEATURES since this OAS is the 'offically released'!)
@source .env; \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@

async def http_exception_handler(request: Request, exc: Exception) -> JSONResponse:
assert request # nosec
assert isinstance(exc, HTTPException)
assert isinstance(exc, HTTPException) # nosec

return create_error_json_response(exc.detail, status_code=exc.status_code)
2 changes: 1 addition & 1 deletion services/catalog/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.5.0
0.6.0
Loading
Loading