-
Notifications
You must be signed in to change notification settings - Fork 422
import pydantic objects from the _pydantic_compat module
#17667
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
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
| 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.
anoadragon453
left a comment
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!
| 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 BaseModeltofrom synapse._pydantic_compat import BaseModel(as well asconstr,conbytes,conint,confloat).It allows
check_pydantic_models.pyto 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.pypatches pydantic models from scim2-models./cc @DMRobertson @gotmax23
fixes #17659
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.(run the linters)