Skip to content
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

Cache invalid CoreSchema discovery #7535

Merged
merged 1 commit into from Sep 21, 2023
Merged

Cache invalid CoreSchema discovery #7535

merged 1 commit into from Sep 21, 2023

Conversation

adriangb
Copy link
Member

@adriangb adriangb commented Sep 21, 2023

Selected Reviewer: @davidhewitt

@cloudflare-pages
Copy link

cloudflare-pages bot commented Sep 21, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

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

View logs

@adriangb
Copy link
Member Author

please review

Comment on lines 137 to 139
def define_expected_missing_refs(
schema: core_schema.CoreSchema, allowed_missing_refs: set[str]
) -> core_schema.CoreSchema:
) -> tuple[core_schema.CoreSchema, bool]:
Copy link
Contributor

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.

Copy link
Member Author

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
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@adriangb adriangb merged commit d8c2adc into main Sep 21, 2023
55 checks passed
@adriangb adriangb deleted the cache-invalid-schemas branch September 21, 2023 13:19
@davidhewitt davidhewitt added the relnotes-performance Used for performance improvements. label Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review relnotes-performance Used for performance improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants