8000 Fix recursion error in json schema generation by adriangb · Pull Request #6720 · pydantic/pydantic · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@adriangb
Copy link
Member
@adriangb adriangb commented Jul 17, 2023

Fixes #6708

Selected Reviewer: @Kludex

@adriangb
Copy link
Member Author

please review cc @dmontagu

@adriangb
Copy link
Member 8000 Author

@commonism any concerns with this?

@cloudflare-workers-and-pages
Copy link
cloudflare-workers-and-pages bot commented Jul 17, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 07ede88
Status: ✅  Deploy successful!
Preview URL: https://adec97b4.pydantic-docs2.pages.dev
Branch Preview URL: https://deepcopy-core-schema.pydantic-docs2.pages.dev

View logs

@adriangb adriangb assigned dmontagu and unassigned Kludex Jul 17, 2023
Copy link
Contributor
@dmontagu dmontagu left a comment

Choose a reason for hiding this comment

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

I think this makes sense, though @adriangb and I both agree that there is probably a better fix out there.

Probably the best way to fix it would be to generate the JSON schema more fully-independently of the core schema, rather than storing JSON-modification functions on the core schema. Or at least rework the core schema generation so that there is less mutability. But this works now.

@MarkusSintonen
Copy link
Contributor

@adriangb amazing thank you for the quick fix! I tested this branch against some of our code which failed previously with random RecursionErrors and now it works with this branch changes

@commonism
Copy link
Contributor

@commonism any concerns with this?

No.

@adriangb adriangb merged commit c28c0c6 into main Jul 18, 2023
@adriangb adriangb deleted the deepcopy-core-schema branch July 18, 2023 09:46
Kludex pushed a commit to Kludex/pydantic that referenced this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Leak in pydantic_js_functions leading to RecursionErrors with V2 generic model json schema handling

6 participants

0