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

Update base URLs #155

Merged
merged 4 commits into from
Feb 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
390 changes: 6 additions & 384 deletions openapi/index_openapi.json

Large diffs are not rendered by default.

1,484 changes: 21 additions & 1,463 deletions openapi/openapi.json

Large diffs are not rendered by default.

34 changes: 20 additions & 14 deletions optimade/server/exception_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,27 @@ def general_exception(
Error(detail=detail, status=status_code, title=str(exc.__class__.__name__))
]

return JSONResponse(
status_code=status_code,
content=jsonable_encoder(
ErrorResponse(
meta=meta_values(
url=str(request.url),
data_returned=0,
data_available=0,
more_data_available=False,
**{CONFIG.provider["prefix"] + "traceback": tb},
),
errors=errors,
try:
response = ErrorResponse(
meta=meta_values(
url=str(request.url),
data_returned=0,
data_available=0,
more_data_available=False,
**{CONFIG.provider["prefix"] + "traceback": tb},
),
exclude_unset=True,
),
errors=errors,
)
except Exception:
# This was introduced due to the original raise of an HTTPException if the
# 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.
Copy link
Member

Choose a reason for hiding this comment

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

So, what exactly would be excepting in the try block?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

@CasperWA CasperWA Feb 3, 2020

Choose a reason for hiding this comment

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

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).

response = ErrorResponse(errors=errors)

return JSONResponse(
status_code=status_code, content=jsonable_encoder(response, exclude_unset=True)
)


Expand Down
60 changes: 35 additions & 25 deletions optimade/server/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
import optimade.server.exception_handlers as exc_handlers


base_urls = {
"major": f"/optimade/v{__api_version__.split('.')[0]}",
"minor": f"/optimade/v{__api_version__.split('.')[1]}",
"patch": f"/optimade/v{__api_version__.split('.')[2]}",
}

app = FastAPI(
title="OPTiMaDe API",
description=(
Expand All @@ -27,9 +33,9 @@
This specification is generated using [`optimade-python-tools`](https://github.com/Materials-Consortia/optimade-python-tools/tree/v{__version__}) v{__version__}."""
),
version=__api_version__,
docs_url="/optimade/extensions/docs",
redoc_url="/optimade/extensions/redoc",
openapi_url="/optimade/extensions/openapi.json",
docs_url=f"{base_urls['major']}/extensions/docs",
redoc_url=f"{base_urls['major']}/extensions/redoc",
openapi_url=f"{base_urls['major']}/extensions/openapi.json",
)


Expand Down Expand Up @@ -61,6 +67,7 @@ def load_entries(endpoint_name: str, endpoint_collection: MongoCollection):
load_entries(name, collection)


# Add various exception handlers
app.add_exception_handler(StarletteHTTPException, exc_handlers.http_exception_handler)
app.add_exception_handler(
RequestValidationError, exc_handlers.request_validation_exception_handler
Expand All @@ -70,28 +77,28 @@ def load_entries(endpoint_name: str, endpoint_collection: MongoCollection):
app.add_exception_handler(Exception, exc_handlers.general_exception_handler)


# Create the following prefixes:
# /optimade
# /optimade/vMajor (but only if Major >= 1)
# /optimade/vMajor.Minor
# /optimade/vMajor.Minor.Patch
valid_prefixes = ["/optimade"]
version = [int(_) for _ in __api_version__.split(".")]
while version:
if version[0] or len(version) >= 2:
valid_prefixes.append(
"/optimade/v{}".format(".".join([str(_) for _ in version]))
)
version.pop(-1)

for prefix in valid_prefixes:
app.include_router(info.router, prefix=prefix)
app.include_router(links.router, prefix=prefix)
app.include_router(references.router, prefix=prefix)
app.include_router(structures.router, prefix=prefix)


def update_schema(app):
# Add various endpoints to `/optimade/vMAJOR`
app.include_router(info.router, prefix=base_urls["major"])
app.include_router(links.router, prefix=base_urls["major"])
app.include_router(references.router, prefix=base_urls["major"])
app.include_router(structures.router, prefix=base_urls["major"])


def add_optional_versioned_base_urls(app: FastAPI):
"""Add the following OPTIONAL prefixes/base URLs to server:
```
/optimade/vMajor.Minor
/optimade/vMajor.Minor.Patch
```
"""
for version in ("minor", "patch"):
app.include_router(info.router, prefix=base_urls[version])
app.include_router(links.router, prefix=base_urls[version])
app.include_router(references.router, prefix=base_urls[version])
app.include_router(structures.router, prefix=base_urls[version])


def update_schema(app: FastAPI):
"""Update OpenAPI schema in file 'local_openapi.json'"""
package_root = Path(__file__).parent.parent.parent.resolve()
if not package_root.joinpath("openapi").exists():
Expand All @@ -102,4 +109,7 @@ def update_schema(app):

@app.on_event("startup")
async def startup_event():
# Update OpenAPI schema on versioned base URL `/optimade/vMAJOR`
update_schema(app)
# Add API endpoints for OPTIONAL base URLs `/optimade/vMAJOR.MINOR` and `/optimade/vMAJOR.MINOR.PATCH`
add_optional_versioned_base_urls(app)
48 changes: 28 additions & 20 deletions optimade/server/main_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
import optimade.server.exception_handlers as exc_handlers


base_urls = {
"major": f"/index/optimade/v{__api_version__.split('.')[0]}",
"minor": f"/index/optimade/v{__api_version__.split('.')[1]}",
"patch": f"/index/optimade/v{__api_version__.split('.')[2]}",
}

app = FastAPI(
title="OPTiMaDe API - Index meta-database",
description=(
Expand All @@ -26,9 +32,9 @@
This specification is generated using [`optimade-python-tools`](https://github.com/Materials-Consortia/optimade-python-tools/tree/v{__version__}) v{__version__}."""
),
version=__api_version__,
docs_url="/index/optimade/extensions/docs",
redoc_url="/index/optimade/extensions/redoc",
openapi_url="/index/optimade/extensions/openapi.json",
docs_url=f"{base_urls['major']}/extensions/docs",
redoc_url=f"{base_urls['major']}/extensions/redoc",
openapi_url=f"{base_urls['major']}/extensions/openapi.json",
)


Expand All @@ -46,6 +52,7 @@
print("done inserting index links...")


# Add various exception handlers
app.add_exception_handler(StarletteHTTPException, exc_handlers.http_exception_handler)
app.add_exception_handler(
RequestValidationError, exc_handlers.request_validation_exception_handler
Expand All @@ -55,26 +62,24 @@
app.add_exception_handler(Exception, exc_handlers.general_exception_handler)


# Create the following prefixes:
# /optimade
# /optimade/vMajor (but only if Major >= 1)
# /optimade/vMajor.Minor
# /optimade/vMajor.Minor.Patch
valid_prefixes = ["/index/optimade"]
version = [int(_) for _ in app.version.split(".")]
while version:
if version[0] or len(version) >= 2:
valid_prefixes.append(
"/index/optimade/v{}".format(".".join([str(_) for _ in version]))
)
version.pop(-1)
# Add various endpoints to `/optimade/vMAJOR`
app.include_router(index_info.router, prefix=base_urls["major"])
app.include_router(links.router, prefix=base_urls["major"])


for prefix in valid_prefixes:
app.include_router(index_info.router, prefix=prefix)
app.include_router(links.router, prefix=prefix)
def add_optional_versioned_base_urls(app: FastAPI):
"""Add the following OPTIONAL prefixes/base URLs to server:
```
/index/optimade/vMajor.Minor
/index/optimade/vMajor.Minor.Patch
```
"""
for version in ("minor", "patch"):
app.include_router(index_info.router, prefix=base_urls[version])
app.include_router(links.router, prefix=base_urls[version])


def update_schema(app):
def update_schema(app: FastAPI):
"""Update OpenAPI schema in file 'local_index_openapi.json'"""
package_root = Path(__file__).parent.parent.parent.resolve()
if not package_root.joinpath("openapi").exists():
Expand All @@ -85,4 +90,7 @@ def update_schema(app):

@app.on_event("startup")
async def startup_event():
# Update OpenAPI schema on versioned base URL `/optimade/vMAJOR`
update_schema(app)
# Add API endpoints for OPTIONAL base URLs `/optimade/vMAJOR.MINOR` and `/optimade/vMAJOR.MINOR.PATCH`
add_optional_versioned_base_urls(app)
2 changes: 1 addition & 1 deletion optimade/server/routers/index_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def get_info(request: Request):
api_version=f"v{__api_version__}",
available_api_versions=[
{
"url": f"{CONFIG.provider['index_base_url']}/v{__api_version__}/",
"url": f"{CONFIG.provider['index_base_url']}/v{__api_version__.split('.')[0]}/",
"version": f"{__api_version__}",
}
],
Expand Down
2 changes: 1 addition & 1 deletion optimade/server/routers/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def get_info(request: Request):
api_version=f"v{__api_version__}",
available_api_versions=[
{
"url": f"{base_url}/optimade/v{__api_version__}",
"url": f"{base_url}/optimade/v{__api_version__.split('.')[0]}",
"version": __api_version__,
}
],
Expand Down
22 changes: 16 additions & 6 deletions optimade/server/routers/utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# pylint: disable=import-outside-toplevel
import urllib
from datetime import datetime
from typing import Union, List, Dict

from starlette.exceptions import HTTPException as StarletteHTTPException
from fastapi import HTTPException
from starlette.requests import Request

from optimade import __api_version__
from optimade.models import (
ResponseMeta,
EntryResource,
Expand Down Expand Up @@ -34,16 +36,24 @@ def meta_values(
**kwargs,
) -> ResponseMeta:
"""Helper to initialize the meta values"""
from optimade import __api_version__
from optimade.models import ResponseMetaQuery, Provider, Implementation
from optimade.server.main import base_urls
from optimade.server.main_index import base_urls as index_base_urls

parse_result = urllib.parse.urlparse(url)

for prefix in list(base_urls.values()) + list(index_base_urls.values()):
if parse_result.path.startswith(prefix):
url_path = parse_result.path[len(prefix) :]
break
else:
# Raise warning
url_path = parse_result.path

provider = CONFIG.provider.copy()
provider["prefix"] = provider["prefix"][1:-1] # Remove surrounding `_`
return ResponseMeta(
query=ResponseMetaQuery(
representation=f"{parse_result.path}?{parse_result.query}"
),
query=ResponseMetaQuery(representation=f"{url_path}?{parse_result.query}"),
api_version=f"v{__api_version__}",
time_stamp=datetime.utcnow(),
data_returned=data_returned,
Expand Down Expand Up @@ -207,7 +217,7 @@ def get_single_entry(
included = get_included_relationships(results, ENTRY_COLLECTIONS)

if more_data_available:
raise StarletteHTTPException(
raise HTTPException(
status_code=500,
detail=f"more_data_available MUST be False for single entry response, however it is {more_data_available}",
)
Expand Down
3 changes: 2 additions & 1 deletion optimade/validator/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
models in this package.

"""
# pylint: disable=import-outside-toplevel

import time
import requests
Expand Down Expand Up @@ -116,7 +117,7 @@ def get(self, request: str):
self.response = requests.get(self.last_request)
except requests.exceptions.ConnectionError:
sys.exit(f"No response from server at {self.last_request}")
except requests.exceptions.MissingField:
except requests.exceptions.MissingSchema:
CasperWA marked this conversation as resolved.
Show resolved Hide resolved
sys.exit(
f"Unable to make request on {self.last_request}, did you mean http://{self.last_request}?"
)
Expand Down
2 changes: 1 addition & 1 deletion tests/server/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
app.include_router(structures.router)
# need to explicitly set base_url, as the default "http://testserver"
# does not validate as pydantic AnyUrl model
CLIENT = TestClient(app, base_url="http://example.org/optimade")
CLIENT = TestClient(app, base_url="http://example.org/optimade/v0")


class EndpointTests(abc.ABC):
Expand Down