Skip to content

Comments

Return correct Retry After Header value#33

Merged
sven-rosenzweig merged 4 commits intomasterfrom
fix/rate_limit_calculus
Jul 25, 2025
Merged

Return correct Retry After Header value#33
sven-rosenzweig merged 4 commits intomasterfrom
fix/rate_limit_calculus

Conversation

@sven-rosenzweig
Copy link
Contributor

If we do not want to suspend requests (max_sleep_time = 0), the lua scripts, calculating the rate limit, always returns 0 for the Retry-After Header value. Tools like terroform/opentofu will not slow down their deployment process, but instead simply abort

With this commit the lua scripts returns the correct value rather than 2 * max_sleep_time. Introducing a boolean indicating whether a request is suspended or not.

This commit adds three test cases for the rate limit calculus:

  1. No limit reached,
  2. Limit reached and suspension
  3. Limit reached and no supspension

@sven-rosenzweig sven-rosenzweig requested a review from mutax June 23, 2025 12:06
@sven-rosenzweig sven-rosenzweig force-pushed the fix/rate_limit_calculus branch 7 times, most recently from d581af1 to 184ab4d Compare June 24, 2025 10:27
@sven-rosenzweig sven-rosenzweig marked this pull request as ready for review July 21, 2025 15:23
mutax

This comment was marked as outdated.

@sven-rosenzweig sven-rosenzweig force-pushed the fix/rate_limit_calculus branch 2 times, most recently from f33db7a to d6253db Compare July 25, 2025 12:02
If we do not want to suspend requests (max_sleep_time = 0), the lua
scripts, calculating the rate limit, always returns 0 for the
Retry-After Header value. Tools like terraform/OpenTofu will not slow
down their deployment process, but instead simply abort.

With this commit the lua scripts returns the correct value rather than
2 * max_sleep_time. Additionally we ensure never to return zero to the
client and log a warning if the lua script calculates zero as value.

This commit adds three test cases for the rate limit calculus:
1. No limit reached
2. Limit reached and suspension
3. Limit reached and no suspension
We do not support python version 2, so we remove it from the setup.cfg.

Run tox with multiple python versions.
@sven-rosenzweig sven-rosenzweig force-pushed the fix/rate_limit_calculus branch from d6253db to 440c0cb Compare July 25, 2025 12:25
Copy link

@mutax mutax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sven-rosenzweig sven-rosenzweig merged commit 51e2830 into master Jul 25, 2025
4 checks passed
@sven-rosenzweig sven-rosenzweig deleted the fix/rate_limit_calculus branch July 25, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants