8000 Cache invalid CoreSchema discovery by adriangb · Pull Request #7535 · pydantic/pydantic · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@adriangb
Copy link
Member
@adriangb adriangb commented Sep 21, 2023

Selected Reviewer: @davidhewitt

@cloudflare-workers-and-pages
Copy link
cloudflare-workers-and-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 force-pushed the cache-invalid-schemas branch from 39f1ae6 to 876a058 Compare September 21, 2023 13:12
@adriangb adriangb merged commit d8c2adc into main Sep 21, 2023
@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.

3 participants

0