Review follow-up on the dedupe.
Cancelling releases the cache_key, so a replacement fetch for that key can
start immediately. But _fetch_data_worker() had no cancellation check: the
cancelled worker still wrote its response to the cache, flipped its own
status from CANCELLED to COMPLETED, and ran its callbacks. The stale
response could therefore land on top of the replacement's fresher data.
The worker cannot abort an HTTP call in flight, so the response is discarded
on return instead: no cache write, no callbacks, status left CANCELLED. The
check sits immediately before the cache write, which is the first
side effect.
Also fixed, found by the new test rather than by reading:
request_id was f"{sport}_{year}_{milliseconds}", which is not unique.
Two submits inside the same millisecond produced the SAME id -- the
test's two sequential fetches collided on a fast mocked response, and
one request silently replaced the other in active_requests and
completed_requests. Rare before this PR; load-bearing now, because
dedupe hands that id back to every joiner as their handle for
get_result(). A per-service counter is appended.
Two test problems of my own, both fixed here rather than left to flake:
- The cancellation test synchronised with time.sleep(0.4). A slow worker
would have made it pass for the wrong reason. It now waits for the
request to be filed in completed_requests.
- The id-uniqueness test patched session.get, but submits are async: the
50 workers outlived the patch and made real DNS calls to the dummy host.
It stubs the executor instead, which is what a submit-time test should
exercise.
20 consecutive runs of the dedupe file: 0 failures. Full suite: 3700 passed,
60 skipped, 1 failure that reproduces identically on unmodified main
(test_install_lowmem, environment-dependent).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
