⬆️Pydantic V2: Diverse fixes after merges from master #6627

sanderegg · 2024-10-29T08:27:51Z

What do these changes do?

removed FieldNotRequired this is not supported anymore
NOTE that writing

class MyClass:
  my_int: int | None

in pydantic V1 would validate when my_int was "forgotten" and set it as None. This is not anymore the case in V2 which is the expected behavior.

Also in V2, FieldNotRequired, which was abusing that concept is not supported anymore.

@matusdrobuliak66 @pcrespov Please check:

services/payments/src/simcore_service_payments/models/db.py changes in example for PaymentsTransactionsDB and 'PaymentsMethodsDB' that were missing values

Related issue/s

How to test

Dev-ops checklist

No ENV changes or I properly updated ENV (read the instruction)

services/efs-guardian/src/simcore_service_efs_guardian/core/settings.py

matusdrobuliak66

thanks

codecov · 2024-10-29T11:21:02Z

Codecov Report

Attention: Patch coverage is 50.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 70.55%. Comparing base (afa0b3c) to head (367c12b).
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    #6627      +/-   ##
===============================================================================
- Coverage                                        70.69%   70.55%   -0.15%     
===============================================================================
  Files                                             1029      922     -107     
  Lines                                            46799    42955    -3844     
  Branches                                          1243      434     -809     
===============================================================================
- Hits                                             33086    30308    -2778     
+ Misses                                           13492    12555     -937     
+ Partials                                           221       92     -129

Flag	Coverage Δ		*Carryforward flag
integrationtests	`64.78% <ø> (-0.01%)`	⬇️	Carriedforward from 2e20fb8
unittests	`83.44% <50.00%> (+2.60%)`	⬆️

*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	`83.79% <ø> (ø)`
pkg_postgres_database	`∅ <ø> (∅)`
pkg_service_integration	`70.00% <ø> (ø)`
pkg_service_library	`∅ <ø> (∅)`
pkg_settings_library	`∅ <ø> (∅)`
pkg_simcore_sdk	`77.44% <ø> (ø)`
agent	`97.21% <ø> (ø)`
api_server	`∅ <ø> (∅)`
autoscaling	`95.25% <ø> (ø)`
catalog	`∅ <ø> (∅)`
clusters_keeper	`98.85% <ø> (ø)`
dask_sidecar	`90.84% <ø> (ø)`
datcore_adapter	`93.14% <ø> (ø)`
director	`58.38% <ø> (ø)`
director_v2	`76.20% <ø> (ø)`
dynamic_scheduler	`∅ <ø> (∅)`
dynamic_sidecar	`59.66% <ø> (ø)`
efs_guardian	`87.12% <43.75%> (∅)`
invitations	`93.44% <ø> (ø)`
osparc_gateway_server	`85.41% <ø> (ø)`
payments	`92.98% <100.00%> (∅)`
resource_usage_tracker	`∅ <ø> (∅)`
storage	`89.63% <ø> (ø)`
webclient	`∅ <ø> (∅)`
webserver	`59.66% <ø> (-0.02%)`	⬇️

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 afa0b3c...367c12b. Read the comment docs.

GitHK

Please double check my highlights

packages/models-library/src/models_library/api_schemas_webserver/projects.py

packages/models-library/src/models_library/utils/pydantic_tools_extension.py

sonarqubecloud · 2024-10-29T17:37:00Z

Quality Gate failed

Failed conditions
6.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

pcrespov · 2024-10-29T13:37:58Z

packages/models-library/src/models_library/api_schemas_webserver/projects.py

@@ -107,13 +110,15 @@ class ProjectReplace(InputSchema):
    last_change_date: DateTimeStr
    workbench: NodesDict
    access_rights: dict[GroupIDStr, AccessRights]
-    tags: list[int] | None = []
+    tags: list[int] | None = Field(
+        default_factory=list, json_schema_extra={"default": []}


I wonder if the rest noticed this. perhaps add it in the description of the PR ?

pcrespov · 2024-10-29T13:43:28Z

packages/models-library/src/models_library/api_schemas_webserver/projects.py

-    quality: dict[str, Any] = FieldNotRequired()
-
-    _empty_is_none = field_validator("thumbnail", allow_reuse=True, pre=True)(
+    name: ShortTruncatedStr | None = Field(default=None)


Why you add Field when we only look for the default?

THOUGHT: I wonder if we should start using Annotated instead of abusing of the "exception to the rule" of adding a default (i.e. Field) that is not an instance of the type annotated. It kind of similar to the story with FieldRequired that exploited an edge case that now was removed.

class MyModel(BaseModel): foo: Annotated[LongTruncatedStr | None , Field(description="foo is the opposite of bar")] = None

Perhaps is an unnecessary burden ... it does not really pay off!

pcrespov · 2024-10-31T09:10:29Z

services/payments/src/simcore_service_payments/models/db.py

@@ -27,6 +28,8 @@
    "invoice_pdf_url": None,
    "initiated_at": "2023-09-27T10:00:00",
    "state": PaymentTransactionState.PENDING,
+    "completed_at": None,


this is fine

sanderegg added t:maintenance Some planned maintenance work efs-guardian labels Oct 29, 2024

sanderegg self-assigned this Oct 29, 2024

sanderegg requested a review from matusdrobuliak66 as a code owner October 29, 2024 08:27

matusdrobuliak66 reviewed Oct 29, 2024

View reviewed changes

services/efs-guardian/src/simcore_service_efs_guardian/core/settings.py Show resolved Hide resolved

matusdrobuliak66 approved these changes Oct 29, 2024

View reviewed changes

sanderegg added this to the Caveman milestone Oct 29, 2024

sanderegg added 10 commits October 29, 2024 11:55

bad merge

a945c6d

removed deprecated code

8ab4371

deprecated

64cf5d0

remove AnyUrlLegacy

4408874

ruff

67254ed

back to the origin

7cbf1e6

one fix

638ded4

bad merge

b2074f7

FieldNotRequired is broken

87e7094

removed FieldNotRequired

3baa3f8

sanderegg force-pushed the migration/fix-tests branch from 304d440 to 3baa3f8 Compare October 29, 2024 10:56

sanderegg requested a review from pcrespov as a code owner October 29, 2024 10:56

sanderegg changed the title ~~⬆️Fix tests after merges~~ ⬆️Pydantic V2: Fix tests after merges Oct 29, 2024

sanderegg changed the title ~~⬆️Pydantic V2: Fix tests after merges~~ ⬆️Pydantic V2: Diverse fixes after merges from master Oct 29, 2024

removed FieldNotRequired

b74a6cf

sanderegg requested a review from GitHK as a code owner October 29, 2024 12:04

sanderegg requested review from giancarloromeo and bisgaard-itis October 29, 2024 12:05

giancarloromeo approved these changes Oct 29, 2024

View reviewed changes

GitHK approved these changes Oct 29, 2024

View reviewed changes

packages/models-library/src/models_library/api_schemas_webserver/projects.py Outdated Show resolved Hide resolved

packages/models-library/src/models_library/utils/pydantic_tools_extension.py Outdated Show resolved Hide resolved

sanderegg added 2 commits October 29, 2024 13:28

@GitHK review: removed

17cbd4f

finished migration of efs-guardian

0f5cd62

sanderegg added 12 commits October 29, 2024 13:43

replaced parse_obj_as

eca9d9a

@GitHK review: remove suspicious code

6b18099

fixed typing

cf55b7b

GenericModel moved

6855b2e

fixed model

a431816

missing examples

1fe02f3

nicer like this

e8d7414

set to 3600 seconds

670200b

ruff

c0f2d97

use ISO

c814e9b

extend the example

e3b7ccc

incorrect

2e20fb8

sanderegg requested review from mrnicegyu11 and YuryHrytsuk as code owners October 29, 2024 16:26

mypy

367c12b

sanderegg merged commit 9e76d4f into ITISFoundation:pydantic_v2_migration_do_not_squash_updates Oct 29, 2024
70 of 89 checks passed

sanderegg deleted the migration/fix-tests branch October 29, 2024 20:54

pcrespov reviewed Oct 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⬆️Pydantic V2: Diverse fixes after merges from master #6627

⬆️Pydantic V2: Diverse fixes after merges from master #6627

sanderegg commented Oct 29, 2024 •

edited

Loading

matusdrobuliak66 left a comment

codecov bot commented Oct 29, 2024 •

edited

Loading

GitHK left a comment

sonarqubecloud bot commented Oct 29, 2024

pcrespov Oct 29, 2024

pcrespov Oct 29, 2024

pcrespov Oct 31, 2024

⬆️Pydantic V2: Diverse fixes after merges from master #6627

⬆️Pydantic V2: Diverse fixes after merges from master #6627

Conversation

sanderegg commented Oct 29, 2024 • edited Loading

What do these changes do?

Related issue/s

How to test

Dev-ops checklist

matusdrobuliak66 left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 29, 2024 • edited Loading

Codecov Report

GitHK left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Oct 29, 2024

Quality Gate failed

pcrespov Oct 29, 2024

Choose a reason for hiding this comment

pcrespov Oct 29, 2024

Choose a reason for hiding this comment

pcrespov Oct 31, 2024

Choose a reason for hiding this comment

sanderegg commented Oct 29, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading