-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Cache invalid CoreSchema discovery #7535
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
Conversation
Deploying with
|
| Latest commit: |
876a058
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bcc87d5a.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://cache-invalid-schemas.pydantic-docs2.pages.dev |
|
please review |
pydantic/_internal/_core_utils.py
Outdated
| def define_expected_missing_refs( | ||
| schema: core_schema.CoreSchema, allowed_missing_refs: set[str] | ||
| ) -> core_schema.CoreSchema: | ||
| ) -> tuple[core_schema.CoreSchema, bool]: |
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.
Potentially document or comment what the bool is here, I had to look at the callsite.
One option to avoid bool (and intermediate tuple, if you care about micro-optimizing) is to return core_schema.CoreSchema | None and return None in the case when there are no missing refs.
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.
Great call, done
| ] = self._needs_apply_discriminated_union | ||
| metadata = schema.setdefault('metadata', {}) | ||
| metadata[NEEDS_APPLY_DISCRIMINATED_UNION_METADATA_KEY] = self._needs_apply_discriminated_union | ||
| metadata['invalid'] = self._has_invalid_schema |
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.
Should 'invalid' maybe also become a constant?
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.
Done
39f1ae6 to
876a058
Compare
Selected Reviewer: @davidhewitt