added time range splitting and process status management

[Dolgalad]

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

[codecov[bot]]

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%)	:arrow_down:

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%)	:arrow_down:
speasy/config/__init__.py	95.65% <100.00%> (+0.09%)	:arrow_up:
speasy/webservices/amda/_impl.py	74.75% <100.00%> (-11.27%)	:arrow_down:
speasy/common/variable.py	0.00% <0.00%> (-100.00%)	:arrow_down:
speasy/core/__init__.py	95.00% <0.00%> (-2.50%)	:arrow_down:
speasy/webservices/amda/ws.py	86.39% <0.00%> (-2.05%)	:arrow_down:
speasy/products/variable.py	93.81% <0.00%> (+0.71%)	:arrow_up:

---

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.

[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[bot]]

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

new alerts:

• 1 for Unused import

[lgtm-com[bot]]

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

new alerts:

• 1 for Unused import

[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[bot]]

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

new alerts:

• 1 for Unused import