Add rate limiting to Discord api requests

CSSS / csss-site-backend

CSSS Website Backend (2024 - Present)

4 stars 0 forks source link

Add rate limiting to Discord api requests #47

Closed DerpyWasHere closed 3 months ago

DerpyWasHere commented 3 months ago

Added rate limiting to _discord_request() as described in issue #42.

A few design choices we could talk about:

The constant we compare against
- I chose 2 in case we have multiple threads running for some reason, we could choose differently. I found that the bucket size was 10 requests per 10 seconds.
Sleep time
- I chose to sleep for the remaining time until reset, but this might introduce blocking behavior we may not want. We could potentially handle this in some sort of priority queue? I think in our case though sleeping for the remaining time is sufficient as we're likely not splitting hairs over ms level behavior.
- Alternatively this doesn't even matter as all of the functions in the Discord module are asynchronous anyway and expect to be waiting.

EarthenSky commented 3 months ago

The constant we compare against

What does 2 mean? 2 seconds to sleep after getting rate limited?

EarthenSky commented 3 months ago

I chose to sleep for the remaining time until reset, but this might introduce blocking behavior we may not want. We could potentially handle this in some sort of priority queue? I think in our case though sleeping for the remaining time is sufficient as we're likely not splitting hairs over ms level behavior.

Do you think this would be worthwhile to implement in the future? If so, let's make an issue for it. I'm happy with extra delay for the time being.

DerpyWasHere commented 3 months ago

The constant we compare against

What does 2 mean? 2 seconds to sleep after getting rate limited?

2 api calls.

DerpyWasHere commented 3 months ago

I chose to sleep for the remaining time until reset, but this might introduce blocking behavior we may not want. We could potentially handle this in some sort of priority queue? I think in our case though sleeping for the remaining time is sufficient as we're likely not splitting hairs over ms level behavior.

Do you think this would be worthwhile to implement in the future? If so, let's make an issue for it. I'm happy with extra delay for the time being.

Honestly, not really. I doubt we'll hit a large enough scale to warrant it.