-
-
Notifications
You must be signed in to change notification settings - Fork 65
Handle rate limits (API 429 and 406) #122
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
base: master
Are you sure you want to change the base?
Conversation
I'm OK with the second commit, but I don't really understand why the first one is necessary, can you tell me more about how we can hit the limits when using this script? |
I was using the original script to add subtitles to my ripped movie collection, in bulk, and it was often failing - so I would just keep re-running it. When I printed the errors in detail, I was getting lots of 429 responses. I checked the website and OpenSubtitles says that applications should respect the rate-limit and retry after the provided cool down period:
https://apidog.com/blog/opensubtitles-api/ So that's what the first commit does: look for 429 responses and retry after the given |
like run ./OpenSubtitlesDownload.py once on a folder with deep hierarchy? |
Yes, I run it in CLI mode either in bulk or against specific shows, managed by a simple script that calls it on a file-by-file basis: getsubs(){
if [[ $# -eq 0 ]]; then
shopt -s nullglob
set -- *.mkv
shopt -u nullglob
fi
for f; do
local srt="${f%.mkv}.srt"
if [[ -f "$srt" ]]; then
printf 'Skipping %s, already has subtitles\n' "$srt"
elif ! mkvinfo "$f" 2>/dev/null | grep -qi "Track type: subtitles"; then
OpenSubtitlesDownload.py -g cli -s filename --noai --nohi -l en -t auto -a -i "$f"
else
printf 'Skipping %s, already has subtitles\n' "$f"
fi
done
} |
Currently the script often fails with error 429 due to exceeding OpenSubtitles API rate limit. Even if these rate limits are honored, the script will then fail with error 406 if the user's account has exceeded its daily download limit.
This PR adds handling for both: