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

Multi-value Literal isn't compliant with OpenAPI #1350

Closed
nphilou opened this issue Mar 30, 2020 · 11 comments · Fixed by #2348
Closed

Multi-value Literal isn't compliant with OpenAPI #1350

nphilou opened this issue Mar 30, 2020 · 11 comments · Fixed by #2348
Labels
Change Suggested alteration to pydantic, not a new feature nor a bug

Comments

@nphilou
Copy link

nphilou commented Mar 30, 2020

Multi-value Literal returns const in the JSON sub-schema of an anyOf, which isn't supported by OpenAPI:

anyOf – the subschemas must be OpenAPI schemas and not standard JSON Schemas.

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.4
            pydantic compiled: False
                 install path: /Users/[...]/venv/lib/python3.7/site-packages/pydantic
               python version: 3.7.4 (default, Sep  7 2019, 18:27:02)  [Clang 10.0.1 (clang-1001.0.46.4)]
                     platform: Darwin-18.7.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions']

Code to reproduce

from pydantic import BaseModel
from typing_extensions import Literal

class FooBar(BaseModel):
    values: Literal["value_1", "value_2"]

print(FooBar.schema_json(indent=2))

output:

{
  "title": "FooBar",
  "type": "object",
  "properties": {
    "values": {
      "title": "Values",
      "anyOf": [
        {
          "const": "value_1",
          "type": "string"
        },
        {
          "const": "value_2",
          "type": "string"
        }
      ]
    }
  },
  "required": [
    "values"
  ]
}

Literal with only one value is OpenAPI compliant btw

This issue is linked to: tiangolo/fastapi#562, #649 (the simple enum would work well)

@nphilou nphilou added the bug V1 Bug related to Pydantic V1.X label Mar 30, 2020
@tiangolo
Copy link
Member

This is not really a bug in Pydantic. Pydantic generates valid JSON Schema.

OpenAPI uses an old version of JSON Schema that only supports enums.

But you can change your model to use an enum instead: https://pydantic-docs.helpmanual.io/usage/types/#enums-and-choices

It achieves the same and will generate a JSON Schema with enums, that is valid with OpenAPI.

@samuelcolvin
Copy link
Member

Should we change pydantic so literal with multiple choices matches enum?

@samuelcolvin samuelcolvin added Change Suggested alteration to pydantic, not a new feature nor a bug and removed bug V1 Bug related to Pydantic V1.X labels Mar 30, 2020
@nphilou
Copy link
Author

nphilou commented Mar 30, 2020

@tiangolo I agree, my concern is more about the benefit of the Literal type as a one-liner choice on a Pydantic/FastAPI/Swagger UI context.

@samuelcolvin If it breaks too many things, I would suggest a simple note in the documentation to avoid this unexpected behaviour (Literal with one element works well in fastapi context but not with multiple elements)

@samuelcolvin
Copy link
Member

Please add a pr to improve the documentation.

We should change literals in V2.

@samuelcolvin samuelcolvin added this to the Version 2 milestone Mar 30, 2020
@tiangolo
Copy link
Member

Should we change pydantic so literal with multiple choices matches enum?

@samuelcolvin actually that makes sense, a Literal generates a JSON Schema const, and it could only have a single value. So, yes, a multi-valued Literal should create a JSON Schema equivalent to an enum.

@samuelcolvin
Copy link
Member

okay, do you think the change represents a "breaking change" that needs to wait for v2, or a bug fix than can go out in a minor version?

@tiangolo
Copy link
Member

Sorry for the delay (still figuring out how to handle GitHub email notifications), I think that looks like a "bug fix" or extra feature that could go in a minor version.

@abersheeran
Copy link

Is there any progress? I have the same problem.

@samuelcolvin
Copy link
Member

Not yet, would you like to submit a PR.

@JavierMartinezAlc
Copy link

Hi guys, thanks for the research on this topics.

For us, this is right now a blocker to go to production (not fulfilling good practices in documentation)...
So would you mind to provide me a small hint on where to start in the code, I will take a look and let see if I can do a PR 😊

Thanks again for the efforts!

@thomasleveil
Copy link

thomasleveil commented Jan 28, 2021

@JavierMartinezAlc it looks like it is implemented here and/or there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Change Suggested alteration to pydantic, not a new feature nor a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants