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

Allow using the response_headers or headers when getting Rate Limit Info #1533

Merged

Conversation

DougEdey
Copy link
Contributor

Resolves #1400


Behavior

Before the change?

  • Octokit::TooManyRequests#context.rate_limit methods return nil

After the change?

  • Octokit::TooManyRequests#context.rate_limit methods return the correct values

Other information

  • I've left this as prioritizing the headers field over response_headers, it's not clear to me when headers was valid (I'm not sure if it was an older version of faraday or something), I looked through the code and I think it's because the on_complete uses response_env (might be here instead of response (Faraday::Response has headers delegating to env.response_headers

Note: I can't see where to add in bug fix information in this repo?


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

  • Bugfix: Type: Bug

spec/octokit/client_spec.rb Outdated Show resolved Hide resolved
@nickfloyd nickfloyd added Priority: High Type: Bug Something isn't working as documented labels Feb 6, 2023
Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Hey, @DougEdey Thank you for the contribution here! ❤️ It's g2g!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Octokit::RateLimit fields are all nil after a Octokit::TooManyRequests exception
2 participants