Skip to content

Improve rate limit exceeded exception #441

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

Merged
merged 1 commit into from
Oct 20, 2016
Merged

Improve rate limit exceeded exception #441

merged 1 commit into from
Oct 20, 2016

Conversation

mohatt
Copy link
Contributor

@mohatt mohatt commented Oct 13, 2016

Adds two helpful methods to ApiLimitExceedException to allow getting the actual limit and reset time. I am working on a console application that needs to get the reset time in order to sleep until Github resets the token. Hope that makes sense, Thanks!

Copy link
Collaborator

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

👍

private $limit;
private $reset;

public function __construct($limit = 5000, $reset = 1800, $code = 0, $previous = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not add the new argument in the middle. It requires everyone using the exception themselves to update their code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, but we are soon about to tag 2.0.0. So it is okey to break BC here.

Though, it should be documented in the change log of the next release.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this going to be only part of a new major release, this change is fine of course.

@Nyholm
Copy link
Collaborator

Nyholm commented Oct 19, 2016

Could you rebase this PR on master so the test will run again?

@mohatt
Copy link
Contributor Author

mohatt commented Oct 19, 2016

Sure

@cursedcoder cursedcoder merged commit f79fb97 into KnpLabs:master Oct 20, 2016
@Nyholm Nyholm mentioned this pull request Oct 20, 2016
@mohatt mohatt deleted the improve-ratelimit-error branch October 20, 2016 11:09
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.

4 participants