Merged
Conversation
* feat: add per-endpoint rate limiting for Scryfall API Scryfall now enforces tiered rate limits: 2/s for /cards/search, /cards/named, /cards/random, /cards/collection, and 10/s for all other endpoints. The four slow endpoints share a single 2/s budget. - Add SlowRateLimiter subclass with 2.0 calls/second default - Change RateLimiter singleton to per-class registry so each rate tier maintains independent state - Add _rate_limiter_class to ScrythonRequestHandler so endpoint classes declare their rate tier - Assign SlowRateLimiter to Search, Named, Random, Collection - rate_limit_per_second kwarg still overrides per-request - rate_limit=False still disables entirely - No public API changes; existing user code works without modification * Formatting * fix: share rate limiter state for rate_limit_per_second overrides The rate_limit_per_second kwarg was creating a new RateLimiter per request, so consecutive calls never throttled against each other. Use a shared registry keyed by (class, rate) so custom rates enforce across calls. Also rename reset_global_limiter to reset_all_limiters to reflect that it clears the entire registry. * docs: add comment explaining limiter coordination trade-off * docs: add Scryfall rate limit docs link * test: widen timing assertion upper bounds for CI reliability * style: revert assert formatting to multi-line form * docs: update README with per-endpoint rate limiting details * Add indirection class method for reset_global_limiter * Add general override key to registry * refactor: move rate_limit_per_second to per-instance limiter Instead of caching override limiters in the global registry (which grows unbounded with unique rate values), store the override limiter on the handler instance. This keeps pagination throttled within a single handler while avoiding an unbounded global cache. Removes get_global_limiter_for_rate() and simplifies the registry type back to dict[type, RateLimiter]. * test: widen all timing test upper bounds for CI reliability Lower bounds prove correctness (rate limiting works). Upper bounds only assert "not absurdly slow" and should be generous enough to pass on heavily loaded CI runners. * docs: fix get_global_limiter docstring to include base class * docs: document rate_limit_per_second per-instance semantics Add __init__ docstring explaining that rate_limit_per_second creates a per-instance limiter scoped to the handler's lifecycle. Also widen test_rate_limit_can_be_disabled upper bound for CI reliability. * fix: add deprecation shim for get_global_limiter param and test override throttling - get_global_limiter() now accepts the old calls_per_second param with a DeprecationWarning (previously would TypeError) - Add test proving per-instance override limiter throttles repeated calls within the same handler (e.g., pagination) - Remove stale rate_limit_per_second reference from iter_all docstring * fix: warn when rate_limit_per_second is passed to iter_all The kwarg is only effective at construction time. Passing it to iter_all() was silently ignored. Now emits a UserWarning and strips the kwarg. Also adds a note to the README that the override is scoped to the handler instance. * docs: update CHANGELOG and CLAUDE.md for per-endpoint rate limiting Add 2.0.1 changelog entry documenting new rate limiting features, behavioral changes, and deprecations. Update CLAUDE.md to reflect built-in rate limiting, correct Python version, and add rate_limiter and cache modules to the file structure. * fix: add class-level default for _override_limiter to prevent AttributeError on from_dict instances * Fix gitignore and changelog
* Re-raise error from urllib as ScryfallError and fix tests * Use .get for optional fields from scryfall data
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.
This pull request introduces robust, automatic per-endpoint rate limiting to the Scrython library, aligning with Scryfall's documented API limits. It also improves error handling, updates documentation, and cleans up some internal implementation details. The most important changes are grouped below:
Rate Limiting Improvements:
Search,Named,Random, andCollectionnow enforce a stricter 2 requests/second limit, while all others default to 10 requests/second. Each endpoint category uses its own global rate limiter instance, ensuring independent throttling. (scrython/cards/cards.py,scrython/rate_limiter.py,README.md,CHANGELOG.md,CLAUDE.md) [1] [2] [3] [4] [5] [6] [7] [8] [9]SlowRateLimiterfor endpoints requiring stricter limits and added the_rate_limiter_classmechanism to endpoint classes for tier assignment. (scrython/cards/cards.py,scrython/rate_limiter.py,CLAUDE.md) [1] [2] [3] [4] [5]rate_limit_per_secondkwarg now creates a per-instance limiter, scoped to the handler's lifecycle. Passing this kwarg toiter_all()emits a warning and is ignored; it must be set at construction time. (scrython/base.py,scrython/base_mixins.py,README.md,CHANGELOG.md) [1] [2] [3] [4] [5] [6] [7]README.md,CLAUDE.md,CHANGELOG.md) [1] [2] [3] [4] [5]Error Handling and Internal Improvements:
ScryfallErrorwith detailed information. (scrython/base.py)scrython/base.py,gen_docs.py,scripts/update_version_for_testpypi.py) [1] [2] [3] [4]Workflow and Miscellaneous:
.github/workflows/claude-code-review.ymlworkflow file.These changes ensure Scrython is compliant with Scryfall's API usage policies by default and provide users with more granular control over request throttling.