update too many req cooldown handling #176
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pvannierop
reviewed
Jul 28, 2025
| public static final String FITBIT_COOLDOWN_STRATEGY_CONFIG = "fitbit.cooldown.strategy"; | ||
| private static final String FITBIT_COOLDOWN_STRATEGY_DOC = "Strategy for handling too many requests cooldown. Options: ROLLING_WINDOW, TOP_OF_HOUR"; | ||
| private static final String FITBIT_COOLDOWN_STRATEGY_DISPLAY = "Cooldown strategy"; | ||
| private static final String FITBIT_COOLDOWN_STRATEGY_DEFAULT = "ROLLING_WINDOW"; |
Contributor
There was a problem hiding this comment.
I think that the TOP_OF_HOUR approach should be the default, right. It conforms to the cooldown strategy of Fitbit.
Member
Author
There was a problem hiding this comment.
Yes, but this is to keep the default behaviour (to not affect existing deployments if they have no notion of this new feature).
|
|
||
| public Duration getTooManyRequestsCooldownInterval() { | ||
| return Duration.ofHours(1); | ||
| return Duration.ofSeconds(getInt(FITBIT_TOO_MANY_REQUESTS_COOLDOWN_CONFIG)); |
Contributor
There was a problem hiding this comment.
Here we have an issue of problems with backwards compatibility I think. We should take care to add some migration note to RADAR-Kubernetes.
Member
Author
There was a problem hiding this comment.
What do you mean? The default is set to 1hr still so should not change any functionality in existing deployments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to this, the rate limit is reset at the top of the hour, unlike the current implementation - using a rolling window from when the error is received.
This PR adds a strategy to calculate cooldown/backoff time based on top of the current hour. But the legacy code is till present and is the default. Configuration can be dones as follows-
This PR also makes the backoff configurable. This was hardcoded to 1hr by default. Though i think we don't need to update this dynamically since rate limit is by the hour (150 req per hour), but would be good for testing and debugging.