Skip to content

🐛 Fix openapi generation with responses kwarg#10895

Merged
tiangolo merged 10 commits intofastapi:masterfrom
flxdot:bugfix/responses-model-openapi
Oct 12, 2024
Merged

🐛 Fix openapi generation with responses kwarg#10895
tiangolo merged 10 commits intofastapi:masterfrom
flxdot:bugfix/responses-model-openapi

Conversation

@flxdot
Copy link
Copy Markdown
Contributor

@flxdot flxdot commented Jan 6, 2024

This PR fixes a bug where the schema of response models defined via the responses keyword where generated as validation schema instead of serialization schema.

Additional discussions for that topic:

@flxdot flxdot changed the title Fix a bug where the openapi generation was wrong when responses kwarg is used 🐛 Fix openapi generation with responses kwarg Jan 6, 2024
@flxdot flxdot force-pushed the bugfix/responses-model-openapi branch from 73235ab to c166a96 Compare January 6, 2024 00:48
@flxdot flxdot force-pushed the bugfix/responses-model-openapi branch from 684ae92 to b5d733e Compare January 6, 2024 00:50
@tiangolo tiangolo added bug Something isn't working investigate p2 and removed investigate labels Jan 13, 2024
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

I've just checked that the problem is still relevant in FastAPI v. 0.109.2 (Pydantic 2.6.0).

This PR fixes this problem.
Code is nice

@simonwallace
Copy link
Copy Markdown

I am also affected by this issue. Is this PR ready to be merged?

@flxdot
Copy link
Copy Markdown
Contributor Author

flxdot commented Jun 3, 2024

I am also affected by this issue. Is this PR ready to be merged?

From my point of view, it is.

@tiangolo is there anything else you need me to do here?

@jw-00000
Copy link
Copy Markdown

Seems like this is ready to be merged. Is there any update on when this will happen? Thanks in advance.

@alejsdev
Copy link
Copy Markdown
Member

alejsdev commented Sep 6, 2024

Hi @flxdot, thanks for your interest in contributing to FastAPI. We're aware of your PR and we'll come back to review it in detail soon. Thanks for your patience 🙏

@svlandeg svlandeg self-assigned this Sep 18, 2024
Copy link
Copy Markdown
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Hi @flxdot,

Thanks for the contribution, and apologies for the delay in reviewing this!

Thanks all for the review and comments as well!

I could reproduce the bug on master and confirm that the test fails on master and succeeds with this PR. I've also pushed a related test from #11517 to this PR, as both PRs include the same fix but tested slightly differently.

This PR was submitted earlier, so I suggest we merge this one and close the other. Either way, I'll leave the final decision with Tiangolo.

Thanks again!

@svlandeg svlandeg removed their assignment Sep 18, 2024
@tiangolo
Copy link
Copy Markdown
Member

Great, thank you @flxdot! 🚀 🍰

And thanks for the help and reviews @YuriiMotov, @svlandeg, @alejsdev 🙇

This will be available in FastAPI 0.115.1 in the next hours. 🎉

@tiangolo tiangolo merged commit e049fc4 into fastapi:master Oct 12, 2024
s-rigaud pushed a commit to s-rigaud/fastapi that referenced this pull request Jan 23, 2025
Co-authored-by: flxdot <felix.fanghaenel@nitrex.com>
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
Co-authored-by: Sławek Ehlert <slawomir.ehlert@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants