Skip to content

Fix detect-changes failing on unauth API rate limit#1562

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix/detect-changes-rate-limit
May 20, 2026
Merged

Fix detect-changes failing on unauth API rate limit#1562
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix/detect-changes-rate-limit

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

Summary

Master Benchmarks run 25920610513 (triggered by PR #1558 merge) failed at Detect Changed Benchmarks with exit code 1 in ~200ms — before any visible work. As a result, the v7 batch-1 outputs from that run never made it to SciMLBenchmarksOutput.

Root cause

.github/scripts/detect-changed-benchmarks.sh calls api.github.com/repos/SciML/SciMLBenchmarksOutput/commits with no auth header, so it uses the anonymous 60-req/hour/IP rate limit. That limit is shared across all GHA-hosted runners on the same egress IP and is easy to hit.

When the limit is hit:

  1. The API returns {"message":"API rate limit exceeded..."} (HTTP 403).
  2. The first grep -oE matches the literal message field but the second grep -oE 'SciMLBenchmarks\.jl@[a-f0-9]{40}' finds nothing → exits 1.
  3. set -eo pipefail is in effect → pipefail propagates to the LAST_BUILT_SHA=$(...) assignment.
  4. set -e kills the script before the [[ -n "${LAST_BUILT_SHA}" ]] check can fall through to the HEAD~1 fallback the comment claims it will.

Fix

  1. Auth the curl with Authorization: Bearer ${GITHUB_TOKEN} when the env var is set. Bumps the limit to 5000 req/hour and is per-installation, not per-IP.
  2. Wrap the pipeline in { ...; } || true so a transient API hiccup or empty output repo cleanly falls through to the HEAD~1 fallback instead of aborting the whole workflow.
  3. Pass secrets.GITHUB_TOKEN to the Detect step in benchmarks.yml.

Test plan

  • Merge → next master push triggers detect-changes → completes successfully on the v7 batch-1 commit set
  • Confirm publish step actually runs and pushes to SciMLBenchmarksOutput

Urgency

This is the only thing blocking the v7 batch-1 outputs from publishing — without this fix, master 25920610513's failure means the 5 batch-1 folders (Testing, IntervalNonlinearProblem, NonStiffBVP, StiffBVP, NonStiffDDE) never make it to the docs site, and every future master push will hit the same wall.

Please ignore until reviewed by @ChrisRackauckas.

Run 25920610513 (master push from PR SciML#1558 merge) failed at
"Detect Changed Benchmarks" with exit code 1 in ~200ms — before any
visible work. Root cause: the script's curl to
api.github.com/repos/SciML/SciMLBenchmarksOutput/commits has no auth
header, so it uses the anonymous 60-req/hour-per-IP rate limit which is
shared across all GHA runners on the same egress. When that limit is
hit, the API returns a JSON error body that contains no
SciMLBenchmarks.jl@<sha> pattern, the second grep -oE exits 1,
pipefail propagates the failure to the LAST_BUILT_SHA assignment, and
`set -e` kills the script — before it ever reaches the HEAD~1 fallback
that the comment claims will catch this case.

Two-part fix:

(1) Pass GITHUB_TOKEN to curl as Bearer auth when present. That bumps
    the limit to 5000 req/hour and is per-installation, not per-IP.

(2) Wrap the curl|grep|grep|head|sed pipeline in `{ ...; } || true` so
    a transient API hiccup or genuinely-empty output repo cleanly falls
    through to the HEAD~1 fallback rather than aborting the workflow.

Also passes secrets.GITHUB_TOKEN to the Detect step in benchmarks.yml.

The fallback path already has `|| true` on `git diff` itself, so once
we reach the `if [[ -n "${LAST_BUILT_SHA}" ]]` check with an empty
LAST_BUILT_SHA, the script proceeds normally on the HEAD~1 diff.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
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.

2 participants