Update models, endpoints and responses to 1.0.0 #380

ml-evs · 2020-06-30T14:14:54Z

Closes #379. This PR brings our models, responses and endpoints up to date with newly released 1.0.0 of the specification.

I've split this up into 4 commits, which should each individually pass the tests:

Enabled serving the API at the unversioned base URL and moved the vMajor router addition to a separate function. This means the generated schemas no longer contain version numbers, as desired.
Added the /versions endpoint from the unversioned base URL for both index and regular db's. This should now have the correct schema response (matching the released version).
Removed the index_base_url field, as it has been from the schema. I've moved the original config option we had for this to the top-level, and had to adjust our tests accordingly.
Updated all the models according to the final changes since 1.0.0-rc.2, e.g.
- meta now being mandatory, but several sub-fields becoming optional,
- consistency in how we represent links (i.e. Union[jsonapi.Links, AnyUrl]).
- descriptions for version/api_version numbers
- fixed some other descriptions, e.g. typos, broken internal links and missing newlines.

codecov · 2020-06-30T14:19:43Z

Codecov Report

Merging #380 into master will decrease coverage by 0.05%.
The diff coverage is 82.22%.

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
- Coverage   90.27%   90.22%   -0.06%     
==========================================
  Files          54       55       +1     
  Lines        2386     2404      +18     
==========================================
+ Hits         2154     2169      +15     
- Misses        232      235       +3

Flag	Coverage Δ
#unittests	`90.22% <82.22%> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
optimade/models/baseinfo.py	`94.87% <ø> (ø)`
optimade/models/entries.py	`100.00% <ø> (ø)`
optimade/server/main.py	`84.78% <44.44%> (-0.94%)`	⬇️
optimade/server/main_index.py	`84.78% <57.14%> (-5.70%)`	⬇️
optimade/models/optimade_json.py	`93.97% <100.00%> (+0.07%)`	⬆️
optimade/models/responses.py	`97.50% <100.00%> (ø)`
optimade/server/config.py	`100.00% <100.00%> (ø)`
optimade/server/routers/index_info.py	`100.00% <100.00%> (ø)`
optimade/server/routers/landing.py	`68.75% <100.00%> (+2.08%)`	⬆️
optimade/server/routers/versions.py	`100.00% <100.00%> (ø)`
... and 1 more

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 cdf3483...65e5136. Read the comment docs.

CasperWA

Should versions also be included in the index meta-database?

optimade/server/main.py

optimade/server/routers/versions.py

tests/server/routers/test_versions.py

CasperWA · 2020-07-01T16:01:04Z

Why isn't /versions added to the OpenAPI schema? Can it not be added? Or it shouldn't?

Edit: I found your "thought" on this in the OP :)

optimade/server/main.py

optimade/models/optimade_json.py

optimade/server/routers/versions.py

ml-evs · 2020-07-01T17:51:34Z

I think we've now ironed out all differences between schemas and the specification, except the way we specify the content type of the versions endpoint, which I'll have a go at later.

ml-evs · 2020-07-01T17:52:28Z

We also need to change a lot of our test data so that the tests pass, e.g. index_base_url no longer existing.

ml-evs · 2020-07-01T22:52:36Z

Pending these test failures, I think that's everything done, except changing the content of /versions to text/csv rather than application/json.

- This URL is now used when generating the OpenAPI schemas

- This commit removes the `index_base_url` from the models and updates all tests, routers and config files accordingly. - `index_base_url` is now a top-level config option.

ml-evs · 2020-07-02T11:20:51Z

I've just spent a while meticulously thematically splitting this PR into just a few commits, each of which should pass tests individually. If you're reviewing, it might make sense to break it down per commit.

CasperWA

We should add homepage to the implementation configurations as well, I think?

All in all, pretty great work @ml-evs !

openapi/index_openapi.json

openapi/openapi.json

optimade/models/optimade_json.py

CasperWA · 2020-07-02T16:24:54Z

optimade/server/config.py

@@ -73,13 +73,16 @@ class ServerConfig(BaseSettings):
        ),
        description="Introspective information about this OPTIMADE implementation",
    )
+    index_base_url: Optional[AnyHttpUrl] = Field(


Why do we have this now?

We can remove it, sure, it's only (optionally) used in the landing page now

Right. Is there a way for us to automagically retrieve it from the links endpoint data JSON file? Then there is only one source of truth and we're not mixing concepts. Especially, when it's using a recently outdated valid field name.

optimade/server/main.py

optimade/server/routers/landing.py

optimade/server/routers/versions.py

tests/server/utils.py

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>

optimade/server/routers/versions.py

ml-evs force-pushed the ml-evs/add_versions_endpoint branch from a59292a to 00da5df Compare June 30, 2020 14:15

ml-evs force-pushed the ml-evs/add_versions_endpoint branch 2 times, most recently from 0dd0557 to b872fe4 Compare June 30, 2020 14:41

ml-evs requested review from CasperWA and shyamd June 30, 2020 14:44

ml-evs force-pushed the ml-evs/add_versions_endpoint branch 2 times, most recently from 0d5af29 to 9991937 Compare June 30, 2020 14:45

ml-evs mentioned this pull request Jun 30, 2020

(Un)versioned URLs #379

Closed

CasperWA requested changes Jul 1, 2020

View reviewed changes

optimade/server/main.py Outdated Show resolved Hide resolved

optimade/server/routers/versions.py Outdated Show resolved Hide resolved

tests/server/routers/test_versions.py Outdated Show resolved Hide resolved

tests/server/routers/test_versions.py Show resolved Hide resolved

shyamd reviewed Jul 1, 2020

View reviewed changes

optimade/server/main.py Outdated Show resolved Hide resolved

CasperWA requested changes Jul 1, 2020

View reviewed changes

ml-evs force-pushed the ml-evs/add_versions_endpoint branch 2 times, most recently from 7e46503 to 24b4f9c Compare July 1, 2020 22:49

ml-evs added 4 commits July 2, 2020 11:08

Enable serving the API at the optional unversioned URL

bddbdc1

- This URL is now used when generating the OpenAPI schemas

Introduced the /versions endpoint to both servers

2dd6426

Removed index_base_url field

a1ed3ee

- This commit removes the `index_base_url` from the models and updates all tests, routers and config files accordingly. - `index_base_url` is now a top-level config option.

Update models for 1.0.0

9b293c9

ml-evs force-pushed the ml-evs/add_versions_endpoint branch from f0fb738 to 9b293c9 Compare July 2, 2020 11:19

ml-evs changed the title ~~Add the /versions endpoint~~ Update models, endpoints and responses to 1.0.0 Jul 2, 2020

ml-evs added priority/high Issue or PR with a consensus of high priority schema Concerns the schema models server Issues pertaining to the example server implementation labels Jul 2, 2020

ml-evs requested review from shyamd and CasperWA July 2, 2020 11:27

shyamd previously approved these changes Jul 2, 2020

View reviewed changes

CasperWA requested changes Jul 2, 2020

View reviewed changes

ml-evs dismissed shyamd’s stale review via 9da9147 July 2, 2020 17:21

CasperWA requested changes Jul 2, 2020

View reviewed changes

tests/server/utils.py Outdated Show resolved Hide resolved

ml-evs force-pushed the ml-evs/add_versions_endpoint branch 2 times, most recently from 4a694dc to 83176d4 Compare July 2, 2020 21:12

Apply suggestions from code review

65e5136

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>

CasperWA requested changes Jul 2, 2020

View reviewed changes

optimade/server/routers/versions.py Outdated Show resolved Hide resolved

ml-evs force-pushed the ml-evs/add_versions_endpoint branch from 83176d4 to 65e5136 Compare July 2, 2020 21:24

CasperWA approved these changes Jul 2, 2020

View reviewed changes

ml-evs merged commit b9ce81c into master Jul 2, 2020

ml-evs deleted the ml-evs/add_versions_endpoint branch July 2, 2020 21:31

ml-evs mentioned this pull request Sep 1, 2020

Suggestion: use absolute imports in app code to allow re-use #298

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update models, endpoints and responses to 1.0.0 #380

Update models, endpoints and responses to 1.0.0 #380

ml-evs commented Jun 30, 2020 •

edited

codecov bot commented Jun 30, 2020 •

edited

CasperWA left a comment

CasperWA commented Jul 1, 2020 •

edited

ml-evs commented Jul 1, 2020

ml-evs commented Jul 1, 2020

ml-evs commented Jul 1, 2020

ml-evs commented Jul 2, 2020

CasperWA left a comment

CasperWA Jul 2, 2020

ml-evs Jul 2, 2020

CasperWA Jul 2, 2020

Update models, endpoints and responses to 1.0.0 #380

Update models, endpoints and responses to 1.0.0 #380

Conversation

ml-evs commented Jun 30, 2020 • edited

codecov bot commented Jun 30, 2020 • edited

Codecov Report

CasperWA left a comment

Choose a reason for hiding this comment

CasperWA commented Jul 1, 2020 • edited

ml-evs commented Jul 1, 2020

ml-evs commented Jul 1, 2020

ml-evs commented Jul 1, 2020

ml-evs commented Jul 2, 2020

CasperWA left a comment

Choose a reason for hiding this comment

CasperWA Jul 2, 2020

Choose a reason for hiding this comment

ml-evs Jul 2, 2020

Choose a reason for hiding this comment

CasperWA Jul 2, 2020

Choose a reason for hiding this comment

ml-evs commented Jun 30, 2020 •

edited

codecov bot commented Jun 30, 2020 •

edited

CasperWA commented Jul 1, 2020 •

edited