-
Notifications
You must be signed in to change notification settings - Fork 1
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
Upgrades/packages #83
Conversation
601124a
to
376d9ce
Compare
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 code looks good, however there are some things I think you should look at. Especially the validator without the pre flag as this handles data not being in the correct form
376d9ce
to
08c8500
Compare
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.
LGTM, good job 🚀
@@ -9,11 +9,16 @@ class Status(str, Enum): | |||
active = "active" | |||
inactive = "inactive" | |||
|
|||
def __str__(self): |
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.
nice 👍
# pre=true makes this validation erun before pydantic's model validator | ||
@validator('role', pre=True) | ||
# mode='before' makes this validation run before pydantic's model validator | ||
@field_validator('role', mode='before') |
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.
good 👍
⬆️ Upgrade all packages
Upgraded all packages to their newest version:
Removed one unused package:
Upgraded python version 3.9 -> 3.11
New enum syntax requires explicit value reference in format strings.
__str__
method added to enum classes to avoid this.Resolves
This PR is based on the changes from #82 and will therefore resolve it. Additionally, it will resolve both pending dependabot PRs #79 and #80.