search

gtalarico / pyairtable

Python Api Client for Airtable
https://pyairtable.readthedocs.io
MIT License
765 stars 138 forks source link

Sleep Time Optimization #218

Closed alpancs closed 1 year ago

alpancs commented 1 year ago

sleep times can be optimized (reduced) by calculating elapsed times between iterations

alpancs commented 1 year ago

All checks have passed. Please review once again @NicoHood. Thanks!

alpancs commented 1 year ago

please also review @gtalarico thank you

gtalarico commented 1 year ago

This looks good, thanks!

Have you tried this? Curious what the actual performance difference would be. Could you do a few timeit against both?

gtalarico commented 1 year ago

Happy to merge this but would like to see a comparison of performance w/. and w/out this. Want to make sure that the overhead of fetching time on each loop is worth

mesozoic commented 1 year ago

Just offering my two cents, instead of adding complexity to the sleep behavior, I'd love to see this library aim to remove time.sleep calls entirely. It could instead rely on a default retrying behavior that accomplishes the same thing.

The reason is that it's very possible to have multiple processes hitting the API simultaneously with no awareness of each other, and in that circumstance there's no way to predict the appropriate amount of time to wait between API calls. Better to let the Airtable API throttle requests and insert escalating amounts of delay when you get 429s. That's what their official JS library does (code).

gtalarico commented 1 year ago

That's a great point @mesozoic

alpancs commented 1 year ago

sorry @gtalarico, I haven't had time to manage this PR, but I agree with @mesozoic. I also had the same concern about the multi-process usage from the beginning.

should this PR be modified to remove time.sleep entirely?