Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling for Rate Limiter errors - Too many requests #71

Open
santhoshramaraj opened this issue Sep 10, 2024 · 6 comments
Open

Handling for Rate Limiter errors - Too many requests #71

santhoshramaraj opened this issue Sep 10, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@santhoshramaraj
Copy link
Member

santhoshramaraj commented Sep 10, 2024

SystemLink Enterprise's services implement rate limiters to limit the client's requests per second. Whenever a client crosses the rate limiter, the client experiences Too many requests error. Currently, the clients do not have any support for a back-off-retry mechanism to simplify the usage.

@santhoshramaraj santhoshramaraj added the enhancement New feature or request label Sep 10, 2024
@santhoshramaraj
Copy link
Member Author

FileClient incorporates a basic rate limiter handler described in #65

@santhoshramaraj
Copy link
Member Author

DataFrameClient incorporates a basic rate limiter handler described in #80

@ChrisStrykesAgain
Copy link

Maybe I'm missing something obvious, but is there a reason to implement this on a per-service level, instead of just putting it in the base class?

@santhoshramaraj
Copy link
Member Author

Per-service level implementation was a temporary solution to get the integration tests working.

#90 aims to enable users to choose and handle the retry for common use cases. Implementing it in the BaseClient is also a solution and exposes them through each Client arg.

@ChrisStrykesAgain
Copy link

I think many consumers of the API won't be thinking about retry handling, so even if we expose methods and tools to enable it they may not use them. Also, I think the situation where a user specifically wants to not retry (especially on 429s) is by far less common. Given these two things, I think building this into the base class as the default behavior (with the capability to disable) would be an ideal scenario and make more users successful. I'm happy to do said implementation if others are onboard with the idea.

@santhoshramaraj
Copy link
Member Author

That is a valid point, I am good with implementing a default retry behavior in BaseClient and allowing the user to override or disable as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants