-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(sdk): upgrade to Pydantic v2 #27
Conversation
youtrack_sdk/entities.py
Outdated
value: Optional[DateTime | StrictStr | StrictInt | StrictFloat] | ||
value: Optional[datetime | StrictStr | StrictInt | StrictFloat] = None | ||
|
||
@field_validator("value") |
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.
Actually I don't understand why you need this in the first place:
https://docs.pydantic.dev/latest/usage/types/datetime/#validation-of-datetime-types
So my suggestion is to meet to discuss this date situation. To me this looks very fishy irrespectively of the code quality of the implementation.
Unfortunately, you didn't explain what the problem here was with the examples in the PR description, but tried to show your solutions assuming that they are correct.
I hope that we can find a better solution here. Could you please prepare for the call, by showing how it worked before on which inputs and how it works now, which makes problems.
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.
Explanation from the meeting: in the past this was in the type, here it is necessary, because the validation result depends on another field. Decision: @IamAbbey will check if it's possible to do it in V2 in the type. If not, then this way will be accepted.
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 am at the moment still unable to find possible way to do it in v2 to access other field value using Annotated Types or in type.
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.
Bottom line: maybe they will add it, but until then, we can use model-bound validator.
New fixup commits added to address open suggestions |
/rebase |
16a7d66
to
9b92d9c
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.
I've pushed a WIP, in as far as I'm concerned that's fine now.
1a4275e
to
4458b2e
Compare
/rebase |
4458b2e
to
6528cff
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.
Tested 👍
/rebase |
a64d665
to
d79066d
Compare
V2 Notes
Custom datatype introduced to for this
TimeZoneDateTime
.A Fix was made by Pydantic, so datetime now includes timezone info by default.
date validation: the input value
provided for a date field must have a nonzero time component. For a
timestamp to parse into a field of type date, the time components must all be zero. Else
date_from_datetime_inexact
error is raised
youtrack_sdk
is that somedate
type have to convert to custom date data typeUnixMidDayDate
e.g
DateIssueCustomField
parse_obj
renamed to model_validateparse_obj_as
replaced withTypeAdapter
BaseModel
config now usesConfigDict
Need to provide
None
default value for optional fieldsPydanticValueError
replaced withPydanticUserError
BaseModel.schema
replacedBaseModel.model_json_schema
BaseModel.dict
replacedBaseModel.model_dump
BaseModel.construct
replacedBaseModel.model_construct
from_unix_seconds
was removed in pydantic v2, so the implementation is nowcopied to
.helpers