Update tests and client to properly test async mode #1517
Conversation
ml-evs
added
tests
Codecov Report
@@ Coverage Diff @@
## master #1517 +/- ##
==========================================
+ Coverage 91.01% 91.02% +0.01%
==========================================
Files 74 74
Lines 4484 4492 +8
==========================================
+ Hits 4081 4089 +8
Misses 403 403
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -119,7 +124,7 @@ def __init__( | |||
| Parameters: | |||
| base_urls: A list of OPTIMADE base URLs to query. | |||
| max_results_per_provider: The maximum number of results to download | |||
| from each provider. | |||
| from each provider (-1 or 0 indicate unlimited). | |||
There was a problem hiding this comment.
What if you only want to know the number of entries that are in the database, without retrieving any actual data?
It would seem to me that if I would fill in 0 that would be exactly what I get. So it seems a bit strange to let "0" mean all results.
There was a problem hiding this comment.
That's what the --count flag is for, which only requests a page_limit=1 and does no pagination.
There was a problem hiding this comment.
This docstring change just reflects the current behavior anyway, if its desirable we can change both
There was a problem hiding this comment.
Thank you for the clarification.
It seems that the optimade python tools indeed return all results for page_limit=0.
So it makes sense to do the same here.
Perhaps we could support returning 0 resources for page_limit=0 in a future version of the optimade python tools.
So the backend database only has to do the count operation and not a find operation as well.
This PR reintroduces proper testing of the async client and better handles incompatible options passed for sync/async.