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

Ensure __get_pydantic_json_schema__ always gets the schema passed down from a wrapping handler #5876

Merged
merged 6 commits into from
May 26, 2023

Conversation

adriangb
Copy link
Member

@adriangb adriangb commented May 26, 2023

Selected Reviewer: @dmontagu

Comment on lines -493 to -498
with pytest.warns(
DeprecationWarning,
match=re.escape('The `schema_json` method is deprecated; use `model_json_schema` and json.dumps instead.'),
):
schema_json = Model.schema_json(indent=2)
loaded_schema_json = json.loads(schema_json)
Copy link
Member Author

Choose a reason for hiding this comment

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

This just made no sense to have here

@cloudflare-pages
Copy link

cloudflare-pages bot commented May 26, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d7a0527
Status: ✅  Deploy successful!
Preview URL: https://e378669c.pydantic-docs2.pages.dev
Branch Preview URL: https://get-json-schema-bug.pydantic-docs2.pages.dev

View logs

@adriangb
Copy link
Member Author

please review

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 the arbitrary_types_allowed=True error should be a little cleaner but overall this seems good to me.

Curious -- is this enough to get recursive type alias types JSON schema over the line, or does that stil need more?

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 the arbitrary_types_allowed=True error should be a little cleaner but overall this seems good to me.

Curious -- is this enough to get recursive type alias types JSON schema over the line, or does that stil need more?

@adriangb
Copy link
Member Author

Curious -- is this enough to get recursive type alias types JSON schema over the line, or does that stil need more?

Unfortunately, no. This still needs the changes in #5861 but since we should fix this anyway I didn't bother trying that branch on top of these changes, etc.

Comment on lines 101 to 108
schema = get_core_schema(cls, partial(gen_schema.generate_schema, from_dunder_get_core_schema=False))
schema = get_core_schema(
cls,
CallbackGetCoreSchemaHandler(
partial(gen_schema.generate_schema_for_type, from_dunder_get_core_schema=False),
gen_schema.generate_schema,
),
)
Copy link
Member Author

Choose a reason for hiding this comment

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

We fixed this bug for BaseModel recently, just neglected to do so for dataclasses

pydantic/_internal/_generate_schema.py Outdated Show resolved Hide resolved
@adriangb
Copy link
Member Author

please review

@adriangb adriangb merged commit 7320a22 into main May 26, 2023
53 checks passed
@adriangb adriangb deleted the get-json-schema-bug branch May 26, 2023 16:13
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.

None yet

2 participants