fix(compliance): cache mothership config per sweep, add rate-limit backoff - #66
Merged
Merged
Conversation
…ckoff Two pre-rollout fixes ahead of scaling CLA/DCO enforcement from 3 repos to 130+: - policy_selector.py's github_api() had no rate-limit awareness, unlike the batch reporting pipeline's detect_org_repo_licenses.py. A 403/429 just silently returned None, which could misclassify a repo's policy, falsely mark an already-signed contributor as non-compliant, or leave a required status check stuck pending forever. Now checks X-RateLimit-Remaining/-Reset, sleeps preemptively when low, and retries once (only) on an actual rate-limit 403 — distinguished from a permissions 403 (e.g. the App not being installed on a repo), which now fails loudly instead of looking identical. - process_single_pr() re-fetched cla/allowlist.yml and both license catalogs (one ~5.9MB) on every single call, including once per PR inside cla_sweeper.py's sweep loop. Harmless at today's handful of PRs; at 130+ repos (open-vm-tools alone has 43 open PRs) this would redundantly re-download that blob hundreds of times per 5-minute sweep. Extracted the fetch into fetch_shared_config(), now called once per sweep and passed through, instead of once per PR. required-compliance.yml's single-PR-per-run usage is unaffected. TEMPORARY: cla_sweeper.yml's checkout ref points at this branch for a live test before merge — revert to `main` before merging. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Was pointed at this branch to live-test the rate-limit/caching fix via a real workflow_dispatch run before merge. Restoring ref: main now that testing is done. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-rollout fixes ahead of scaling CLA/DCO enforcement from the current 3 gated repos to ~130+ additional public repos. Both are operational/scale fixes only — the CLA-vs-DCO decision logic itself is unchanged and already proven correct across all code paths (member bypass, commit sign-off, comment+sweeper) on the 3 existing test repos.
policy_selector.py'sgithub_api()): previously had zero rate-limit awareness, unlike the batch reporting pipeline'sdetect_org_repo_licenses.py. A 403/429 just silently returnedNone, which could misclassify a repo's policy, falsely mark an already-signed contributor as non-compliant, or leave a required status check stuck pending forever. Now checksX-RateLimit-Remaining/-Reset, sleeps preemptively when low, and retries once (only) on an actual rate-limit 403 — distinguished from a permissions 403 (e.g. the App not being installed on a repo), which now fails loudly instead of looking identical to a rate limit.process_single_pr()re-fetchedcla/allowlist.ymland both license catalogs (one ~5.9MB, fetched via a full blob download) on every single call, including once per PR insidecla_sweeper.py's sweep loop. Harmless at today's scale; at 130+ repos (open-vm-toolsalone has 44 open PRs) this would redundantly re-download that blob hundreds of times per 5-minute sweep cycle. Extracted the fetch intofetch_shared_config(), now called once per sweep and passed through, instead of once per PR.required-compliance.yml's single-PR-per-run usage is unaffected — it already only fetches once.Test plan
None,fetch_shared_config()shape, andprocess_single_pr()correctly using vs. re-fetching config based onshared_config.cla_sweeper.yml's checkout at this branch, ran the real Compliance Sweeper viaworkflow_dispatch(run 33015966605), confirmed the allowlist/license-catalog fetch happened exactly once for the whole run (not once per PR), zero errors, and compared directly against the prior scheduled run onmain(33013958831) — identical outcome: same PR (#802), same policy decision (CLA), same commit paintedsuccess.cla_sweeper.ymlhas zero diff againstmainother than the two real script changes.🤖 Generated with Claude Code