Skip to content

Capture ratelimit-remaining and ratelimit-reset headers in AbstractReply #351

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.

Sign up for GitHub

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

Jump to bottom
Closed
GrizzlT wants to merge 7 commits into from
Closed

Capture ratelimit-remaining and ratelimit-reset headers in AbstractReply #351

GrizzlT wants to merge 7 commits into from

Conversation

@GrizzlT
Copy link
Contributor

@GrizzlT GrizzlT commented Dec 17, 2020

This is a handy addition to provide an easy way to know how many requests this key has left and how many seconds until the request counter for this key gets reset all shipped with the reply.

@GrizzlT
Copy link
Contributor Author

GrizzlT commented Dec 17, 2020 •
edited
Loading

This pull request has the Example module disable in the pom.xml due to incorrect referencing to the latest version. #342 has this fixed.

@HydroPage
Copy link
Contributor

HydroPage commented Apr 21, 2021

It seems our PRs are both very similar (#327). If my PR is merged, would this remain useful?

HydroPage
HydroPage suggested changes Apr 21, 2021
View reviewed changes
@GrizzlT GrizzlT mentioned this pull request Apr 22, 2021
Closed
@GrizzlT
Copy link
Contributor Author

GrizzlT commented Apr 24, 2021

I have not had time to test this yet, trying to do this soon

ConnorLinfoot
ConnorLinfoot requested changes May 10, 2021
View reviewed changes
Copy link
Member

@ConnorLinfoot ConnorLinfoot left a comment

Choose a reason for hiding this comment

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

The implementation of this seems pretty hacked in with how it's being appended to the response body. It may be better to branch out from #399 and adding the headers to the HypixelHttpResponse object.

@GrizzlT GrizzlT changed the base branch from master to 4.0.0 May 11, 2021 12:15
@GrizzlT GrizzlT changed the base branch from 4.0.0 to master May 11, 2021 12:19
@GrizzlT
Copy link
Contributor Author

GrizzlT commented May 11, 2021

With #399 coming up, it doesn't make much sense anymore to pass these headers to the HypixelHttpResponse, they would be used before reaching that point, that's why I implemented a transport using Project Reactor that uses these headers internally to handle rate-limiting automatically. See #425
I'll close this pr

@GrizzlT GrizzlT closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ConnorLinfoot ConnorLinfoot ConnorLinfoot requested changes

+1 more reviewer

@HydroPage HydroPage HydroPage requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GrizzlT @HydroPage @ConnorLinfoot