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

Rate limit requests for addons.xml.gz #271

Merged
merged 3 commits into from
May 25, 2024
Merged

Conversation

MoojMidge
Copy link
Contributor

Rate limiting has been added to mirrors.kodi.tv which the multiple requests for the various repository addons.xml.gz will trigger.

This adds some rate limiting, error handling and retry backoff to avoid the rate limit resulting in the addon checker failing

Rate limiting has been added to mirrors.kodi.tv which the multiple requests for the various repository addons.xml.gz will trigger.

This adds some rate limiting, error handling and retry backoff to avoid the rate limit resulting in the addon checker failing
def __init__(self, version, path):
super().__init__()
self.version = version
self.path = path
self.addons = []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this line should be moved back to where it was, so that the checker will fail if the versioned repository download fails due to some other server error that cannot be resolved by the allowed retries?

@MoojMidge
Copy link
Contributor Author

One issue with this PR is that, while it does prevent the rate limiting, it may allow for the possibility of the checker appearing to complete successfully, if some other server error occurs.

In this instance the checker would just run with an empty list of addons for the versioned repository that failed to download. Perhaps this should still raise an exception?

It was also suggested that not all these repositories (currently Gotham and up) should be checked anymore. Maybe only Leia and above?

@basrieter
Copy link
Contributor

Could we not just send a specific header that will disable the rate limit for that call? The main reason we limit the rate is the normal repository fetching of addon.xml(.md5) that floods the servers. So we could perhaps differentiate on the agent header in the rate limiter?

@MoojMidge
Copy link
Contributor Author

Yes, that is also an option, but would only apply to mirrors.kodi.tv itself.

The actual mirrors themselves would not respond in the same manner.

I updated the PR:

  • removed the explicit time delays between requests, while still keeping the retry back-off
  • instead limit the number of connections per host to 3
  • applies to mirrors.kodi.tv and any mirrors used, up to the default connection pool limit of 10
  • will block any further connections per host until the existing 3 connections to the host have finished

Should be a little bit more friendly to all the servers involved.

Also moved the self.addons = [] line back to where it was originally, so that the checker would fail if the list of addons still failed to download for some reason.

@razzeee
Copy link
Member

razzeee commented May 25, 2024

Thanks for working on this

@razzeee razzeee merged commit 5630833 into xbmc:master May 25, 2024
@MoojMidge MoojMidge deleted the patch-1 branch May 26, 2024 00:56
@MoojMidge
Copy link
Contributor Author

Just realised there is an issue with this PR if used with older distributions that have old versions of requests/urllib3 e.g. Ubuntu 20.04

requests.adapters.Retry is using the allowed_methods parameter, which was introduced in v1.26 of urllib3 a bit over 3 years ago.

Shall I create a PR for https://github.com/xbmc/addon-check/blob/master/requirements.txt to use this as the minimum version required?

@basrieter
Copy link
Contributor

Thay makes sense.

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