Repository navigation
Hitting rate limits with Travis commit message linting #24567
Description
Opening this as a separate issue for tracking/discussion:
false positive which we speculate was perhaps due to hitting the> GitHub API rate limit or a network issue (#23739 (comment)).
FTR: Here's one example where the rate limit is hit:
https://travis-ci.com/nodejs/node/jobs/158565896#L447
$ if [ "${TRAVIS_PULL_REQUEST}" != "false" ]; then bash -x tools/lint-pr-commit-message.sh ${TRAVIS_PULL_REQUEST}; fi
+GH_API_URL=https://api.github.com
+PR_ID=24366
+'[' -z 24366 ']'
+'[' -z 24366 ']'
++curl -s https://api.github.com/repos/nodejs/node/pulls/24366/commits
+PR_COMMITS='{
"message": "API rate limit exceeded for 52.54.40.118. (But here'\''s the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
"documentation_url": "https://developer.github.com/v3/#rate-limiting"
}'
++node -p 'JSON.parse(process.argv[1])[0].url' '{
"message": "API rate limit exceeded for 52.54.40.118. (But here'\''s the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
"documentation_url": "https://developer.github.com/v3/#rate-limiting"
}'
+FIRST_COMMIT=
+echo 'Unable to determine the first commit for pull request 24366.'
Unable to determine the first commit for pull request 24366.
+exit 1
The command "if [ "${TRAVIS_PULL_REQUEST}" != "false" ]; then bash -x tools/lint-pr-commit-message.sh ${TRAVIS_PULL_REQUEST}; fi" exited with 1.
I suggest we keep an eye out for if this becomes a more common occurrence. Note that the rate limit for unauthenticated GitHub API requests is IP based so it's whatever Travis is running on that IP (so may not be entirely our jobs).
Authenticated GitHub API requests on Travis may be tricky to implement without exposing the token publicly. Encrypted environment variables are not available to pull requests from forks.
Originally posted by @richardlau in #24254 (comment)
Activity
richardlau commentedon Nov 22, 2018
Another occurrence: #21408 (comment)
rvagg commentedon Nov 22, 2018
A workaround might be to set up a very limited proxy that does it for us and only allow the Travis IP addresses to access it. Maybe an extension of the github-bot functionality since it has its own server.
Trott commentedon Nov 22, 2018
Solution used by MozillaSecurity/orion is to not use the API but to scrape the GitHub website instead. 😱
We could do the same, replacing URLs like https://api.github.com/repos/nodejs/node/pulls/24366/commits with https://github.com/nodejs/node/pull/24366/commits and scraping for the info.
Trott commentedon Nov 22, 2018
If we're careful about what we display in our Travis output, I suppose we might be able to use encrypted variables in Travis to provide Travis with authentication information so we can enable authenticated access to the API to increase our limits.
Trott commentedon Nov 22, 2018
/ping @codebytere in case there's some easy way to solve this by asking GitHub. 😄
richardlau commentedon Nov 22, 2018
@Trott but encrypted variables are not available to pull requests from other forks.
Trott commentedon Nov 22, 2018
And, of course, we can always decide to give up and remove the automatic linting for commit message format. Maybe leave the script in tools and mention it CONTRIBUTING.md or whatever.
Trott commentedon Nov 22, 2018
ESLint has a commit-message status check on PRs. Looks like they created it themselves. /ping @not-an-aardvark
Trott commentedon Nov 22, 2018
ESLint GitHub bot: https://github.com/eslint/eslint-github-bot
Commit message linting: https://github.com/eslint/eslint-github-bot/blob/master/src/plugins/commit-message/index.js
I guess if we switch to a bot rather than Travis for this, authenticated access to the API is not-a-problem.
antsmartian commentedon Nov 22, 2018
Saw this now as well : https://travis-ci.com/nodejs/node/jobs/160333929, from #24569.
refack commentedon Nov 22, 2018
@richardlau since the other jobs in the Travis matrix take a long time anyway maybe we can sleep and retry a few times?
refack commentedon Nov 22, 2018
Also we could run this script in Jenkins.
richardlau commentedon Nov 22, 2018
The rate limit is per hour so I'm not sure how practical that would be. The API includes information about the rate, including when it resets, in the response headers (not currently logged in the job/script).
refack commentedon Nov 23, 2018
Well it was an idea 🤷♂️
But we could run it in jenkins with the format I suggested in nodejs/build#1554 (independent nodejs checkout validating the "un-sanitazied" checkout)
tools: don't use GH API for commit message checks
rvagg commentedon Nov 23, 2018
simple fix proposed in #24574
tools: don't use GH API for commit message checks
Trott commentedon Dec 1, 2018
Fixed in 76faccc
tools: don't use GH API for commit message checks
tools: don't use GH API for commit message checks
tools: don't use GH API for commit message checks
tools: don't use GH API for commit message checks