jaraco / irc

Full-featured Python IRC library for Python.
MIT License
392 stars 87 forks source link

Throttle and set_rate_limit have limited usability #25

Closed jaraco closed 8 years ago

jaraco commented 8 years ago

I refrained from reopening issue #18 and opened a new issue instead.

Right now, Throttle and set_rate_limit work fine as long as the calls to the function are made as fast as possible.

However, if the function is not called for some time, a following burst of calls will not get limited. This is not useful as a burst to, for example, send_raw can happen in any point in time.

I propose a different implementation where Throttle keeps the time it was last called and decides to throttle based on that.

If you agree that this should be the intended behavior, I can happily write a patch and also append to the relevant test. However, the test will likely take a couple more seconds to run as I'll need to idle for a bit between bursts.


jaraco commented 8 years ago

Attached a patch anyway. Please test against all environments as I only tested against python 2.7 on linux.

Bottom line:


Original comment by: Nick Raptis

jaraco commented 8 years ago

Would you consider uploading the patch as a pull request, either here in bitbucket or on the github mirror? That'll help me review and assign attribution.


Original comment by: Jason R. Coombs

jaraco commented 8 years ago

I was trying to avoid that actually. I have no idea how to properly manage branches in mercurial and how bitbucket does pull requests, but it's as good time as any to try it. Just tell me, should I open a new branch for my commit or is it all the same to you and default is ok?


Original comment by: Nick Raptis

jaraco commented 8 years ago

It's easy. Just 'fork' the repo, commit your changes in a clone (of either repo) (or edit the file in your fork directly through the web), push the changes to your fork (if necessary), and finally, click Pull Request in your fork.

You may use the default branch for all of this.


Original comment by: Jason R. Coombs

jaraco commented 8 years ago

Merged in nickraptis/irc (pull request #18)

Better rate limiting (fixes issue #25)

→ <<cset 441b50f97e73>>


Original comment by: Jason R. Coombs

jaraco commented 8 years ago

Better rate limiting (fixes issue #25)

Previous reate limiting implementation did not work when bursts of calls were separated by a significant amount of time. Now limits based on the time of last call instead of the time the Throttler was started.

→ <>


Original comment by: Jason R. Coombs