Swagger and Redoc docs does not work for Litestar 2.12.1

[sinisaos]

Swagger and Redoc docs does not work with the latest version of Litestar. For now we can pin the version to Litestar==2.11.0 (latest working version). I will open an issue in the Litestar repo.

[dantownsend]

Interesting. Is that our custom Swagger docs from Piccolo API, or just the built in Swagger docs for Litestar?

[sinisaos]

Our custom Swagger docs and Admin (FastAPI app) work as expected. The error is only in the Swagger/Redoc docs for Litestar. I've opened a ticket in the Litestar repo with some ideas on how to fix the problem (I don't know if that's good enough, I'm not a Litestar expert). If it doesn't resolve, we can always pin it to Litestar==2.11.0 version and everything will work.

[sinisaos]

This is a bug on Piccolo's side. Litestar is strict about the OpenAPI schema, which does not specify an extra property field as of version 3.1, nor does it allow arbitrary fields, but Piccolo seems to use this to include additional information.

The spec does allow for extensions of the schema, but those fields must be prefixed with an x-: https://spec.openapis.org/oas/v3.1.0.html#specification-extensions

From the answer above (from Litestar issue), this problem will not be solved in Litestar because they are strict about OpenAPI 3.1 spec. The problem is an extra property field that does not exist in OpenAPI 3.1. Reading the Openapi 3.1 specification, it is not very clear to me what the other name of the property field (as a replacement for extra) could be used besides additional_properties. If we change extra to additional_properties in pydantic.py, like this

def create_pydantic_model():
    ...
    field = pydantic.Field(
        json_schema_extra={"additional_properties": extra},
        **params,
    )
...
json_schema_extra_["additional_properties"]["help_text"] = table._meta.help_text

Litestar Swagger docs works, but I haven't tried other frameworks. Also Piccolo Admin doesn't work because we will have to change all extra keywords to additional_properties (also in Piccolo API). Do we even want to do this or do we just pin Litestar to version 2.11.0, or you have some other ideas?

[dantownsend]

Thanks for investigating.

I looked into a bit, but the docs aren't the easiest to understand. Here's one example where someone added extra properties:

https://redocly.com/docs/api-reference-docs/spec-extensions

In Piccolo v1, I namespaced all of our custom properties under extra. Can we change extra to x-piccolo? Does that work?

Like you say, we will have to update Piccolo Admin too.

[sinisaos]

Unfortunately, x-piccolo does not work in Litestar. First I tried x-extra which also doesn't work. I use additional_properties because that argument exists in the Litestar Schema (I think we should use that if we want to keep Litestar support). I tried it also in FastAPI and Blacksheep and it works. If you agree that we use additional_properties instead of extra, tomorrow I can deal with that and try the rest of the frameworks if we want to go that route. I also tried Piccolo Admin, we need to make a lot of changes, but it can be done.

[dantownsend]

I'm not sure. I don't fully understand the purpose of additionalProperties.

Even though it stops Litestar from complaining, from the description here it sounds like it's used for declaring optional return types:

https://www.apimatic.io/openapi/additionalproperties

There's also a pattern_properties option, but I'm not completely sure what that does.

I think our options are:

1. Play around a bit more, and try and get x- working.
2. Stop using OpenAPI to drive the UI in Piccolo Admin, because honestly it's kind of a pain. We could easily just have our own custom schema endpoint, which is far simpler. We would then remove all of the custom attributes which are added in create_pydantic_model.
3. Just version pin Litestar for now, because this is a lot of unintended work.

[sinisaos]

I'm not sure. I don't fully understand the purpose of additionalProperties.

Even though it stops Litestar from complaining, from the description here it sounds like it's used for declaring optional return types:

https://www.apimatic.io/openapi/additionalproperties

There's also a pattern_properties option, but I'm not completely sure what that does.

I absolutely agree with you. I also don't fully understand that OpenAPI 3.1 spec. I really don't know which property field to use as a replacement for extra.

I think our options are:

1. Play around a bit more, and try and get x- working.

We can try to use that, but from what I understand nothing will work for Litestar if it is not specified in their Schema. Any schema keys that are not in that schema will raise a ValueError and the Swagger docs will not be rendered. That's the main reason why I was trying to use additional_properties in the first place (although I'm not sure if that's the right way and if they're intended for that purpose). In the answer to the issue in Litestar, extensions of the schema prefixed with an x- were mentioned, but honestly I don't know how to use that and make work

2. Stop using OpenAPI to drive the UI in Piccolo Admin, because honestly it's kind of a pain. We could easily just have our own custom schema endpoint, which is far simpler. We would then remove all of the custom attributes which are added in create_pydantic_model.

I agree with you. I don't know how much work that change requires, but it would be much more practical and less dependent on other people's work (changes in dependencies)

3. Just version pin Litestar for now, because this is a lot of unintended work.

:thumbsup:

[dantownsend]

OK, lets do option 3 for now then. It seems like option 1 might not even be possible.

Which leaves us with option 2 as the long term fix. We could either:

1. Completely replace the current schema endpoint with our own custom one.
2. Keep the current /schema endpoint in Piccolo Admin (minus all of the custom attributes), and have a second endpoint (something like /meta) which contains all of the custom attributes. The downside is it means 2 API calls instead of 1.
3. Have our own custom endpoint, but embed the existing schema JSON within it. Something like {"schema": ..., "custom": ...}.

I'm not sure which is least effort / cleanest.

[sinisaos]

We have another option for Litestar. Since we are scaffolding a simple asgi application, we don't need to use create_pydantic_model, but we can use the Pydantic BaseModel directly in the Litestar template (only in that template). With that change we can use the latest version of Litestar (we don't need to pin Litestar to 2.11.0) without any other changes because other frameworks work with create_pydantic_model. What do you think about that?

[dantownsend]

@sinisaos Yes, that makes sense. We could add a small comment in the file for now saying that create_pydantic_model isn't compatible with the latest Litestar version.

[sinisaos]

It's not a big change and I can include it in the same PR with the fix for Esmerald. Or should I do a separate PR for it?

[dantownsend]

Do them as separate PRs if you don't mind - it makes it easier for tracking.

[sinisaos]

I'm going to do a PR for Esmerald now, and I'll do another PR tomorrow. Is that okay?

[dantownsend]

Yes, that's great - thanks.

[dantownsend]

@sinisaos Do you think we should re-open this? I know we have kind of fixed it for now, but long term it might need more work?

[dantownsend]

Or maybe we just open a new issue in Piccolo Admin linked to this one.

[sinisaos]

@dantownsend Feel free to re-open this issue, but I don't know how to solve that problem with Litestar, except to replace the create_pydantic_model extra key with some key that exists in the Litestar Schema as I wrote before.

Or maybe we just open a new issue in Piccolo Admin linked to this one.

I think that's a better idea since we talked about removing OpenAPI from admin. In Litestar Piccolo Admin works as expected, the issue was rendering of openapi docs generated from create_pydantic_model in Litestar.

[dantownsend]

OK, I'll open a related issue in Piccolo Admin.