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

include headers in Strava::Errors::Fault response #21

Merged
merged 3 commits into from
Nov 8, 2019
Merged

include headers in Strava::Errors::Fault response #21

merged 3 commits into from
Nov 8, 2019

Conversation

JamesChevalier
Copy link
Contributor

Why:

  • by including the headers, we have access to x-ratelimit-limit and x-ratelimit-usage

How:

  • pass along the headers, similar to message and errors
  • include the headers (as they exist in the current cassette) in the existing specs
  • update the CHANGELOG.md file, as requested
  • update the README.md file to include the existence of e.headers, and include a truncated example of its value

Relevant Links:

Why:
* by including the headers, we have access to `x-ratelimit-limit` and `x-ratelimit-usage`

How:
* pass along the headers, similar to `message` and `errors`
* include the headers (as they exist in the current cassette) in the existing specs
* update the `CHANGELOG.md` file, as requested
* update the `README.md` file to include the existence of `e.headers`, and include a truncated example of its value

Relevant Links:
* #11
* https://developers.strava.com/docs/#rate-limiting
my local `rake` usage display 142 offenses detected, so it seems like there's something different in my local environment compared to Travis CI
I'm hoping this change clears up the errors that it reported, but I can't be certain...
@dblock dblock merged commit f072fa8 into dblock:master Nov 8, 2019
@dblock
Copy link
Owner

dblock commented Nov 8, 2019

Nice and easy.

@dblock
Copy link
Owner

dblock commented Nov 8, 2019

You might still want to do #11 ;)

@JamesChevalier
Copy link
Contributor Author

🤔 Do you think it's worth mentioning details about how to access the data in the README?

For example...
accessing the current usage:
quarterly_usage, daily_usage = e.headers["x-ratelimit-usage"].split(',').map(&:to_i)
and accessing the current limits:
quarterly_limits, daily_limits = e.headers["x-ratelimit-limit"].split(',').map(&:to_i)

👍 Yeah, I still want to get these details into each response. I don't have a lot of confidence in how I'll approach that, so that pull request probably won't be a quick merge like this. 😄

@dblock
Copy link
Owner

dblock commented Nov 8, 2019

Definitely useful for a README, can't hurt.

You could wrap headers into a class and expose an additional method called x_rate_limit and expose limit and usage as first class objects, too.

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.

None yet

2 participants