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

Request: Include remaing rate-limit in responses #504

Closed
saguiitay opened this issue Jun 3, 2014 · 6 comments
Closed

Request: Include remaing rate-limit in responses #504

saguiitay opened this issue Jun 3, 2014 · 6 comments

Comments

@saguiitay
Copy link

For example, in the SearchCodeResult (and all other Result objects), include a remaining rate limits.

This will allow an extremely simple to implement throttling of my requests

@shiftkey
Copy link
Member

shiftkey commented Jun 3, 2014

Possibly related to #352

@shiftkey
Copy link
Member

shiftkey commented Jul 7, 2014

Going to move this to post-v1 as I think this is hard to do without impacting existing APIs

@Red-Folder
Copy link
Contributor

Couple of options maybe:

  1. Use the suggestion mentioned in Support Etags #352 of storing the last ApiInfo.
  • Add a ApiInfo property to HttpClientAdapter (LastApiInfo).
  • Set the LastApiInfo in HttpClientAdapter.Send (amend the return BuildResponse)
  • In your code, manually new the HttpClientAdapter (so you can keep a reference), manually new a Connection (using the HttpClientAdapter) and pass into GitHubClient
  • In your code, call an API, then use your reference to HttpClientAdapter to get the LastApiInfo
  1. Create a base class for all Api models. Within the base call, store the ApiInfo. Given that some Octokit call will have multiple GitHub Api call, possible store all ApiInfo responses for each call (1 per page?).

I'm currently using the former in some test code - but doesn't feel correct to add to a PR.

@shiftkey
Copy link
Member

I'm currently using the former in some test code - but doesn't feel correct to add to a PR.

I think 1) is going to be a more maintainable solution in the long-term. The other solution means deserializing becomes a two-step operation (read the content, then read the headers). Would love to hear your concerns.

@Red-Folder
Copy link
Contributor

Gonna have a go at option 1 and produce a PR for review (hopefully next 24 hours)

Basically just going to provide a property on the client which calls through to connection which calls through the HTTPClientAdaptor (which will store the last ApiInfo during the Send).

Putting the chained calls all the way through the stack avoids having to keep a reference to the HttpClientAdaptor - which makes it easier for the user.

@shiftkey
Copy link
Member

As this requires significant thought - and likely significant changes to the API responses - as well as there exists the ability to get this data using IGitHubClient.GetLastApiInfo() I'm happy to close this out for now.

I've opened #1142 to add some documentation for this feature before I forget.

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

No branches or pull requests

3 participants