Ask pypistats politely: a User-Agent that identifies us, and backoff - #140
Merged
Merged
Conversation
Publishing the count instead of letting shields render it live fixed shields asking pypistats for every project it serves. It did not fix the other end: pypistats rate-limits per IP, a GitHub-hosted runner's IP is shared with everyone else's Actions, and the first real run got 429 in under half a second having asked for nothing. So the caller is now a polite one. A User-Agent that says who it is and where to complain, and five attempts spread over about four minutes; a daily job can afford to wait, and a badge that publishes a wrong number cannot. Anything still not 200 at the end leaves the badge on its last value, which is the failure this was designed around. Demonstrated rather than assumed: run locally against a deliberately exhausted limit, the first attempt returned 429, the second at +15s returned 200, and the step set read=true with 3,450/month.
arpanghoshal
enabled auto-merge (squash)
September 11, 2026 16:11
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe downloads workflow now retries pypistats requests with increasing delays, identifies requests with a ChangesTraffic badge retrieval
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The workflow now handles transient rate limits while preserving the public badge when retrieval ultimately fails. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arpanghoshal
added a commit
that referenced
this pull request
Sep 11, 2026
…verflowError (#142) Found by `fuzz` on #140, which is how a non-required check earns its place: the failure had nothing to do with that PR and everything to do with main. `Policy.from_yaml` promises PolicyError for anything malformed, without qualification, and strict_load is the one loader every CTRLRun document goes through. PyYAML converts a scalar before it has decided the document is well formed, so three conversions came back as the interpreter's own exception and went past `except yaml.YAMLError`: an over-long \U escape overflows converting the codepoint to a C int; "\U00110000" is above the Unicode maximum and chr() says ValueError; and `x: 2026-99-99` is a ValueError from datetime, which is the one that matters because a mistyped month is a thing a person writes. Nothing unsafe was admitted -- the document is refused either way -- so what leaked is the exception type, and the type is the contract: a caller catching PolicyError around a policy load caught none of these. ValueError and OverflowError are refusals now. Deliberately not `except Exception`: strict_load calls one thing, so a MemoryError there is not the document's fault. RecursionError stays uncaught for the same reason fuzz/properties.py excludes it. Three regression tests, the first seeded from the fuzzer's own crash unit decoded as the harness decodes it. Docs tail in CTRLRun/ctrlrun-docs@8f84def. Merged under the authority granted 2026-09-11. CodeRabbit was rate limited and reviewed nothing, which its passing status does not say; what stands behind this instead is scripts/check.sh green locally (2,948 passed, mypy --strict clean), fuzz passing in CI, and the crash unit replayed through check_policy.
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
1 participant
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.
The 429
Publishing the count rather than letting shields render it live fixed one end of the problem: shields asks pypistats on behalf of every project it serves. It did not fix the other. pypistats rate-limits per IP, and a GitHub-hosted runner's IP is shared with everyone else's Actions. The first real run got
429in under half a second, having asked for nothing:The change
A
User-Agentthat identifies the caller and points at this repository, plus five attempts spread over roughly four minutes (0, 15, 45, 90, 120s). A daily job can afford to wait; a badge that publishes a wrong number cannot. Anything still not 200 leaves the badge on its last value, which is the designed failure and already how the missing-token branch behaves.Also replaces
curl -fwith an explicit status read here, matching the traffic step:-fexits 22 and prints the code alone, and the code is the whole diagnosis.Evidence
Run locally against a limit I had already exhausted: attempt 1 returned
429, attempt 2 at +15s returned200, and the step setread=truewith3,450/month.🤖 Generated with Claude Code
Summary by CodeRabbit