added rate limit with exponential backoff and retry

[jspaaks]

Describe the changes made in this pull request

• Added rate limit with exponential backoff and retry. Refs GitHub API rate limiting #93

• Removed sleep from livetests and clitests

• Added -a and --apikeys-filename to cli. Refs add -a | --apikeys-filename option to have higher rate limit #263 Now reading from env vars

• the functions with which to interact with github, gitlab, etc are in requesting and are organized as follows:

  .
  └── get_from_platform
      ├── get_from_github
      │   ├── get_from_github_no_auth
      │   │   ├── get_from_github_no_auth_api
      │   │   ├── get_from_github_no_auth_frontend
      │   │   └── get_from_github_no_auth_raw
      │   └── get_from_github_with_auth
      │       ├── get_from_github_with_auth_api
      │       ├── get_from_github_with_auth_frontend
      │       └── get_from_github_with_auth_raw
      └── get_from_gitlab
          ├── get_from_gitlab_no_auth
          │   ├── get_from_gitlab_no_auth_api
          │   ├── get_from_gitlab_no_auth_frontend
          │   └── get_from_gitlab_no_auth_raw
          └── get_from_gitlab_with_auth
              ├── get_from_gitlab_with_auth_api
              ├── get_from_gitlab_with_auth_frontend
              └── get_from_gitlab_with_auth_raw
  

  This gives the developer control over the level at which to use the rate limiting decorators, for example different rates apply for each platform, a given platform may have different rate limits for unauthenticated/authenticated, as well as different rates for different parts of their service.

Instructions to review the pull request

Try running the unit tests

pytest tests/

Now replace get_calls() in the decorator in requesting/get_from_github_no_auth.py with 60. Running the tests again will halt halfway through because of rate limits. It will continue very slowly due to exponential backoff and retry, but the retries happen too slow for the ratelimit of 60 per hour, therefore they all ultimately fail with a too-many-calls error.

cli now has a -a | --apikeys-filename option that can be used to point to a secrets file with api keys

Apikeys are read from env vars named APIKEY_GITHUB and APIKEY_GITLAB, e.g.:

$ cat secrets.txt
APIKEY_GITHUB=<user who made the key>:<longish key with random characters>
APIKEY_GITLAB=<user who made the key>:<longish key with random characters>

source secrets.txt
howfairis https://github.com/fair-software/howfairis

Open questions / problems

1. How to bypass the ratelimit decorators when using mocks fixed it, but hides the next point about Repo
2. Repo might need to be extended with apikeys arg as well, since it's making a request to raw. No longer needed now that we're reading from env vars
3. What about instantiating Repo within Checker.__init__? Checker's constructor would then need additional arguments url, branch and path, and could drop repo.
4. On GitHub and GitLab, raw seems to have its own limits, not covered by api key increased rate limit.
5. On GitLab, unauthenticated api calls return a ratelimit of 2000, not 500 like the documentation says. authenticated requests also have ratelimit 2000, this is consistent with the documentation

[fdiblen]

Suggested change

secrets.yml

[jspaaks]

done in upcoming commit

[eriktks]

Suggested change

	github_apikey_is_invalid = apikeys is not None and (apikeys.get("github-key") is None or
	apikeys.get("github-user") is None)
	if apikeys is None or github_apikey_is_invalid:
	if apikeys is None or apikeys.get("github-key") is None or apikeys.get("github-user") is None:

[eriktks]

This code is much shorter than the equivalent code for github. Is it complete?

[jspaaks]

fixed it same as github now

[eriktks]

Probably github here should be changed

[jspaaks]

changed to only json, also added headers to equivalent noauth gitlab case.

[eriktks]

these two lines are not in alphabetical order

[jspaaks]

fixed it

[eriktks]

We went through the code and added some minor comments. Well done!

[fdiblen]

Awesome sos! 🍜
A few suggestions which we should create issues about:

• We need to update the documentation to reflect this work.
• It is a good idea to raise some exceptions, e.g. when the authentication fails.
• If for some reason we cannot use authenticantion and use unathenticated method, we should warn the user.
• Number of calls are hardcoded. We can consider making them configurable.

[jspaaks]

Spawned

1. add documentation to explain requesting module, ratelimiting, decorators #297
2. raise exception when authenticated api request fails while apikeys not None #298
3. Look into making api rate limits configurable #299

the remaining bullet point from #286 (review) is covered by #298

[sonarcloud]

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

72.1% Coverage
0.0% Duplication