🐛 Fixes Retry-After header in global_rate_limit_route

[pcrespov]

What do these changes do?

• 🐛 Fixes Retry-After header in global_rate_limit_route
  • Retry-After is a relative time in secs, not absolute: see https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429
• ♻️ cleanup tests and updated implementation

How to test

cd services/web/server
make install-dev
pytest -vv tests/unit/isolated/test_utils_rate_limiting.py

[codecov]

Codecov Report

Merging #3379 (4c100c7) into master (4fc72ac) will decrease coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff            @@
##           master   #3379     +/-   ##
========================================
- Coverage    83.0%   83.0%   -0.1%     
========================================
  Files         811     811             
  Lines       34410   34416      +6     
  Branches     1358    1358             
========================================
- Hits        28570   28567      -3     
- Misses       5659    5668      +9     
  Partials      181     181             

Flag	Coverage Δ
integrationtests	68.6% <77.7%> (-0.1%)	⬇️
unittests	79.8% <100.0%> (-0.1%)	⬇️

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

Impacted Files	Coverage Δ
...c/simcore_service_webserver/utils_rate_limiting.py	100.0% <100.0%> (ø)
...ector_v2/modules/comp_scheduler/background_task.py	87.5% <0.0%> (-8.4%)	⬇️
.../simcore_service_catalog/db/repositories/groups.py	64.8% <0.0%> (-8.2%)	⬇️
...rector_v2/modules/comp_scheduler/base_scheduler.py	91.1% <0.0%> (-1.3%)	⬇️
...tor_v2/modules/dynamic_sidecar/scheduler/_utils.py	89.6% <0.0%> (-1.2%)	⬇️
...imcore_service_webserver/garbage_collector_core.py	62.0% <0.0%> (-0.7%)	⬇️
.../director/src/simcore_service_director/producer.py	67.3% <0.0%> (-0.3%)	⬇️
...rvice_datcore_adapter/utils/requests_decorators.py	83.7% <0.0%> (+8.1%)	⬆️

[GitHK]

👍

[sanderegg]

not sure I understand why we do not use an external rate limiter, but it was already there I guess.

[sanderegg]

Question: why sync sleep instead of asyncio here?

[pcrespov]

not sure I understand why we do not use an external rate limiter, but it was already there I guess.

Yes it was there already. This is just a fix and some cleanup

@sanderegg Is there a way to rate limit specific entrypoints in our reverse proxy? If so, perhaps we could create a maintenance issue and add it.

[pcrespov]

Question: why sync sleep instead of asyncio here?

It was already there ... it paces the rate in which it hits the entrypoint ... but on second thoughts, if the tasks does not execute, the time.sleep will block it and might cause flaky test . I will change it Did not work, not sure why ... left a note for a more illuminating moment

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[elisabettai]

@pcrespov, description of this one (or title) could be made more clear. ;) But I leave it up to decide if you want to change it.