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

Added ratelimit headers as class properties #57

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

Conversation

riquedev
Copy link

I added three new properties to the OpenCageGeocode class:

  • ratelimit_limit
  • ratelimit_remaining
  • ratelimit_reset

These properties are initialized as None and are populated based on the API response headers. To prevent the code from breaking if the headers are missing or renamed, I implemented the following logic:

headers_keys = {
    'ratelimit_limit': {
        'header': 'x-ratelimit-limit',
        'conversor': lambda v: int(v) if v else self.ratelimit_limit
    },
    'ratelimit_remaining': {
        'header': 'x-ratelimit-remaining',
        'conversor': lambda v: int(v) if v else self.ratelimit_remaining
    },
    'ratelimit_reset': {
        'header': 'x-ratelimit-reset',
        'conversor': lambda v: datetime.fromtimestamp(int(v) / 1e3) if v else self.ratelimit_reset
    }
}

for prop_name, prop_data in headers_keys.items():
    header_value = response.headers.get(prop_data['header'])
    conversor = prop_data['conversor']
    setattr(self, prop_name, conversor(header_value))

Motivation:

I needed to capture this information in another project where I use this package. Since I’m currently not on the paid plan, the rate limit data is not included in the response. However, I thought it would be useful to have these properties available for potential future use if this data becomes available.

Tests:

I added simple tests for these new properties in the test/test_ratelimit_properties.py file to ensure that the changes work as expected.

@mtmail
Copy link
Member

mtmail commented Sep 18, 2024

Exposing the limits to the developer makes sense but I'm not sure yet his approach is the best to understand. The usage resets (at that timestamp), users might up- or downgrade in the meantime. What could work better is to have a method last_reported_rate_limits(). It's similar but more descriptive. It would also need to work with the async methods.

Would that work for you? I can move the code around a bit.

Since I’m currently not on the paid plan, the rate limit data is not included in the response.

Free trial plans have rate limit data included, so do one-time plans.

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.

2 participants