-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
🐛 Fix bug preventing to use OpenAPI when using tuples #3874
Conversation
Had the issue and am waiting for a patch right now, would love to see this merged |
While this PR at least allows generation of A See this example from https://pydantic-docs.helpmanual.io/usage/schema/#unenforced-field-constraints from pydantic import BaseModel, Field, PositiveInt
try:
# this won't work since PositiveInt takes precedence over the
# constraints defined in Field meaning they're ignored
class Model(BaseModel):
foo: PositiveInt = Field(..., lt=10) I had to change my Model hints to use from pydantic import BaseModel, Field,conlist
voltage: conlist(int,min_items=4,max_items=4)
# OR
voltage: List[int] = Field(..., min_items=4,max_items=4) Only then would the models properly render in the |
Thanks for the comments @RileyMShea! Indeed, the generated It does appear that this would in addition require a fix on the That is, the following models have the same
When one would expect that the latter has a schema with |
@victorbenichoux that actually isn't correct - openapi has a special syntax for tuples as you correctly detailed in your original post |
My bad then, I had not correctly read the initial issue (which I did not write though 😄 ). Then the solution would be to implement the tuple validation for openapi with I still feel, however, that it would be best to be implemented on the |
Something worth noting, as I mention in pydantic/pydantic#3210, is that the specification for Tuples in the This PR at least reverts the behavior to what it was in |
@RileyMShea @Mause : |
Codecov Report
@@ Coverage Diff @@
## master #3874 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 410 522 +112
Lines 10281 13136 +2855
===========================================
+ Hits 10281 13136 +2855
Continue to review full report at Codecov.
|
📝 Docs preview for commit 969f7d3 at: https://61edd15b3a3b71a28401e87b--fastapi.netlify.app |
Thanks @victorbenichoux! 🍰 Good detective work! 🚀 And thanks everyone for the comments. I added a lot of tests to check that using tuples doesn't break the rest of OpenAPI. For anyone else paying attention to this, have in mind that Swagger UI doesn't support OpenAPI 3.1.0 yet. That is the first version that supports using tuples (also the most recent one), because it's based on JSON Schema draft 07. They recently changed how JSON Schema defines tuples in the latest drafts as @victorbenichoux pointed out (with So, whenever Swagger UI supports tuples at some level it will be first with the current results. But again, have in mind it's not really supported by the docs UI yet and they won't be properly rendered. That aside, because the internal models were defining the OpenAPI models so tightly, they didn't allow using tuples with OpenAPI and Swagger UI (because, technically it's not supported by the spec 😅 ). But with this more relaxed version now you can continue using tuples, even if they are not properly rendered in Swagger UI, but you can still benefit from the rest of OpenAPI and Swagger UI. This is available in FastAPI |
Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>
This PR is meant to fix issues (#3665, #3782) encountered generating
openapi.json
with nested models that have iterable-typed attributes (e.g.Tuple
orList
).Briefly, this behavior did not occur with 0.67.0, and was introduced in 0.68.0. Bisecting the issue led me to the following commit 97fa743.
As per the error raised when using nested schemas of iterables:
I realized that there must be an issue when initializing the
OpenAPI
model, and specifically theitems
field of theSchema
. This makes sense, since the commit introduced changes to the specification of theOpenAPI
model.And indeed, consider that we have the following
Model
to add to the openAPI specification:We have to instantiate a
Schema
withIn other words, the definition of the
Schema.items
attribute cannot beBut must be
Which is the change introduced in this commit.
Hopefully this is helpful, let me know if there is anything that I can add to this PR!