-
Notifications
You must be signed in to change notification settings - Fork 203
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
import pydantic objects from the _pydantic_compat
module
#17667
import pydantic objects from the _pydantic_compat
module
#17667
Conversation
this allows `check_pydantic_models.py` to mock those pydantic objects only in the synapse module, and not interfere with pydantic objects in external dependencies
@@ -384,7 +380,7 @@ class TypingExtension(RequestBodyModel): | |||
receipts: Optional[ReceiptsExtension] = None | |||
typing: Optional[TypingExtension] = None | |||
|
|||
conn_id: Optional[str] |
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.
My modifications revealed an issue with this str
type that was rejected by check_pydantic_models
. I am not sure if this was wanted though.
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.
This change seems fine to me. conn_id
should be a string.
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 new ergonomics of the imports are great. Thanks for this!
@@ -384,7 +380,7 @@ class TypingExtension(RequestBodyModel): | |||
receipts: Optional[ReceiptsExtension] = None | |||
typing: Optional[TypingExtension] = None | |||
|
|||
conn_id: Optional[str] |
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.
This change seems fine to me. conn_id
should be a string.
This ensures the compatibility with scim2-models. The replaces HAS_PYDANTIC_V2 by PYDANTIC_VERSION, but HAS_PYDANTIC_V2 was not used anymore since element-hq#17667
This PR changes
from pydantic import BaseModel
tofrom synapse._pydantic_compat import BaseModel
(as well asconstr
,conbytes
,conint
,confloat
).It allows
check_pydantic_models.py
to mock those pydantic objects only in the synapse module, and not interfere with pydantic objects in external dependencies.This should solve the CI problems for #17144, which breaks because
check_pydantic_models.py
patches pydantic models from scim2-models./cc @DMRobertson @gotmax23
fixes #17659
Pull Request Checklist
EventStore
toEventWorkerStore
.".code blocks
.(run the linters)