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

Enhancement: Only generate example once when Parameter(examples=[...]) is specified for path arg and its schema #3057

Open
1 of 4 tasks
tuukkamustonen opened this issue Feb 1, 2024 · 4 comments
Labels
area/openapi This PR involves changes to the OpenAPI schema Enhancement This is a new feature or request Help Wanted 🆘 This is good for people to work on

Comments

@tuukkamustonen
Copy link
Contributor

tuukkamustonen commented Feb 1, 2024

Description

Explained below.

MCVE

import json

from litestar import Litestar, post
from litestar.openapi.spec import Example
from litestar.params import Parameter


@post("/{path_arg:str}")
def endpoint(
    path_arg: str = Parameter(examples=[Example(value="EXAMPLE_VALUE")]),
) -> None:
    pass

app = Litestar(route_handlers=[endpoint])

print(json.dumps(app.openapi_schema.to_schema(), indent=4))

The generated schema contains:

                    {
                        "name": "path_arg",
                        "in": "path",
                        "schema": {
                            "type": "string",
                            "examples": {  <------- HERE
                                "path_arg-example-1": {
                                    "value": "EXAMPLE_VALUE"
                                }
                            }
                        },
                        "required": true,
                        "deprecated": false,
                        "allowEmptyValue": false,
                        "allowReserved": false,
                        "examples": { <------- HERE
                            "path_arg-example-1": {
                                "value": "EXAMPLE_VALUE"
                            }
                        }
                    }

I would expect one occurrence is sufficient. The example was given for/as OpenAPI path parameter (via Parameter) so the latter is correct, and the JSON schema example (former one) can be dropped.

Steps to reproduce

1. `python app.py`
2. See the generated schema

Screenshots

No response

Logs

No response

Litestar Version

2.5.1

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@tuukkamustonen tuukkamustonen added the Bug 🐛 This is something that is not working as expected label Feb 1, 2024
@tuukkamustonen tuukkamustonen changed the title Bug: Example is generated twice for Parameter(examples=[...]), for path part arg and its schema Bug: Example is generated twice for Parameter(examples=[...]), for path arg and its schema Feb 1, 2024
@provinzkraut provinzkraut added the Enhancement This is a new feature or request label Feb 1, 2024
@provinzkraut provinzkraut changed the title Bug: Example is generated twice for Parameter(examples=[...]), for path arg and its schema Enhancement: Only generate example once when Parameter(examples=[...]) is specified for path arg and its schema Feb 1, 2024
@provinzkraut provinzkraut removed the Bug 🐛 This is something that is not working as expected label Feb 1, 2024
@provinzkraut
Copy link
Member

Not really a bug I think but rather an enhancement. We have a few instances where this kind of duplication occurs and it's reasonable to optimise some of these away. Iirc @peterschutt you were discussing a similar case recently regarding DTOs?

@tuukkamustonen
Copy link
Contributor Author

tuukkamustonen commented Feb 2, 2024

This perhaps-too-eager-example-generation also applies to the response examples created via OpenAPIConfig(..., create_examples=True) as well, e.g.:

  "components": {
    "schemas": {
      "Response": {
        "properties": {
          "text": {
            "type": "string",
            "examples": {
              "text-example-1": {  <---- example per field, unnecessary?
                "description": "Example text value",
                "value": "DwEkQQHiBrmXZcSFtoJx"
              }
            }
          },
          "num": {
            "type": "integer",
            "examples": {
              "num-example-1": {  <---- example per field, unnecessary?
                "description": "Example num value",
                "value": 4966
              }
            }
          }
        },
        "type": "object",
        "required": [
          "num",
          "text"
        ],
        "title": "Response",
        "examples": {
          "response-example-1": {  <---- example per model, this is ok
            "description": "Example  value",
            "value": {
              "text": "IgNZYFcagWptUqCwdERi",
              "num": 3674
            }
          }
        }
      }
    }
  }

I don't know if the GUIs actually need or benefit from field-specific examples, but perhaps the model level one would suffice.

@provinzkraut
Copy link
Member

I don't know if the GUIs actually need or benefit from field-specific examples, but perhaps the model level one would suffice.

IMO as long as we don't have more granular settings for turning example generation on/off, generating them everywhere it's supported is the more reasonable approach since it covers more use cases. Say we don't generate examples for the fields but a user wanted examples generated for that, they would have no way of turning this on. This way, we might make the schema a bit more verbose, but it actually doesn't do any harm to have these examples in there.

@tuukkamustonen
Copy link
Contributor Author

tuukkamustonen commented Feb 2, 2024

That's a fair statement. All off by default, all enabled with create_examples=True.

However, currently errors do get examples by default, e.g.:

                  "examples": {
                    "NotAuthorized": {
                      "value": {
                        "status_code": 401,
                        "detail": "Missing or invalid credentials",
                        "extra": {}
                      }
                    }
                  }
                }

That's an exception to the rule.

And then you allow disabling example generation with ResponseSpec. Another exception, perhaps (but no harm in that).


I guess there are roughly 4 modes here:

  1. JSON schema only
  2. OpenAPI schema only
  3. Both JSON + OpenAPI
  4. None

There could be a toggle, but I agree that global on/off should be sufficient, as the OpenAPI schema is really intended as machine-readable and not really for humans.

(The other issue #3058 confused me a bit here.)

@peterschutt peterschutt added Help Wanted 🆘 This is good for people to work on area/openapi This PR involves changes to the OpenAPI schema labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/openapi This PR involves changes to the OpenAPI schema Enhancement This is a new feature or request Help Wanted 🆘 This is good for people to work on
Projects
Status: Triage
Development

No branches or pull requests

3 participants