Update base URLs #155
Update base URLs #155
Conversation
Have OpenAPI schemas only contain a single version of the full API, i.e., the routers for `/optimade/vMAJOR.MINOR` and `/optimade/vMAJOR.MINOR.PATCH` are added after the OpenAPI schema creation. The version-less base URL has been removed and the standard is now `/optimade/vMAJOR`.
Due to the server tests, it is not straightforward to raise during creation of the top-level meta field, if the expected base URL prefix path is not found, e.g., `/optimade/v0`. Indeed, this should also issue an OPTiMaDe warning instead.
|
let me know once you're done |
Should be fine now since all checks have passed. |
| # path prefix could not be found, e.g., `/optimade/v0`. | ||
| # However, due to the testing, this error cannot be raised anymore. | ||
| # Instead, an OPTiMaDe warning should be issued. | ||
| # Having this try and except is still good practice though. |
There was a problem hiding this comment.
So, what exactly would be excepting in the try block?
There was a problem hiding this comment.
Yeah ... not much at this moment in time - but I hope to implement the OPTiMaDe warnings similarly to Python Warnings, which are a sub-class of Exceptions. I.e., at some point again (hopefully soon) there will be raised something, which can be caught here and relayed as an OPTiMaDe warning ... But at the moment... yeah, not much.
There was a problem hiding this comment.
I don't quite follow - the except block here would only be activated if there was a programming error in the try block, i.e. in the preparation of the error response object.
In this case, one should not raise a Warning but an Error (since something would have gone seriously wrong).
The only difference I see between the response in try and in except is that the one in except does not add all the optimade stuff.
Perhaps one can simply remove the try/except here?
There was a problem hiding this comment.
The except block would be activated if the utility function meta_values raises, for one reason or another. Within meta_values is where the meta->query->representation is created and returned, which is why i originally created the optional_base_urls function - to return all 6 different path prefixes so they could be removed from the urlparsed path.
I don't know if this makes sense ... but if it should happen that the URL path of the server, for any reason, does not have the prefixes we set for it (e.g., /optimade/v1, which actually happens for the TestClient used to test the server) it would in my original commit raise a HTTPException. I tried to fix this in the test setup, but it eluded me, so instead I chose to backtrack, remove the HTTPException raised in meta_values and simply keep the try and except in this part - although it's technically not necessary (unless meta_values should fail due to other unknown reasons).
CasperWA
force-pushed
the
fix_122_base_urls
branch
from
97af883
to
141b412
Compare
|
Looks like this has broken the heroku? https://optimade.herokuapp.com/optimade/info errors for me |
Sure. The "basic" base URL is now |
Ah okay (I see now from the description above that we removed the unversioned URL). The heroku badge in our README is still broken at least, will make a quick PR fixing that. |
Closes #122
The OpenAPI schemas now only provide information about a single base URL, i.e., the routers are not reproduced over and over.
After the schema creation, the separate, now OPTIONAL, versioned base URLs are created.
This means:
/optimade/vMAJOR,MINOR/optimade/vMAJOR.MINOR.PATCHFor the regular server, and similar for the index meta-database, only it's starting with
/index/optimade/....The version-less base URL has been removed, the standard is now
/optimade/vMAJORor/index/optimade/vMAJOR.representationundermeta->querywas wrongly showing part of the base URL, this has now been remedied.Also, it fixes #129