fix(prices): batch get_multiple — one request per 20 codes, not per code (api#7240) - #97
Merged
Merged
Conversation
…ode (api#7240) get_multiple() looped over get(), issuing one HTTP request per commodity. The REST API accepts up to 20 codes in ONE request that counts ONCE against quota, so the method whose whole purpose is fetching several prices cost up to 20x more quota than writing the call by hand. Through this method the free plan was 50 code-reads a day. Through the raw API it is 1,000. A plausible contributor to a measured result: sdk-python converts at 2.2% while people using plain Python and ignoring the SDK convert at 4.2% (api#7214). Our SDK made the free plan twenty times harder to live inside than not using it. Verified against production before writing the fix (2026-08-23): 20 codes -> 200; 21 -> 400 "Too many commodity codes requested (max: 20)" one multi-code call writes ONE api_requests row 1 code -> flat data object; 2+ -> data.prices[] Both clients fixed. The async one was worse: asyncio.gather fanned out one request PER CODE concurrently, which also trips the 60-per-60s rate limit on a long list. It now gathers CHUNKS, so 25 codes are 2 concurrent requests. Failure contract preserved. The API rejects the whole request when any code in it is unknown, so a chunk failure does not say which code was at fault — that chunk (and only that chunk) is retried per code to rebuild the per-code failure list. A one-code chunk has nothing to narrow down and is recorded directly rather than refetched. Live smoke against production: 10 codes -> 1 request, 10 prices (was 10 requests) 1 code -> 1 request, 1 price 30 codes -> 2 requests, 29 prices (was 30 requests) bad code -> 3 requests, 1 ok + 1 reported failure Four new tests assert REQUEST COUNT, not just results — the regression is invisible without that. Two existing tests asserted call_count == 2 for two codes, encoding the defect; they now assert 1 and carry a note saying why. Repo-wide ruff/black findings are pre-existing and unchanged (prices.py and async_client.py: 0 before, 0 after). Closes api#7240 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JKAExynd9zoKwt6rYA66EA
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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 |
CI's mypy step failed on the new helper with 8 errors across both clients: 'Missing type parameters for generic type dict' and incompatible argument types for commodity/value/timestamp. The errors are real but not new behaviour — the previous Price(**mapped_data) form passed a dict of kwargs, which mypy cannot check, so the same looseness was always there and simply invisible. Writing the call out explicitly exposed it. Typed the parameter as Dict[str, Any] and cast the three fields pydantic validates anyway, with a comment saying why the casts are there. Not silenced with type: ignore — the shape is genuinely Any coming off JSON, and pydantic raises on anything actually wrong. mypy on both files: 0 errors. Full suite still 645 passed. Live smoke re-run after the change: 5 codes -> 1 request, 5 prices, BRENT_CRUDE_USD $93.60 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JKAExynd9zoKwt6rYA66EA
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.
Closes #7240 (filed as api#7240).
The defect
get_multiple()looped overget(), issuing one HTTP request per commodity. The REST API accepts up to 20 codes in one request that counts once against quota.So the method whose entire purpose is fetching several prices cost up to 20× more quota than writing the call by hand. Through it, the free plan was 50 code-reads a day; through the raw API it is 1,000.
That is a plausible contributor to a measured result:
sdk-pythonconverts at 2.2% while people using plain Python and ignoring our SDK convert at 4.2% (api#7214). Our SDK made the free plan twenty times harder to live inside than not using it.Verified against production before writing the fix
And one multi-code call writes one
api_requestsrow — confirmed by making a marked 7-code call and finding a single row for it. Response shape: 1 code → flatdata; 2+ →data.prices[].Both clients
The async one was worse:
asyncio.gatherfanned out one request per code concurrently, which also trips the 60-per-60s rate limit on a long list. It now gathers chunks — 25 codes are 2 concurrent requests, not 25.Failure contract preserved
The API rejects the whole request when any code in it is unknown, so a chunk failure does not say which code was at fault. That chunk — and only that chunk — is retried per code to rebuild the per-code failure list. A one-code chunk has nothing to narrow down and is recorded directly rather than refetched.
Live smoke against production
Tests
Four new tests assert request count, not just results — this regression is invisible without that. And two existing tests asserted
call_count == 2for two codes, encoding the defect; they now assert1and carry a note explaining why they changed.Repo-wide ruff/black findings are pre-existing and unchanged —
prices.pyandasync_client.pyare 0 before, 0 after.Behaviour change worth noting in release notes
Existing users' quota consumption drops — in their favour, but it is a change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JKAExynd9zoKwt6rYA66EA