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

Add rate limiting information to client responses #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mboylevt
Copy link
Contributor

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

@coveralls
Copy link

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 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

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

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
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

.travis.yml Show resolved Hide resolved
shapeways/oauth2_client.py Show resolved Hide resolved
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