⬆️ pydantic migration catalog #6629

pcrespov · 2024-10-29T08:36:12Z

What do these changes do?

Migrates the catalog service to pydantic v2
Extends test coverage
upgrades to latest $OPENAPI_GENERATOR_NAME as in ⬆️ Upgrade openapi generator osparc-simcore-clients#202. This way we can validate the new openapi.json environs
regenerated openapi.json and increased api version 0.5.0 → 0.6.0

Related issue/s

Part of Major upgrade to pydantic 2.0 #4481

How to test

cd services/catalog
make install-dev
make test-dev-unit

Dev-ops checklist

None

codecov · 2024-10-29T09:29:54Z

Codecov Report

Attention: Patch coverage is 92.53731% with 5 lines in your changes missing coverage. Please review.

Project coverage is 71.49%. Comparing base (05cff9e) to head (be23200).
Report is 1 commits behind head on pydantic_v2_migration_do_not_squash_updates.

Additional details and impacted files

@@                               Coverage Diff                               @@
##           pydantic_v2_migration_do_not_squash_updates    #6629      +/-   ##
===============================================================================
- Coverage                                        71.53%   71.49%   -0.05%     
===============================================================================
  Files                                             1094      992     -102     
  Lines                                            48641    44968    -3673     
  Branches                                          1243      496     -747     
===============================================================================
- Hits                                             34795    32148    -2647     
+ Misses                                           13625    12730     -895     
+ Partials                                           221       90     -131

Flag	Coverage Δ		*Carryforward flag
integrationtests	`64.79% <ø> (ø)`		Carriedforward from afd3bbe
unittests	`84.52% <92.53%> (+2.49%)`	⬆️

*This pull request uses carry forward flags. Click here to find out more.

Components	Coverage Δ
api	`∅ <ø> (∅)`
pkg_aws_library	`∅ <ø> (∅)`
pkg_dask_task_models_library	`∅ <ø> (∅)`
pkg_models_library	`∅ <ø> (∅)`
pkg_notifications_library	`∅ <ø> (∅)`
pkg_postgres_database	`∅ <ø> (∅)`
pkg_service_integration	`70.00% <ø> (ø)`
pkg_service_library	`∅ <ø> (∅)`
pkg_settings_library	`90.74% <100.00%> (+0.01%)`	⬆️
pkg_simcore_sdk	`77.44% <ø> (ø)`
agent	`97.21% <ø> (ø)`
api_server	`∅ <ø> (∅)`
autoscaling	`95.25% <ø> (ø)`
catalog	`90.59% <92.30%> (∅)`
clusters_keeper	`98.85% <ø> (ø)`
dask_sidecar	`90.84% <ø> (ø)`
datcore_adapter	`93.14% <ø> (ø)`
director	`58.38% <ø> (ø)`
director_v2	`76.22% <ø> (ø)`
dynamic_scheduler	`∅ <ø> (∅)`
dynamic_sidecar	`59.66% <ø> (ø)`
efs_guardian	`87.12% <ø> (ø)`
invitations	`93.44% <ø> (ø)`
osparc_gateway_server	`85.15% <ø> (ø)`
payments	`92.98% <ø> (ø)`
resource_usage_tracker	`∅ <ø> (∅)`
storage	`89.63% <ø> (ø)`
webclient	`∅ <ø> (∅)`
webserver	`59.70% <ø> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05cff9e...be23200. Read the comment docs.

services/catalog/src/simcore_service_catalog/exceptions/handlers/__init__.py

giancarloromeo

Thanks

sonarcloud · 2024-10-31T16:20:05Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

sanderegg · 2024-11-01T07:08:20Z

packages/models-library/src/models_library/services_metadata_editable.py

@@ -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": {}}


@pcrespov question, is this necessary? is this used in a json schema? well I guess it does not hurt

Yes, you are right. It is only necessary for the models in the OAS. I was writing those systematically and missed this.

sanderegg · 2024-11-01T07:09:05Z

packages/models-library/src/models_library/services_resources.py

    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!


so how does it work now? what does validate_assignment=True do? nothing?

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

sanderegg · 2024-11-01T07:09:30Z

packages/models-library/tests/test_services_resources.py

+    assert res.reservation == 10
+
+    # https://docs.pydantic.dev/latest/api/config/#pydantic.config.ConfigDict.validate_assignment
+    # before-validators DO NOT work on Assignment!!!


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

I should have tried that... yes. I will do that

sanderegg · 2024-11-01T07:15:03Z

services/catalog/src/simcore_service_catalog/core/settings.py

-            "reservation": parse_obj_as(ByteSize, "2Gib"),
-        },
-    },
+_in_bytes = TypeAdapter(ByteSize).validate_python


not sure I get that one.
but anyway you forgot the typing

sanderegg · 2024-11-01T07:16:30Z

services/catalog/src/simcore_service_catalog/exceptions/handlers/_http_error.py

@@ -29,4 +30,4 @@ async def _http_error_handler(_: Request, exc: type[BaseException]) -> JSONRespo
            content=jsonable_encoder({"errors": [str(exc)]}), status_code=status_code
        )

-    return _http_error_handler
+    return _http_error_handler  # type: ignore[return-value]


why is this one needed? that looks weird.

sanderegg · 2024-11-01T07:17:06Z

services/catalog/src/simcore_service_catalog/models/services_db.py

-    # for a partial update all members must be Optional
-    classifiers: list[str] | None = Field(default_factory=list)
-    owner: PositiveInt | None
+    # for a partial update all Editable members must be Optional


Suggested change

# for a partial update all Editable members must be Optional

# for a partial update all editable members must be Optional

sanderegg · 2024-11-01T07:17:53Z

services/catalog/src/simcore_service_catalog/services/services_api.py

-            if service_db.thumbnail
-            else None
-        ),
+        thumbnail=HttpUrl(service_db.thumbnail) if service_db.thumbnail else None,


that does not validate does it?

sanderegg · 2024-11-01T07:21:25Z

services/catalog/src/simcore_service_catalog/utils/service_resources.py

-                    res_value * _DOCKER_TO_OSPARC_RESOURCE_CONVERTER[res_name],
-                )
+
+            scale = _DOCKER_TO_OSPARC_RESOURCE_CONVERTER[res_name]


it is not a necessaryily a scaling. it is a conversion between 2 types.

sanderegg · 2024-11-01T07:23:00Z

services/catalog/tests/unit/with_dbs/test_api_rest_services_resources.py

-    return f"{randint(0,100)}.{randint(0,100)}.{randint(0,100)}"
+def service_version(faker: Faker) -> str:
+    return (
+        f"{faker.random_int(0,100)}.{faker.random_int(0,100)}.{faker.random_int(0,100)}"


what is the difference with using faker.pyint(min_value=0, max_value=100)?

sanderegg · 2024-11-01T07:23:59Z

services/catalog/tests/unit/with_dbs/test_api_rest_services_resources.py

    mocked_director_service_labels.respond(json={"data": params.simcore_service_label})
    url = URL(f"/v0/services/{service_key}/{service_version}/resources")
    response = client.get(f"{url}")
    assert response.status_code == 200, f"{response.text}"
    data = response.json()
-    received_resources: ServiceResourcesDict = parse_obj_as(ServiceResourcesDict, data)
+    received_resources: ServiceResourcesDict = ServiceResourcesDict(**data)


does this now validate? before it would not

sanderegg · 2024-11-01T07:24:50Z

services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py

+    service_key = (
+        f"simcore/services/{faker.random_element(['comp', 'dynamic'])}/jupyter-math"
+    )
+    service_version = (
+        f"{faker.random_int(0,100)}.{faker.random_int(0,100)}.{faker.random_int(0,100)}"
+    )
    url = URL(


why not re-use the fixture in the other file?

sanderegg · 2024-11-01T07:24:59Z

services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py

+    service_key = f"simcore/services/{faker.random_element(['comp', 'dynamic'])}/{faker.pystr().lower()}"
+    service_version = (
+        f"{faker.random_int(0,100)}.{faker.random_int(0,100)}.{faker.random_int(0,100)}"
    )


sanderegg · 2024-11-01T07:25:31Z

services/catalog/tests/unit/with_dbs/test_api_rest_services_specifications.py

    url = URL(
        f"/v0/services/{service_key}/{service_version}/specifications"
    ).with_query(user_id=user_id)
    response = client.get(f"{url}")
    assert response.status_code == status.HTTP_200_OK
-    service_specs = ServiceSpecificationsGet.parse_obj(response.json())
+    service_specs = ServiceSpecifications.model_validate(response.json())


went from ServiceSpecificationsget to ServiceSpecifications. is that a typo?

sanderegg · 2024-11-01T07:26:06Z

services/catalog/tests/unit/with_dbs/test_api_rpc.py

@@ -161,7 +161,7 @@ async def test_rpc_catalog_client(
            "description": "bar",
            "version_display": "this is a nice version",
            "description_ui": True,  # owner activates wiki view
-        },
+        },  # type: ignore


pcrespov self-assigned this Oct 29, 2024

pcrespov mentioned this pull request Oct 29, 2024

Major upgrade to pydantic 2.0 #4481

Closed

pcrespov force-pushed the is4481/pydantic-migration-catalog branch from 6728e00 to b051792 Compare October 29, 2024 10:21

pcrespov added this to the Caveman milestone Oct 29, 2024

pcrespov added 21 commits October 31, 2024 10:12

reqs

f460240

settings

8bb2c77

update

eb1a27f

fixes models

b4626ff

parse_obj

5a395d5

construct

1de0edc

parse_obj_as

726f452

Config

7ea5c71

model copy

bc1a2a4

dict -> model_dump

79fcc83

minor errors

e48a25b

cleanup

8680e2f

common linbrary and some mypy

c4aa75c

pylint fixes

43a8732

fixes test

5b021ae

fixes resources test

f935de8

fixes ProjectGet

21e13d6

fixes rest tests

ac49962

fixes comparison

9640081

fixes

5efe92e

fixes fixture

e1d9437

pcrespov force-pushed the is4481/pydantic-migration-catalog branch from 1fad4bb to e1d9437 Compare October 31, 2024 11:21

pcrespov added 3 commits October 31, 2024 15:01

fixes test

a237671

fixes test

1e4dc4d

moves SC_BOOT_MODE up

4d3ebfc

mypy

fdc5bc0

pcrespov changed the title ~~⬆️ Is4481/pydantic migration catalog~~ ⬆️ pydantic migration catalog Oct 31, 2024

pcrespov marked this pull request as ready for review October 31, 2024 15:20

pcrespov requested review from sanderegg and matusdrobuliak66 as code owners October 31, 2024 15:20

pcrespov requested review from giancarloromeo and GitHK October 31, 2024 15:21

pcrespov added the t:maintenance Some planned maintenance work label Oct 31, 2024

pcrespov requested a review from bisgaard-itis October 31, 2024 15:21

services/catalog version: 0.5.0 → 0.6.0

afd3bbe

giancarloromeo reviewed Oct 31, 2024

View reviewed changes

services/catalog/src/simcore_service_catalog/exceptions/handlers/__init__.py Outdated Show resolved Hide resolved

giancarloromeo approved these changes Oct 31, 2024

View reviewed changes

@giancarloromeo review: changed signature to make mypy happy

be23200

pcrespov merged commit 34c9f3e into ITISFoundation:pydantic_v2_migration_do_not_squash_updates Oct 31, 2024
9 of 10 checks passed

pcrespov deleted the is4481/pydantic-migration-catalog branch October 31, 2024 16:19

sanderegg reviewed Nov 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⬆️ pydantic migration catalog #6629

⬆️ pydantic migration catalog #6629

pcrespov commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading

giancarloromeo left a comment

sonarcloud bot commented Oct 31, 2024

sanderegg Nov 1, 2024 •

edited

Loading

pcrespov Nov 1, 2024

sanderegg Nov 1, 2024

pcrespov Nov 1, 2024

sanderegg Nov 1, 2024

pcrespov Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

sanderegg Nov 1, 2024

	# for a partial update all Editable members must be Optional
	# for a partial update all editable members must be Optional

⬆️ pydantic migration catalog #6629

⬆️ pydantic migration catalog #6629

Conversation

pcrespov commented Oct 29, 2024 • edited Loading

What do these changes do?

Related issue/s

How to test

Dev-ops checklist

codecov bot commented Oct 29, 2024 • edited Loading

Codecov Report

giancarloromeo left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 31, 2024

Quality Gate passed

sanderegg Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcrespov commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading

sanderegg Nov 1, 2024 •

edited

Loading