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

add providers to openapi.json #303

Closed
wants to merge 3 commits into from

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Jun 10, 2020

fixes #193

The OpenAPI 3 specification allows the addition of a list of servers to
the openapi.json file:
https://swagger.io/docs/specification/api-host-and-base-path/

This is can be used to get a fully interactive swagger documentation,
which also allows users to make requests.

Try it live
(and, in particular, use it to try out queries)

The idea is to replace the "API Documentation" link on http://www.optimade.org/ by this

So far, this PR contains just the basics to make it work. Happy to improve configurability / think about default behavior etc.

The OpenAPI 3 specification allows the addition of a list of servers to
the openapi.json file:
https://swagger.io/docs/specification/api-host-and-base-path/

This is can be used to get a fully interactive swagger documentation,
which also allows users to make requests.
@ltalirz ltalirz requested review from CasperWA and ml-evs June 10, 2020 23:07
@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #303 (36a9c18) into master (3c8c47f) will decrease coverage by 0.15%.
The diff coverage is 16.66%.

❗ Current head 36a9c18 differs from pull request most recent head a231f9f. Consider uploading reports for the commit a231f9f to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #303      +/-   ##
==========================================
- Coverage   90.03%   89.88%   -0.16%     
==========================================
  Files          54       54              
  Lines        2268     2273       +5     
==========================================
+ Hits         2042     2043       +1     
- Misses        226      230       +4     
Flag Coverage Δ
unittests 89.88% <16.66%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
optimade/server/main.py 73.01% <0.00%> (-4.96%) ⬇️
optimade/server/config.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c8c47f...a231f9f. Read the comment docs.

@CasperWA
Copy link
Member

Will this fix and close #193?

@CasperWA
Copy link
Member

You should add https://optimade.odbx.science as well I think, this is @ml-evs's implementation based on this repo.
Maybe @shyamd can lead you to a CORS ready implementation as well? And maybe also @markus1978?

@ml-evs
Copy link
Member

ml-evs commented Jun 11, 2020

It sounds like "test databases" are going to be useful for the final NaCl paper query, would it make sense to link to that one here? I'm guessing I'll host it at https://optimade-test.odbx.science if you want to put that link up.

@ltalirz
Copy link
Member Author

ltalirz commented Jun 11, 2020

It sounds like "test databases" are going to be useful for the final NaCl paper query, would it make sense to link to that one here?

Definitely.
Happy to add whatever DB links you can suggest.

Please also have a look at the PR when you find time; if possible, I'd like to make all changes in one go.

Cheers!

@ml-evs
Copy link
Member

ml-evs commented Nov 24, 2020

Hi @ltalirz, this was a nice idea that we've forgotten about!

I just have two concerns now:

  1. This package now provides the "ground truth" schemas for the specification repo, which probably shouldn't include the server list, so we'll need to differentiate in the invoke tasks what the purpose of the generated specification is (e.g. invoke generate_openapi_with_servers or something)
  2. How do we decide which implementations to include in this list, now that we potentially have so many? I have some code in Moving and adding some utilities for client code #589 that grabs all sub-db's from implementations in the provider list, but it would be nice if this was more dynamic.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 24, 2020

Maybe host it somewhere else and include it via $ref ?

@ltalirz
Copy link
Member Author

ltalirz commented Jan 22, 2021

  1. This package now provides the "ground truth" schemas for the specification repo

Just to understand: these schemas are copied over / replicated to the specification repo?
Would a servers field in the openapi spec pose a problem for this procedure (would it not just be ignored)?

It's certainly possible to create another copy of the openapi spec for the swagger UI - it's just that then there are two openapi specs in the repo instead of just one.

In terms of fetching the list of servers - is there now a list hosted elsewhere that one could use to fetch those programmatically?

@ltalirz ltalirz requested a review from JPBergsma as a code owner June 1, 2021 14:41
"description": "Heroku instance tracking 'master' branch of optimade-python-tools"
},
{
"url": "https://dev-aiida-dev.materialscloud.org/optimade-sample/optimade",
Copy link
Contributor

@JPBergsma JPBergsma Jun 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this url: "https://dev-aiida-dev.materialscloud.org/optimade-sample/optimade" is correct?
I could not connect to it.
See also cors_providers.json

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I just saw this
image
I certainly cannot remember ever doing that (sorry if I did this "in my dreams" ;-) )... I still think the idea in this PR makes sense but I currently don't have time to work on it.

The updated URL would be https://aiida.materialscloud.org/optimade-sample/optimade

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for providing the correct link.
In May, I started a Postdoc at Cecam and I will be working on the Optimade project. Therefore, Matthew made me code owner for part of the code. Perhaps this triggered the review request. As I am still quite new to the project, I think it would be better to let Matthew and Casper decide about what to do with this pull request.

openapi/openapi.json Outdated Show resolved Hide resolved
@ml-evs ml-evs added the on-hold For PRs/issues that are on-hold for an unspecified time label Sep 2, 2021
@ml-evs
Copy link
Member

ml-evs commented Nov 29, 2022

I think this PR is now pretty stale, please feel free to reopen if someone still wants this in.

@ml-evs ml-evs closed this Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on-hold For PRs/issues that are on-hold for an unspecified time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add server(s) to openapi.json
4 participants