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

Improvements/fixes for openapi.json #332

Closed
1 of 3 tasks
giovannipizzi opened this issue Jun 12, 2020 · 6 comments · Fixed by #351
Closed
1 of 3 tasks

Improvements/fixes for openapi.json #332

giovannipizzi opened this issue Jun 12, 2020 · 6 comments · Fixed by #351
Assignees
Labels
OpenAPI OpenAPI / Swagger related issue priority/high Issue or PR with a consensus of high priority

Comments

@giovannipizzi
Copy link

giovannipizzi commented Jun 12, 2020

After the issue that @CasperWA found in #306, I gave a look to the openapi.json file.
I admit I'm not 100% sure of understanding it, so some comments here are wrong.

@CasperWA CasperWA changed the title Improvements/fixes for opeapi.json Improvements/fixes for openapi.json Jun 12, 2020
@CasperWA
Copy link
Member

CasperWA commented Jun 12, 2020

Thanks for the report and considerations @giovannipizzi.
These are all valid points that ends up (at least for point 1 and 3) pointing to a recheck of how the OpenAPI JSON files are generated in certain situations from the pydantic models.

Point 2 is remedied by pydantic validators, but we could instead move to enums, for sure. If there is a schema-generating difference, and we find it's more accurate for one or the other, we should use the better one, however, if there is no difference in the schema generated with these two approaches, I consider it a development choice, and something we don't wish to change at this stage, since it will not alter any functionality or validation otherwise.

@CasperWA CasperWA added OpenAPI OpenAPI / Swagger related issue priority/high Issue or PR with a consensus of high priority labels Jun 12, 2020
@CasperWA
Copy link
Member

Leaving this here for reference of updated behaviour of enums in OpenAPI scheme generation through FastAPI (they are treated as models).

@CasperWA
Copy link
Member

FastAPI also has extensive documentation on customizing and extending the OpenAPI schemas.

@ml-evs
Copy link
Member

ml-evs commented Jun 15, 2020

It would be good to fix some of these things in parallel to updating the models, so that we can provide our best OpenAPI schema ASAP for Materials-Consortia/OPTIMADE#286

@CasperWA
Copy link
Member

For sure - also, several of the models updates can be fixed by a single PR.

@CasperWA
Copy link
Member

#350 implements Enums where possible, hence solving the 2nd point in the OP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenAPI OpenAPI / Swagger related issue priority/high Issue or PR with a consensus of high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants