-
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
Upgrade invitations service (Pydantic v2) #6513
Upgrade invitations service (Pydantic v2) #6513
Conversation
"alias": "c", | ||
}, | ||
} | ||
alias_generator=_to_initial, |
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.
Using the alias_generator
removes the need of the code above.
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.
The implementation you did of _to_initial
has the risk of producing inadvertently duplicate aliases as we extend the number of fields. An explicit mapping, as we have now, makes a mistake more obvious but I would also add an assert
in the code.
I propose to either do an explicit map as an implementation or your first-letter implementation of _to_initial
generator but in both cases make sure that there are no repetitions as
assert not [item for item, count in Counter([field.alias for field in _ContentWithShortNames.__fields__.values()]).items() if count > 1] #nosec
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.
I added the assert
statement as test to catch alias repetition early and have a clean separation between validation and model logic.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## pydantic_v2_migration #6513 +/- ##
=======================================================
Coverage ? 78.5%
=======================================================
Files ? 304
Lines ? 11834
Branches ? 880
=======================================================
Hits ? 9301
Misses ? 2410
Partials ? 123
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
🎉
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.
thx. Please check the alias generator
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.
thx
Quality Gate passedIssues Measures |
bbe92a2
into
ITISFoundation:pydantic_v2_migration
What do these changes do?
Upgrade the invitation service (Pydantic v2).
Related issue/s
How to test
Dev-ops checklist