added time range splitting and process status management

[Dolgalad]

Added process status management for AMDA requests
Split time range by days.

[codecov]

Codecov Report

Merging #41 (482c3c6) into main (6282950) will decrease coverage by 1.31%.
The diff coverage is 96.29%.

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   87.23%   85.92%   -1.32%     
==========================================
  Files          31       31              
  Lines        1489     1520      +31     
  Branches      252      258       +6     
==========================================
+ Hits         1299     1306       +7     
- Misses        127      151      +24     
  Partials       63       63              

Flag	Coverage Δ
unittests	85.92% <96.29%> (-1.32%)	⬇️

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

Impacted Files	Coverage Δ
speasy/webservices/amda/rest_client.py	78.00% <90.00%> (-4.23%)	⬇️
speasy/config/__init__.py	95.65% <100.00%> (+0.09%)	⬆️
speasy/webservices/amda/_impl.py	74.75% <100.00%> (-11.27%)	⬇️
speasy/common/variable.py	0.00% <0.00%> (-100.00%)	⬇️
speasy/core/__init__.py	95.00% <0.00%> (-2.50%)	⬇️
speasy/webservices/amda/ws.py	86.39% <0.00%> (-2.05%)	⬇️
speasy/products/variable.py	93.81% <0.00%> (+0.71%)	⬆️

---

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 6282950...482c3c6. Read the comment docs.

[jeandet]

I left few comments in the code. I'm still not 100% sure I want to follow this option.
My main concern is about using a fixed threshold that might really impact DL performances of slow products. For examples trajectories that are sampled each minute could be downloaded for 1 year in one go would be split in 365 requests. And we would pay 365 times the requests latency (IIRC generating requests has side effects on AMDA about TMP folder, is this still true @brenard-irap?).
This PR should have at least one test.

[brenard-irap]

My main concern is about using a fixed threshold that might really impact DL performances of slow products.

A simple rule could be defined to automatically adapt the threshold in relation with the min sampling of the parameter.

IIRC generating requests has side effects on AMDA about TMP folder, is this still true @brenard-irap?

I don't see any problem in my side. The same session will be used for all "splitted request", and the result will have almost the same size in our TMP folder even it's split into several files.

[jeandet]

My main concern is about using a fixed threshold that might really impact DL performances of slow products.

A simple rule could be defined to automatically adapt the threshold in relation with the min sampling of the parameter.

I prefer something like this, then I don't think we need anymore to let the user change it. We now have to define a good threshold.

IIRC generating requests has side effects on AMDA about TMP folder, is this still true @brenard-irap?

I don't see any problem in my side. The same session will be used for all "splitted request", and the result will have almost the same size in our TMP folder even it's split into several files.

Ok, I didn't remember if there was something bound to requests count.

[lgtm-com]

This pull request introduces 1 alert when merging 03bde87 into 6282950 - view on LGTM.com

new alerts:

• 1 for Unused import

[lgtm-com]

This pull request introduces 1 alert when merging 7490295 into 6282950 - view on LGTM.com

new alerts:

• 1 for Unused import

[jeandet]

Looks better to me, can you add a test that validates the case when the request is too long?

[Dolgalad]

Looks better to me, can you add a test that validates the case when the request is too long?

How do you propose to implement the test. speasy does not check if the request is too long, it will just split the requests if the time range exceeds 1 day. I don't like the idea of having a test that tries to download more than a day's worth of data from AMDA.

[jeandet]

Looks better to me, can you add a test that validates the case when the request is too long?

How do you propose to implement the test. speasy does not check if the request is too long, it will just split the requests if the time range exceeds 1 day. I don't like the idea of having a test that tries to download more than a day's worth of data from AMDA.

I think we don't have the choice, if this is implemented, this has to be tested. If this is something users might do then the CI has also to reproduce this behavior, I don't see any other way to ensure this code path works and will work in the future (we might brake it changing something else for example).

[Dolgalad]

Alright, in that case I would propose to modify the test test_amda_parameter by changing the dates to cover more than one day :

• start : 2000-01-01
• stop : 2000-01-02T00:01:00

On another note : the instructions in the documentation for running a single test suite doesn't seem to work : running py.test tests.test_amda returns

=================================== no tests ran in 0.00 seconds ====================================
ERROR: file not found: tests.test_amda

[jeandet]

Alright, in that case I would propose to modify the test test_amda_parameter by changing the dates to cover more than one day :

• start : 2000-01-01
• stop : 2000-01-02T00:01:00

This test focuses on AMDA parameter properties, so the best is to add a test here https://github.com/SciQLop/speasy/blob/main/tests/test_amda.py#L53-L71 as we already test few requests, you can name it test_long_request. The benefit also to make a dedicated test is to be able to tag it and skip it in some configs on CI if we have some issues.

On another note : the instructions in the documentation for running a single test suite doesn't seem to work : running py.test tests.test_amda returns

=================================== no tests ran in 0.00 seconds ====================================
ERROR: file not found: tests.test_amda

Exact it's inherited from https://github.com/audreyfeldroy/cookiecutter-pypackage/blob/master/%7B%7Bcookiecutter.project_slug%7D%7D/CONTRIBUTING.rst#tips :/ I'll update this, the syntax is py.test tests/file.py

[lgtm-com]

This pull request introduces 1 alert when merging d6ac8aa into 651b52a - view on LGTM.com

new alerts:

• 1 for Unused import