-
Notifications
You must be signed in to change notification settings - Fork 563
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
Switch kin-openapi to libopenapi #1539
Conversation
Signed-off-by: Yorick van Pelt <yorick@yorickvanpelt.nl>
4a46a00
to
90b35aa
Compare
Sadly, libopenapi doesn't discover the invalidness of https://github.com/replicate/cog/blob/main/test-integration/test_integration/fixtures/invalid-int-project/predict.py |
17fe5d2
to
f500c0e
Compare
Signed-off-by: Yorick van Pelt <yorick@yorickvanpelt.nl>
f500c0e
to
cbdec05
Compare
I think it's okay to delete that test case. It's a pretty rare case, and wouldn't be hard to upstream a change to libopenapi to handle that case. |
@yorickvP could you describe in a bit more depth why we need to switch to libopenapi? It's not obvious to me. |
This comment contains a summary: #1186 (comment) We could try to get away with kin-openapi, that would just require fixing the test for the version in #1552 |
cc @nickstenning who's got a fair bit of experience working with kin-openapi |
We use What is actually wrong with |
No OpenAPI v3.1 support: getkin/kin-openapi#230 (comment) However, it seems we happen not to use any of the unsupported 3.1 features. Would be good to check somehow. |
Our own API's spec is 3.1, and we use |
Here's a list of changes: https://www.openapis.org/blog/2021/02/16/migrating-from-openapi-3-0-to-3-1-0 . My concern here is that these schemas are generated from user input by pedantic, so we don't necessarily control it when they break. |
Closing in favor of #1687, which rewrites the OpenAPI 3.1 spec version emitted by FastAPI to OpenAPI 3.0. |
Required for #1482, #1186, #1216