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.

Sign up for GitHub

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

Jump to bottom

Update base URLs #155

Merged
merged 4 commits into from
Feb 3, 2020
Merged

Update base URLs #155

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6f6541
Use new versioned base URLs
CasperWA Feb 3, 2020
e4a0bd1
Create util function for retrieving versioned URLs
CasperWA Feb 3, 2020
796b6eb
Revert HTTPException raise for path prefix in meta
CasperWA Feb 3, 2020
141b412
Use dicts instead of function for base URLs
CasperWA Feb 3, 2020
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
View file
Open in desktop

Large diffs are not rendered by default.

1,484 changes: 21 additions & 1,463 deletions openapi/openapi.json
View file
Open in desktop

Large diffs are not rendered by default.

34 changes: 20 additions & 14 deletions optimade/server/exception_handlers.py
View file
Open in desktop
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 •
edited

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
57 changes: 31 additions & 26 deletions optimade/server/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@
from .entry_collections import MongoCollection
from .config import CONFIG
from .routers import info, links, references, structures
from .routers.utils import get_providers
from .routers.utils import get_providers, optional_base_urls

from optimade import __api_version__, __version__
import optimade.server.exception_handlers as exc_handlers


versioned_base_url = f"/optimade/v{__api_version__.split('.')[0]}"
app = FastAPI(
title="OPTiMaDe API",
description=(
Expand All @@ -27,9 +28,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"{versioned_base_url}/extensions/docs",
redoc_url=f"{versioned_base_url}/extensions/redoc",
openapi_url=f"{versioned_base_url}/extensions/openapi.json",
)


Expand Down Expand Up @@ -61,6 +62,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 +72,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=versioned_base_url)
app.include_router(links.router, prefix=versioned_base_url)
app.include_router(references.router, prefix=versioned_base_url)
app.include_router(structures.router, prefix=versioned_base_url)


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 prefix in optional_base_urls(both=False, index=False, include_major=False):
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: 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 +104,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)
44 changes: 24 additions & 20 deletions optimade/server/main_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@

from .config import CONFIG
from .routers import index_info, links
from .routers.utils import optional_base_urls

from optimade import __api_version__, __version__
import optimade.server.exception_handlers as exc_handlers


versioned_base_url = f"/index/optimade/v{__api_version__.split('.')[0]}"
app = FastAPI(
title="OPTiMaDe API - Index meta-database",
description=(
Expand All @@ -26,9 +28,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"{versioned_base_url}/extensions/docs",
redoc_url=f"{versioned_base_url}/extensions/redoc",
openapi_url=f"{versioned_base_url}/extensions/openapi.json",
)


Expand All @@ -46,6 +48,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 +58,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=versioned_base_url)
app.include_router(links.router, prefix=versioned_base_url)


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 prefix in optional_base_urls(both=False, index=True, include_major=False):
app.include_router(index_info.router, prefix=prefix)
app.include_router(links.router, prefix=prefix)


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 +86,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
View file
Open in desktop
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
View file
Open in desktop
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
49 changes: 43 additions & 6 deletions optimade/server/routers/utils.py
View file
Open in desktop
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 All @@ -26,6 +28,35 @@
}


def optional_base_urls(
CasperWA marked this conversation as resolved.
Show resolved Hide resolved
CasperWA marked this conversation as resolved.
Show resolved Hide resolved
both: bool = True, index: bool = False, include_major: bool = True
) -> list:
"""Create and return OPTIONAL versioned base URLs

The parameter `both` takes precedence over the parameter `index`.

:param both: Return path prefixes for both the regular and index meta-database servers (default: True).
:param index: Return _only_ path prefixes for index meta-database (True) or regular server (False, default).
:param include_major: Append `/optimade/vMAJOR` or `/index/optimade/vMAJOR` to the returned list (default: True).
"""
possible_prefixes = []

version = [int(_) for _ in __api_version__.split(".")]
while version:
if (not include_major and len(version) > 1) or include_major:
ver = ".".join([str(_) for _ in version])
if both:
possible_prefixes.append(f"/optimade/v{ver}")
possible_prefixes.append(f"/index/optimade/v{ver}")
elif index:
possible_prefixes.append(f"/index/optimade/v{ver}")
else:
possible_prefixes.append(f"/optimade/v{ver}")
version.pop(-1)

return possible_prefixes


def meta_values(
url: str,
data_returned: int,
Expand All @@ -34,16 +65,22 @@ def meta_values(
**kwargs,
) -> ResponseMeta:
"""Helper to initialize the meta values"""
from optimade import __api_version__
from optimade.models import ResponseMetaQuery, Provider, Implementation

parse_result = urllib.parse.urlparse(url)

for prefix in optional_base_urls():
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 +244,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
View file
Open in desktop
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
View file
Open in desktop
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