binderhub icon indicating copy to clipboard operation
binderhub copied to clipboard

CI: fix (regression I introduced?) regarding GitHub API rate limitations

Open consideRatio opened this issue 4 years ago • 4 comments

Not understanding the criteria for when to run the github_api marked pytests or not or the risks of being rate limited, I may have enabled them on PRs and pushes to master and such in #1188, and have now experienced api rate limiting causing failures.

I think we can perhaps choose to run these only once in the PRs to reduce the risk of getting rate limited.

consideRatio avatar Nov 04 '20 01:11 consideRatio

I thought the tests have access to a GitHub API token to avoid the rate limit? What was the state before you changed things in terms of which tests got run and which skipped (should be in the logs of an old job).

betatim avatar Nov 04 '20 14:11 betatim

The API token got rate limited as well i think ;) I think what i did was to make it available on all tests and removed the pytest mark filtering out that these kinds of tests, and that caused this.

consideRatio avatar Nov 04 '20 15:11 consideRatio

Secrets aren't available on PRs, so the token shouldn't be available to those tests, hence the much lower rate limit. I don't think we can run these on PRs at all, since they will never have access to credentials. That was the reasoning behind this bit. We should only run the github_api tests if a token is available.

minrk avatar Feb 08 '21 08:02 minrk

This is documented to only run when github token is provided, but, i believe that may be untrue. Suggested action points for this issue:

  1. Enforce that the github_api marked tests only to run when a github api token is provided. This can be done in a pytest hook to skip all tests marked as github_api if a criteria isn't met - such as the github token isn't configured. https://github.com/jupyterhub/binderhub/blob/94183eafb3124ae1536560ac45a36ab6f79a339e/binderhub/tests/conftest.py#L40-L56
  2. Document the marker that we need to configure c.GitHubRepoProvider.access_token = blabla to have github_api tests run.
  3. Perhaps implement consistently like has been done with other things. https://github.com/jupyterhub/binderhub/blob/94183eafb3124ae1536560ac45a36ab6f79a339e/binderhub/tests/conftest.py#L302-L313 https://github.com/jupyterhub/binderhub/blob/94183eafb3124ae1536560ac45a36ab6f79a339e/binderhub/repoproviders.py#L731-L736

consideRatio avatar Oct 10 '21 10:10 consideRatio