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

Provider list fallback and list of providers in both servers' /links-endpoints #455

Merged
merged 7 commits into from
Aug 24, 2020

Conversation

CasperWA
Copy link
Member

@CasperWA CasperWA commented Aug 5, 2020

Fixes #450
Fixes #454

We experienced that providers.optimade.org was down for some time, hence this will make sure to use some fallback's to try and retrieve the list of providers.

Another thing is fixed in this PR. Namely that the list of providers was not included for the index meta-database.
There was a good reason why: The same links MongoDB collection was used in-memory when starting both servers within the same environment.
This has been fixed by the use of replace_one/replaceOne thanks to some help from @shyamd.

CasperWA and others added 2 commits August 5, 2020 18:03
Order:

1. providers.optimade.org
2. GitHub source code raw content for providers repo
3. Log a warning

The list of providers has also been added to the index meta-database
implementation's `/links`-endpoint.
This required a purging of the MongoMock in-memory collections for the
invoke task that updates the OpenAPI JSON files, since the collection
names are shared, and the `insert_many` method would fail when importing
both applications without using re-initalizing the MongoMock in-memory
database and it's collections.
This makes sure we get the list of providers in the `/links`-endpoint,
whether it's for the regular or index meta-database server.

The "original" data is still inserted with `insert_many`, since it's
considered unique to the specific server.

Co-authored-by: Shyam Dwaraknath <shyamd@lbl.gov>
@CasperWA CasperWA requested review from ml-evs and shyamd August 5, 2020 16:09
@CasperWA
Copy link
Member Author

CasperWA commented Aug 5, 2020

Note: This version lost two commits (where I removed the providers submodule). Hence the get_providers() function does not use data from the submodule, while the providers.json is still accessible through optimade.server.data.providers.

I have left it like this for now, but it should go one way or the other:

  1. Either we remove the providers.json and don't use data from the submodule whatsoever, or
  2. We implement another fallback, i.e., to use the providers data from the submodule if the URLs fail, just before emitting a warning if this can also not be found ...

Personally, if we're holding on keeping the submodule in, I'd prefer to also then use it.

@codecov
Copy link

codecov bot commented Aug 5, 2020

Codecov Report

Merging #455 into master will increase coverage by 0.39%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #455      +/-   ##
==========================================
+ Coverage   91.26%   91.65%   +0.39%     
==========================================
  Files          60       60              
  Lines        2815     2828      +13     
==========================================
+ Hits         2569     2592      +23     
+ Misses        246      236      -10     
Flag Coverage Δ
#unittests 91.65% <100.00%> (+0.39%) ⬆️

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

Impacted Files Coverage Δ
optimade/server/main.py 95.08% <100.00%> (+0.34%) ⬆️
optimade/server/main_index.py 95.16% <100.00%> (+0.42%) ⬆️
optimade/server/routers/utils.py 98.01% <100.00%> (+6.85%) ⬆️

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 dc47d80...8f3de2b. Read the comment docs.

@CasperWA
Copy link
Member Author

CasperWA commented Aug 6, 2020

Note: This version lost two commits (where I removed the providers submodule). Hence the get_providers() function does not use data from the submodule, while the providers.json is still accessible through optimade.server.data.providers.

I have left it like this for now, but it should go one way or the other:

  1. Either we remove the providers.json and don't use data from the submodule whatsoever, or
  2. We implement another fallback, i.e., to use the providers data from the submodule if the URLs fail, just before emitting a warning if this can also not be found ...

Personally, if we're holding on keeping the submodule in, I'd prefer to also then use it.

I have implemented option 2. from above now, and also added tests to make sure every possibility reachable is tested and returns/warns what is expected.

@CasperWA CasperWA merged commit 8c2ca1d into master Aug 24, 2020
@CasperWA CasperWA deleted the fix_450_providers-fallback branch August 24, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants