-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add rate limit delay support #177
Conversation
This limits number of requests allowed to be performed in sequence
Tested : too much errors for me with 1.1sec delay. |
@twolaw so better go back to 1.2s? |
@twolaw and how do you even test this? as the rate limit affects only POST method. do you clear your library to get a big amount of updates? can you post a log of how this behaved for you? |
Yes, I cleared a TV show each time I wanted to try another delay value with massive POST requests. I did tests from France, did you test it with differents delay values ? |
@twolaw no, I only tested that if trakt throws, the PTS code handles it. |
@twolaw slow connection should rather benefit for rate limit, as you be making connections slower... and can you share what output did you get? the rate limit itself worked, and you got warning messages on the terminal? you can obfuscate the irrelevant details like show name if you wish. |
That's not good enough because trakt errors slow down the script, having to resend the requests.
Yes the rate limit works, the objective is to be as fast as possible and to get very few trakt rate-limit warnings. |
perhaps add 0.2-second delay additionally to the value of retry-after header? or how is my implementation different than yours? |
maybe it was always that slow (after rate limit being implemented), just did not notice it as sleeping was not logged? |
You should remove the "Sleeping for x sec" logging because it is a normal behaviour and it will create too much noise in logs. |
@twolaw so, it's just a logging matter? or you are still getting hit by rate limit sooner than before? as you didn't share your output I have to guess what's happening. you didn't answer to my previous questions. |
The previous remark about logging is something else I just noticed. Here is logs with 1.2s delay. As you can see, trakt raises very few rate-limit errors.
|
@twolaw the log looks okay. and it's from old code, I have not touched shows sync. I don't see "Sleeping for x sec" at all, so, no logs no problem. however, the mark_as_watched is POST request, how can you make more than 1 request per second? their docs are outdated? |
Communication between trakt server and user computer is not realtime. Requests may take more or less time to reach each other. |
I found one bug, maybe related to problems you were having: |
Not a bug in As I said 1.1s is too short, do you have logs where 1.1s delay doesn't raise too much errors ? |
Since you did not indicate the version you were using, I assumed you used code from this Pull Request.
I'm using 1.1s and have not noticed excess delays that become annoying to me. But I don't clear my libraries either, so based on normal usage. I'm planning to bulk update the watched status, so this becomes rather irrelevant in the new code. With the bulk update, can actually implement |
Implemented batching add_to_collection trakt api calls: |
To rate limit requests without hitting rate limit error, decorate the method like:
All methods share the same pool for rate limits.
Currently, all methods in TraktApi all
GET
method based, so for test only.me
method is rate limitedrefs: