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

Fixed: TMDB Failing due to missing response header #3610

Merged
merged 1 commit into from Sep 1, 2019
Merged

Conversation

Qstick
Copy link
Member

@Qstick Qstick commented Jul 10, 2019

Database Migration

NO

Description

Seems our friends at tmdb have changed something, and the rate limit headers are not always returned. This will check for header in response instead of assuming it will be there

Issues Fixed or Closed by this PR

@Qstick Qstick requested a review from galli-leo July 10, 2019 03:12
@galli-leo
Copy link
Contributor

Is there another header we could use to check? Or is it surely gonna be there when we need it?

@Qstick
Copy link
Member Author

Qstick commented Jul 10, 2019

We can just set rate limit property for the httpRequest as that functionality is already built in and used for indexers. I am unsure if it shows up when needed or not as I didn't try loading it, it's still listed as an available auth header in the authHeaders header of the response but doesn't show up by default it seems. Could also be a temporary thing?

@galli-leo
Copy link
Contributor

@Qstick If I understand the ratelimiting mechanic correctly, it basically only allows you to limit the request to one request per TimeSpan. I guess that would still work though, i think we should try doing that in addition to this update.

@Qstick
Copy link
Member Author

Qstick commented Jul 11, 2019

Sounds good, should we set timespan to something like 0.5 sec? Given rate limit is 40 per 10 seconds, That gives us some buffer.

@Qstick
Copy link
Member Author

Qstick commented Jul 12, 2019

Seems headers are back now, but I'll update this with the ratelimit as well so we are guarded in future if this happens again.

@Qstick
Copy link
Member Author

Qstick commented Jul 13, 2019

@galli-leo looking at this a bit, not sure the built in rate limit will help given we are building a new HttpRequest for each Movie info call. Though I'm not familiar enough with the Http code to know.

@Qstick Qstick merged commit df068e9 into develop Sep 1, 2019
@Qstick Qstick deleted the fix-tmdb-header branch September 1, 2019 15:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants