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.

Sign up for GitHub

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

Jump to bottom

Migrate to Pydantic 2.x #408

Merged
merged 72 commits into from Dec 15, 2023
Merged

Migrate to Pydantic 2.x #408

merged 72 commits into from Dec 15, 2023

Conversation

Mark90
Copy link
Contributor

@Mark90 Mark90 commented Oct 31, 2023 •
edited

Breaking changes

  • Removed deprecated build_extendend_domain_model() -> use build_extended_domain_model() instead
  • Removed SubscriptionInstanceList -> use an annotated list instead (TODO link to migration guide)
  • Removed @serializable_property -> use @computed_field instead
  • Removed DomainModel.get_properties -> use BaseModel.model_computed_fields() instead

Deprecations

  • VlanRanges which is now in nwa-stdlib

Internal changes

  • Removed dependency on pydantic's ModelMetaClass
  • Partially refactored base.py
  • Improved typing
  • Change Database.database_scope to release the DB session in case of an error

Todo

  • Replace usages of deprecated Pydantic v1 methods in untyped code
  • Replace pydantic.v1.EnumMemberError and pydantic.v1.typing.is_union
  • Test in SURF Orchestrator
  • Include migration guide with script
  • Remove any deprecated functions -> Partially done, remainder in Remove deprecated process types/fields/functions #342
  • Self-review Mark
  • Self-review Maurits

closes #327

@Mark90 Mark90 force-pushed the orchestrator-core-327-pydantic-v2-bumped branch 3 times, most recently from 14be259 to f50d82f Compare November 2, 2023 16:07
@codecov Codecov
Copy link

codecov bot commented Nov 2, 2023 •
edited

Codecov Report

Attention: 211 lines in your changes are missing coverage. Please review.

Comparison is base (d7b7ad7) 72.88% compared to head (5614326) 71.34%.

Files Patch % Lines
orchestrator/devtools/scripts/migrate_20.py 0.00% 129 Missing ⚠️
orchestrator/devtools/populator.py 0.00% 30 Missing ⚠️
orchestrator/types.py 84.15% 10 Missing and 6 partials ⚠️
orchestrator/domain/base.py 88.37% 6 Missing and 4 partials ⚠️
orchestrator/workflows/utils.py 53.33% 7 Missing ⚠️
orchestrator/graphql/schemas/subscription.py 0.00% 6 Missing ⚠️
orchestrator/forms/validators.py 79.16% 3 Missing and 2 partials ⚠️
orchestrator/api/api_v1/endpoints/settings.py 60.00% 2 Missing ⚠️
...trator/distlock/managers/redis_distlock_manager.py 0.00% 2 Missing ⚠️
orchestrator/graphql/resolvers/settings.py 0.00% 2 Missing ⚠️
... and 2 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #408      +/-   ##
==========================================
- Coverage   72.88%   71.34%   -1.54%     
==========================================
  Files         189      191       +2     
  Lines        9486     9588     +102     
  Branches     1576     1590      +14     
==========================================
- Hits         6914     6841      -73     
- Misses       2369     2541     +172     
- Partials      203      206       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Mark90 Mark90 force-pushed the orchestrator-core-327-pydantic-v2-bumped branch from f50d82f to ba17d78 Compare November 2, 2023 17:15
Mark90
Mark90 commented Nov 2, 2023
View reviewed changes
orchestrator/domain/base.py Outdated Show resolved Hide resolved
Mark90
Mark90 commented Nov 3, 2023
View reviewed changes
orchestrator/types.py Outdated Show resolved Hide resolved
Mark90
Mark90 commented Nov 3, 2023
View reviewed changes
orchestrator/domain/base.py Outdated Show resolved Hide resolved
@Mark90 Mark90 changed the title Orchestrator core 327 pydantic v2 bumped Pydantic 2.x Nov 3, 2023
@Mark90 Mark90 force-pushed the orchestrator-core-327-pydantic-v2-bumped branch from 0931a99 to a4aff80 Compare November 3, 2023 17:03
@Mark90 Mark90 changed the title Pydantic 2.x Migrate to Pydantic 2.x Nov 3, 2023
@Mark90 Mark90 force-pushed the orchestrator-core-327-pydantic-v2-bumped branch 3 times, most recently from 8b18ea9 to d5fc39c Compare November 6, 2023 14:02
tjeerddie
tjeerddie reviewed Nov 6, 2023
View reviewed changes
.pre-commit-config.yaml Outdated Show resolved Hide resolved
tjeerddie
tjeerddie reviewed Nov 7, 2023
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@Mark90 Mark90 force-pushed the orchestrator-core-327-pydantic-v2-bumped branch from a729cba to bf670a4 Compare December 13, 2023 15:31
@Mark90
#327 Breaking change: remove @serializable_property and DomainModel.g…
a31e1c4 
…et_properties

- Instead of serializable_property use pydantic.computed_fields
- Instead of DomainModel.get_properties() use pydantic's BaseModel.model_computed_fields
@Mark90
Improve logging related to subscription caching
0373e1d
@Mark90
#327 Add refactoring of serializable_property to migration script
e697194
@Mark90
#327 Format special types in domain-model response
071c512 
Needed to make pydantic 2.x serialize ipaddress types
@Mark90
Fix test_save_load, use dirty_equals.IsUUID instead of mock.ANY
78aea13
@Mark90 Mark90 mentioned this pull request Dec 14, 2023
Open
Mark90
Mark90 commented Dec 14, 2023
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
Mark90
Mark90 commented Dec 14, 2023
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@Mark90 Mark90 mentioned this pull request Dec 14, 2023
Open
@Mark90 Mark90 force-pushed the orchestrator-core-327-pydantic-v2-bumped branch from b654104 to 989271a Compare December 14, 2023 17:07
mrijk
mrijk reviewed Dec 15, 2023
View reviewed changes
orchestrator/__init__.py Outdated Show resolved Hide resolved
mrijk
mrijk reviewed Dec 15, 2023
View reviewed changes
orchestrator/domain/helpers.py
from orchestrator.types import filter_nonetype, get_origin_and_args, is_union_type


def _to_product_block_field_type_iterable(product_block_field_type: Union[type, Tuple[type]]) -> Iterable[type]:
Copy link
Contributor

Choose a reason for hiding this comment

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

why marked as private function?

mrijk
mrijk reviewed Dec 15, 2023
View reviewed changes
pyproject.toml
@@ -43,14 +43,15 @@ dependencies = [
"click==8.*",
"deprecated",
"deepmerge==0.1.0",
"fastapi~=0.99.1",
"fastapi~=0.103.2",
"fastapi-etag==0.4.0",
"more-itertools~=9.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

We can update to 10.1.0 without breaking anything

mrijk
mrijk approved these changes Dec 15, 2023
View reviewed changes
Copy link
Contributor

@mrijk mrijk left a comment

Choose a reason for hiding this comment

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

Lets go!

@Mark90 Mark90 marked this pull request as ready for review December 15, 2023 15:35
@Mark90 Mark90 merged commit 1409f3d into main Dec 15, 2023
6 of 8 checks passed
@Mark90 Mark90 deleted the orchestrator-core-327-pydantic-v2-bumped branch December 15, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pboers1988 pboers1988 pboers1988 left review comments

@mrijk mrijk mrijk approved these changes

@tjeerddie tjeerddie tjeerddie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pydantic v2
4 participants
@Mark90 @mrijk @pboers1988 @tjeerddie