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

Wait up to 10 seconds for the current API rate-limit period to expire… #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jasongillis
Copy link

… instead of 10 or more seconds.

Thank you for contributing to MythTV.

As many of our developers do not visit GitHub it is important to track
all contributions at our central ticket tracker over at code.mythtv.org

To prevent your contributions being overlooked please create a new ticket
there and refer to this pull request. (Bonus points for linking to the
ticket in the pull request, too. It helps us noticed potentially overlooked
pull requests due to missing tickets.)

https://code.mythtv.org/trac/wiki/TicketHowTo
https://code.mythtv.org/trac/newticket

… instead of 10 or more seconds.

It should be sleeping until the current 40 reqs / 10 seconds rate window is closed instead of waiting 10 seconds. I was seeing 40 requests in about 9 seconds due to network latency, so it should have been sleeping 1 second or so.

It turns out that when handling of the rate limiting was put in, max() was used instead of min(). The section of code that changed should have used min() since it needs to wait for up to 10 seconds to comply with the rate limit guidelines set by tmdb (​https://www.themoviedb.org/faq/api).
@kmdewaal
Copy link
Contributor

The way the code now works, if my understanding is correct, that the time specified in the retry-after field is used with a minimum time of 10 seconds. So if the retry-after time is for example 15 seconds then 15 seconds is used.
I think the current code is correct.
The proposed change would ignore the retry-after value if it is more than 10 seconds which is not correct.

The description mentions that the time used for retries is not taken into account and that this time should be subtracted from the wait time. This may make sense but that is not what the implementation does.

@rcrdnalor
Copy link
Contributor

The corresponding trac issue is https://code.mythtv.org/trac/ticket/13325
See my comment there. Sorry for not updating this pull request with a back link to the ticket.
TLDR: rate limiting is no more in use by themoviedb.
I kept this issue open because tmdb3.py and its associated cache implements a kind of rate-limiting, too.

BTW, the new thetvdb.com api used by ttvdbv4.py takes longer to fetch a single entry than tmdb3.py.

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.

3 participants