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

Minor re-design #82

Merged
merged 13 commits into from
Sep 7, 2021
Merged

Minor re-design #82

merged 13 commits into from
Sep 7, 2021

Conversation

CasperWA
Copy link
Member

@CasperWA CasperWA commented Jun 8, 2021

This PR was originally intended to implement pagination according to the discussion in #66.
However, due to issues in the re-design mentioned below, this PR is now being remade to represent a minor re-design of the gateway.


To begin with, the response from a gateway query has been re-modelled:

A new data structure has been added to represent a gateway query response: GatewayQueryResponse. This new response changes the structure for the value of the top-level data field by always returning a dictionary/JSON object. The keys of
the dictionary are the database ids and the value is a list of the returned resources from each database. In this way there is no need to alter/add anything to the returned resources, which has been the case up to now, where the id for each resource was prepended with the database id. This is however still in place, somewhat, if a QueryResource is not used for the query, but a straight-through result is desired. Then an OPTIMADE-compliant response is produced, where the database id is added as a meta entry to each returned resource.

To handle this new approach a new utility function for processing an OPTIMADE database response has been written and implemented.

An Enum for OPTIMADE endpoint entry types has been implemented with methods for returning the correct pydantic model for either the resources or response (both many and single), removing the need to pass the import path for these models.

@CasperWA CasperWA added the enhancement New feature or request label Jun 8, 2021
@CasperWA CasperWA force-pushed the issue_66_pagination branch from ad61c98 to c1719df Compare June 9, 2021 09:24
@codecov
Copy link

codecov bot commented Jun 9, 2021

Codecov Report

Merging #82 (784c5af) into main (f6d12a9) will increase coverage by 0.17%.
The diff coverage is 79.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #82      +/-   ##
==========================================
+ Coverage   88.00%   88.17%   +0.17%     
==========================================
  Files          43       40       -3     
  Lines        1250     1192      -58     
==========================================
- Hits         1100     1051      -49     
+ Misses        150      141       -9     
Flag Coverage Δ
pytest 88.17% <79.87%> (+0.17%) ⬆️

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

Impacted Files Coverage Δ
optimade_gateway/common/utils.py 89.74% <0.00%> (-4.86%) ⬇️
optimade_gateway/models/gateways.py 82.92% <ø> (ø)
optimade_gateway/models/resources.py 94.44% <ø> (ø)
optimade_gateway/routers/search.py 75.28% <57.69%> (-5.55%) ⬇️
optimade_gateway/queries/perform.py 86.41% <59.09%> (-7.28%) ⬇️
optimade_gateway/queries/prepare.py 69.56% <60.00%> (-9.75%) ⬇️
optimade_gateway/queries/utils.py 80.55% <70.83%> (-19.45%) ⬇️
optimade_gateway/mongo/collection.py 81.51% <72.58%> (-3.64%) ⬇️
optimade_gateway/routers/queries.py 93.02% <83.33%> (-4.66%) ⬇️
optimade_gateway/models/queries.py 92.59% <89.06%> (-0.75%) ⬇️
... and 19 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 f6d12a9...784c5af. Read the comment docs.

@CasperWA CasperWA force-pushed the issue_66_pagination branch 3 times, most recently from 150d4e0 to a3b70b2 Compare June 21, 2021 12:23
@CasperWA CasperWA force-pushed the issue_66_pagination branch from a3b70b2 to 84aaf0a Compare July 13, 2021 17:34
@CasperWA CasperWA force-pushed the issue_66_pagination branch from c42de7d to e216ffc Compare July 22, 2021 10:48
@CasperWA CasperWA force-pushed the issue_66_pagination branch 4 times, most recently from 48a0231 to 6d02673 Compare August 10, 2021 13:35
@CasperWA CasperWA force-pushed the issue_66_pagination branch from 9e5c2ef to d369865 Compare August 17, 2021 07:31
A new data structure has been added to represent a gateway query
response: `GatewayQueryResponse`.
This new response changes the structure for the value of the top-level
`data` field by always returning a dictionary/JSON object. The keys of
the dictionary are the database `id`s and the value is a list of the
returned resources from each database.
In this way there is no need to alter/add anything to the returned
resources, which has been the case up to now, where the `id` for each
resource was prepended with the database `id`.
This is however still in place, somewhat, if a `QueryResource` is not
used for the query, but a straight-through result is desired. Then an
OPTIMADE-compliant response is produced, where the database `id` is
added as a `meta` entry to each returned resource.

To handle this new approach a new utility function for processing an
OPTIMADE database response has been written and implemented.

An Enum for OPTIMADE endpoint entry types has been implemented with
methods for returning the correct pydantic model for either the
resources or response (both many and single), removing the need to pass
the import path for these models.

Make entry ID ambiguity a warning instead of an exception.
Only supply the pydantic model to `response_model` that represents the
successful response.
Add a dictionary of erroneous status codes with the ErrorResponse
pydantic model to `responses` for all routes.

Fix bugs caught by the tests introduced in the latest commit,
modularizing out processing a database response.

Use new specific exception classes from `optimade`.
@CasperWA CasperWA force-pushed the issue_66_pagination branch from d369865 to 125157d Compare August 23, 2021 08:35
@CasperWA CasperWA changed the title Pagination Minor re-design Sep 6, 2021
CasperWA and others added 8 commits September 7, 2021 16:25
- Minor renaming of routers in main.py.
- Fix validation errors when returning error responses.
- Minimize special meta fields for query response.
- Utilize local `LinksMapper` in `DatabaseMapper`.
Instead of returning a `GatewayQueryResponse` for the single
`QueryResource` endpoints as well as the `GET /search` endpoint, a
`QueriesResponseSingle` is now returned.
This is done to ensure all necessary information is communicated along
with the actual data response, like the used query parameters and such.

The example/test gateways have been updated to use "correct" database
`id`s, where the provider is prepended, separating the provider `id` and
the database `id` with a slash.
This is instead of redirecting to /gateways/{id}/structures as this is
now moving more towards *not* reproducing an actual OPTIMADE
implementation.
Add possibility to return responses from `GET /search` as an OPTIMADE
entry-listing response or as the "standard" gateway query response.

In doing this, some things have been optimized.
Mainly the logic surrounding retrieving and naming databases in the
`POST /search` endpoint.
In order to return a query response as a valid OPTIMADE response, a
method has been added to the `QueryResource` model, which works
similarly to `GET /gateways/<id>/structures`, but working from the basis
of the finished query.
It updates the entry `id`s by prepending the `provider/database/` name,
making the `id`s unique.

Extra safety has been added to the creation of gateway resources, as the
`resource_factory()` now expects a pre-treating `GatewayCreate` entity,
where there are no "unknown" database `id`s in the `database_ids`
attribute. Unknown meaning that all `database_ids` must be represented
in the `databases` attribute.
Remove root validator from `Response` for `GatewayQueryResponse`.
The `GatewayQueryResponse` is special, since it can allow `errors` to be
present together with `data`.
And `meta` is guaranteed to always be present, so there is no need for
the validator.

Also, update the test data to respect the new validators for
`chemical_formula_anonymous`.

Update optimade and pre-commit dependencies.
Remove the routers and tests.

This also simplifies the query processing as a `QueryResource` will
*always* be used, and hence all special cases where it is not used can
be removed.
@CasperWA CasperWA force-pushed the issue_66_pagination branch from 3cdbde5 to 0cd2480 Compare September 7, 2021 14:31
@CasperWA CasperWA marked this pull request as ready for review September 7, 2021 14:32
@CasperWA CasperWA force-pushed the issue_66_pagination branch from 0cd2480 to 36e5ace Compare September 7, 2021 14:54
Apparently its dependency `jsmin` can result in unstable
builds/installations.
@CasperWA CasperWA force-pushed the issue_66_pagination branch from 36e5ace to 784c5af Compare September 7, 2021 14:59
@CasperWA CasperWA merged commit cb7430c into main Sep 7, 2021
@CasperWA CasperWA deleted the issue_66_pagination branch September 7, 2021 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant