Opened 14 months ago

Last modified 14 months ago

#13325 new Patch - Bug Fix

tmdb3 python bindings wait too long for the API rate limit to expire

Reported by: jasongillis Owned by: Raymond Wagner
Priority: trivial Milestone: needs_triage
Component: Bindings - Python Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

For a long time now, I've been noticing that fetching video metadata became really slow. I put up with it (for three years!) until this weekend when I had some time to dig in. In looking at the debug for tmdb3.py, it notes that after 40 queries, it sleeps for 10 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).

Pull request for this trivial change is:

https://github.com/MythTV/mythtv/pull/171

This is my first ever pull request, so let me know if I've done this wrong.

Thanks, Jason

Change History (1)

comment:1 Changed 14 months ago by ijc

I'm not a mythtv dev but IMHO it'd be good if some of the verbiage from above made its way into the commit message (I'm thinking perhaps everything from "It should be sleeping..." in the first para to the end of the second paragraph).

Part of me wonders why we don't just wait for whatever the Retry-After header says if it is present, without either the min or max clamping. Presumably with a 10s fallback if the header is not present.

Which lead me to wonder if the behaviour here has changed when the header is missing since min/max on (None,10) do different things (likewise calling them on ("", 10)). But actually I suppose/assume that e.headers['retry-after'] will raise a key-not-found exception of some sort in that case so min/max never gets called and there's no change from this commit.

So, LGTM modulo the commit message.

Note: See TracTickets for help on using tickets.