Skip to content

Add rate limiting information to client responses#5

Open
mboylevt wants to merge 4 commits intomasterfrom
mb-RateLimit
Open

Add rate limiting information to client responses#5
mboylevt wants to merge 4 commits intomasterfrom
mb-RateLimit

Conversation

@mboylevt
Copy link
Copy Markdown
Contributor

This PR surfaces our rate limiting headers from both of our RL sources - cloudflare and shapeways.com

@coveralls
Copy link
Copy Markdown

coveralls commented Nov 22, 2019

Coverage Status

Coverage decreased (-4.5%) to 74.392% when pulling d3dc8c7 on mb-RateLimit into 4b82082 on master.

Comment thread shapeways/oauth2_client.py Outdated
Comment on lines +93 to +115
if response.status_code != HTTP_OK:
if response.status_code == HTTP_RATE_LIMITED:
# We're rate limited
rate_limit.is_rate_limited=True

if CF_RATE_LIMIT_RETRY in headers.keys():
# Limited by CF - backoff for a number of seconds
rate_limit.rate_limit_type=CF_RATE_LIMIT
rate_limit.rate_limit_remaining=0
rate_limit.rate_limit_retry_inseconds=headers[CF_RATE_LIMIT_RETRY]
else:
# Shapeways Rate limiting - stupidly, we move the retryInSeconds entry from the response headers
# to the body. Dealing with this here.
rate_limit.rate_limit_remaining = 0
rate_limit.rate_limit_retry_inseconds = response.json()['rateLimit']['retryInSeconds']

return {CONTENT_SUCCESS: False, CONTENT_RATE_LIMIT: rate_limit.__dict__}
else:
# Generic error
content = response.json()
content[CONTENT_SUCCESS] = False
content[CONTENT_RATE_LIMIT] = rate_limit.__dict__
return content
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tiny recommendation to slightly reduce code complexity would be to remove unnecessary elses:

(untested)

# Handle HTTP errors
if response.status_code != HTTP_OK:
    # Generic error
    content = response.json()
    content[CONTENT_SUCCESS] = False
    content[CONTENT_RATE_LIMIT] = rate_limit.__dict__

    if response.status_code == HTTP_RATE_LIMITED:
        # We're rate limited
        rate_limit.is_rate_limited=True

        # Shapeways Rate limiting - stupidly, we move the retryInSeconds entry from the response headers
        # to the body.  Dealing with this here.
        rate_limit.rate_limit_remaining = 0
        rate_limit.rate_limit_retry_inseconds = response.json()['rateLimit']['retryInSeconds']

        if CF_RATE_LIMIT_RETRY in headers.keys():
            # Limited by CF - backoff for a number of seconds
            rate_limit.rate_limit_type=CF_RATE_LIMIT
            rate_limit.rate_limit_retry_inseconds=headers[CF_RATE_LIMIT_RETRY]

        content = {CONTENT_SUCCESS: False, CONTENT_RATE_LIMIT: rate_limit.__dict__}
        
    return content

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not required change => ✅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So i had that originally - trouble is that the json payload only has the rateLimitRetry key if and only if the rate limiter has been achieved. Therefore, this statement cannot be treated as always valid

rate_limit.rate_limit_retry_inseconds = response.json()['rateLimit']['retryInSeconds']

Hence, extra elses

Comment thread .travis.yml
Comment thread shapeways/oauth2_client.py
Comment thread shapeways/oauth2_client.py
Comment thread shapeways/oauth2_client.py
Comment thread shapeways/oauth2_client.py
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