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

New repo maintainer(s) / alternative projects? #51

Open
JWCook opened this issue Jan 5, 2021 · 7 comments
Open

New repo maintainer(s) / alternative projects? #51

JWCook opened this issue Jan 5, 2021 · 7 comments

Comments

@JWCook
Copy link

JWCook commented Jan 5, 2021

Hi @tomasbasham, first of all thanks for this handy package. It solves a small but common enough problem such that this is currently the top Google result for "python rate limit" and similar searches.

It appears that the repo has accumulated a number of unanswered issues and PRs over the last couple years. It's understandable if you don't have time to maintain this, but since there are others who are willing and able to make improvements to it, would you be willing to either:

  • Transfer the repo to a different owner, or
  • Add some users with 'Collaborator' level permissions so they can help maintain the project?

Plan B

Otherwise, would anyone else like to volunteer to do the following?

  • Start a fork (if you haven't done so already) that will be a recommended replacement for ratelimit
  • Be available to respond to issues and PRs within a reasonable amount of time
  • Add at least one or two Collaborators to help with this
  • Be willing to transfer the repo to a different owner if at some point in the future you will no longer have time to maintain it

I would suggest @deckar01's fork, containing changes described in #31, as a good starting point. I would also like to see @evemorgen and @Jude188's changes from issue #26 / PR #39 included. I believe the changes from these two forks could be integrated with the use of aiosqlite to support both sliding log persistence and async calls.

@JWCook
Copy link
Author

JWCook commented Feb 22, 2021

After a bit more looking around, I discovered pyrate-limiter, which is actively maintained. It uses the leaky bucket algorithm, which accomplishes the same thing as the 'sliding window' feature described in #31.

I also came across aiolimiter, which is an async implementation of the same algorithm.

After reviewing those two libraries, I think contributing to one or both of those would be preferable to forking ratelimit.

@DavidFelsen
Copy link

Thanks for following up on this. I tried using aiolimiter but it just does not seem to work as well. Ultimately a mix of @deckar01 (fixed logic) and @evemorgen (asyncio compatibility) would have been ideal.

Please let us know if you come across other similar maintained packages.

@JWCook
Copy link
Author

JWCook commented Feb 22, 2021

I'm curious, what issues did you run into with aiolimiter?

I have use cases for both synchronous and asynchronous requests, and I haven't yet worked on the async ones. For those, I'd like to compare aiolimiter, the async fork of ratelimit, and maybe also see what it would take to add async support to pyrate-limiter (which does have a couple extra features I like).

@DavidFelsen
Copy link

DavidFelsen commented Feb 23, 2021

I might be using it wrong but it managed to fail the two toy examples I ran ...

import asyncio
from datetime import datetime
from time import perf_counter

from aiolimiter import AsyncLimiter

# API limits: 5 requests per second, up to 10 requests per second in bursts
rate_limit = AsyncLimiter(5, 1)

async def coro(ref):
	async with rate_limit:
		print('coro', f"t + {(perf_counter() - ref):>7.5f}s")

async def main():
	print(f"Limit should be 5 calls per second\n\n")
	ref = perf_counter()

	tasks_continuous = [coro(ref) for _ in range(10)]

	print('Start of 10 coro calls (burst)')
	print(datetime.now())
	await asyncio.gather(*tasks_continuous)
	print('Finished')
	print(datetime.now())

	print('\n', '#'*80, '\n')

	await asyncio.sleep(1)

	ref = perf_counter()	

	tasks_first_burst = [coro(ref) for _ in range(3)]
	
	print('Start of 3 coro calls then short pause, then 7 more')
	print(datetime.now())
	await asyncio.gather(*tasks_first_burst)
	await asyncio.sleep(0.99)

	tasks_second_burst = [coro(ref) for _ in range(7)]

	await asyncio.gather(*tasks_second_burst)
	print('Finished')
	print(datetime.now())

asyncio.run(main())

Gives me the following:

Limit should be 5 calls per second


Start of 10 coro calls (burst)
2021-02-23 16:10:10.409058
coro t + 0.00009s
coro t + 0.00012s
coro t + 0.00013s
coro t + 0.00014s
coro t + 0.00015s
coro t + 0.20061s
coro t + 0.40102s
coro t + 0.60140s
coro t + 0.80176s
coro t + 1.00207s
Finished
2021-02-23 16:10:11.411169

 ################################################################################

Start of 3 coro calls then short pause, then 7 more
2021-02-23 16:10:12.412351
coro t + 0.00012s
coro t + 0.00015s
coro t + 0.00017s
coro t + 0.99137s
coro t + 0.99141s
coro t + 0.99143s
coro t + 0.99145s
coro t + 0.99146s
coro t + 1.19183s
coro t + 1.39217s
Finished
2021-02-23 16:10:13.804552

As you can see it failed in both cases since both times I end up with more than 5 requests in a 1 second span.

For comparison, ratelimit (with @evemorgen's asyncio update) only fails in the 2nd case, as expected since the logic wasn't fixed:

@sleep_and_retry
@limits(calls=5, period=1)
async def coro(ref):
	print('coro', f"t + {(perf_counter() - ref):>7.5f}s")

output:

Limit should be 5 calls per second


Start of 10 coro calls (burst)
2021-02-23 16:16:25.955455
coro t + 0.00010s
coro t + 0.00011s
coro t + 0.00013s
coro t + 0.00014s
coro t + 0.00015s
coro t + 1.00134s
coro t + 1.00137s
coro t + 1.00140s
coro t + 1.00141s
coro t + 1.00143s
Finished
2021-02-23 16:16:26.956917

 ################################################################################

Start of 3 coro calls then short pause, then 7 more
2021-02-23 16:16:27.958109
coro t + 0.00012s
coro t + 0.00014s
coro t + 0.00016s
coro t + 0.99136s
coro t + 0.99139s
coro t + 1.00158s
coro t + 1.00161s
coro t + 1.00163s
coro t + 1.00165s
coro t + 1.00166s
Finished
2021-02-23 16:16:28.959782

Please let me know your thoughts as I am probably doing something wrong

@JWCook JWCook changed the title New repo owner or maintainer(s)? New repo maintainer(s) / alternative projects? Feb 23, 2021
@JWCook
Copy link
Author

JWCook commented Feb 23, 2021

Isn't that the expected behavior? After the first 5 requests, it reaches the rate limit and starts inserting a delay of 0.2s before each subsequent request, for an average of 5 requests/second.

The total expected request time should be (n_requests/5) - 5 , plus whatever overhead there is for the function call itself.
So if you run your example code with, say, 55 requests, it should take 10 seconds plus some change:

tasks_continuous = [coro(ref) for _ in range(55)]    
start_time = datetime.now()                          
await asyncio.gather(*tasks_continuous)              
print(f'Elapsed time: {datetime.now() - start_time}')

Which for me results in:

Elapsed time: 0:00:10.045647

Is the issue that you would expect those first 5 requests to be spread across 1 second rather than being run immediately? Or for there to be a 1s pause after the first 5 requests instead of a 0.2s pause?

@DavidFelsen
Copy link

The average rate becomes 5/sec but that is not what I expect: I would like my requests to go through as quickly as possible as long as there is 'capacity' available. On top of that if you look at the first example, you can see that 9 requests went through in the first second despite the limit being 5/sec.

@JWCook
Copy link
Author

JWCook commented Feb 23, 2021

Ah! I see what you mean now. In that case I think it would be worth creating an issue for that on the aiolimiter repo.

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

2 participants