Skip to content
This repository was archived by the owner on Sep 19, 2018. It is now read-only.

Rate Limiting Handling#12

Open
z64 wants to merge 2 commits into
masterfrom
dev/mutex
Open

Rate Limiting Handling#12
z64 wants to merge 2 commits into
masterfrom
dev/mutex

Conversation

@z64
Copy link
Copy Markdown
Contributor

@z64 z64 commented Jul 26, 2017

Implements a mutex around API#request that protects against making rate limited requests.

This works by:

  • Caches the last response we received from the API
  • Checks headers for how many requests we have left (:x_rate_limit_remaining)
  • If we have no requests remaining, and we haven't passed the reset timestamp (x_rate_limit_reset), sleep for x_rate_limit_reset_remaining milliseconds
  • This is all done inside of a Mutex#synchronize block and is therefore thread-safe.

Todo:

  • Squash

Example raw rate limit headers:

x-rate-limit-limit →1s
x-rate-limit-remaining →0
x-rate-limit-reset →2017-07-26T14:39:58.2291610Z
x-rate-limit-reset-remaining →206

@z64 z64 requested a review from snapcase July 26, 2017 16:15
Comment thread lib/rls/api.rb

# @return [Integer] number of requests until we're rate limited
def remaining_requests
last_header(:x_rate_limit_remaining)&.to_i || -1
Copy link
Copy Markdown
Contributor Author

@z64 z64 Jul 26, 2017

Choose a reason for hiding this comment

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

To clarify this defaulting to -1:

last_header(:x_rate_limit_remaining)&.to_i returns [Integer, nil] which would fail the subsequent #zero? check in #will_be_rate_limited?. I opted just to return -1 as it isn't zero (it's unknown until we make our first request).

In contrast, if we default to zero, then #will_be_rate_limited? may return true on the first request and sleep for 0 seconds. This isn't a big deal either, but if we implement something like rate limit logging later, it may look pretty confusing. 1 may also be an acceptable default for this method, but it isn't true (we didn't get that from the API) ; so I think this justifies -1 as a special case.

@z64 z64 added this to the v1.0.0 milestone Jul 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant