Context
PR #332 (#330) fixed a Copilot-flagged thread-safety concern in the ENA validator by giving each ThreadPoolExecutor worker its own requests.Session via threading.local() (scripts/validate_ena_accessions.py, _get_session()). The /simplify altitude review noted this leaves the repo's only pooled-session code in a one-off validation script, while the code doing the bulk of the repo's HTTP under threads has no session reuse at all:
src/meta_disco/fetchers.py — fetch_content_length uses bare requests.head, _fetch_range uses bare requests.get; driven concurrently by pipeline._run_parallel (ThreadPoolExecutor)
scripts/classify_hprc_files.py — runs fetch_content_length across a thread pool
scripts/validate_1000g_samples.py — bare requests.get across a thread pool
Every call in those paths constructs a throwaway session and pays a fresh TLS handshake — exactly the cost the ENA validator now avoids. The repo also has no retry/backoff policy anywhere (no Retry/max_retries usage).
Proposal
Add src/meta_disco/http.py holding the thread-local session factory (move _get_session() there), and use it from:
fetchers.py (requests.head/requests.get → get_session().head/.get)
scripts/validate_ena_accessions.py (replace the local helper)
scripts/classify_hprc_files.py and scripts/validate_1000g_samples.py
This gives the production fetch path keep-alive under _run_parallel, puts one owner on "how this repo does HTTP from a thread pool", and is the natural place to hang a shared Retry/backoff policy later (that can be a separate decision — the helper is useful without it).
Notes
- Per-thread sessions keep the thread-safety property
requests.Session doesn't guarantee for concurrent use; keep-alive is per worker, so handshake count = worker count.
- Behavior change to watch: session reuse in
fetchers.py touches the classification hot path, so verify golden/e2e outputs are unchanged.
Origin: /simplify altitude review on PR #332.
Context
PR #332 (#330) fixed a Copilot-flagged thread-safety concern in the ENA validator by giving each
ThreadPoolExecutorworker its ownrequests.Sessionviathreading.local()(scripts/validate_ena_accessions.py,_get_session()). The /simplify altitude review noted this leaves the repo's only pooled-session code in a one-off validation script, while the code doing the bulk of the repo's HTTP under threads has no session reuse at all:src/meta_disco/fetchers.py—fetch_content_lengthuses barerequests.head,_fetch_rangeuses barerequests.get; driven concurrently bypipeline._run_parallel(ThreadPoolExecutor)scripts/classify_hprc_files.py— runsfetch_content_lengthacross a thread poolscripts/validate_1000g_samples.py— barerequests.getacross a thread poolEvery call in those paths constructs a throwaway session and pays a fresh TLS handshake — exactly the cost the ENA validator now avoids. The repo also has no retry/backoff policy anywhere (no
Retry/max_retriesusage).Proposal
Add
src/meta_disco/http.pyholding the thread-local session factory (move_get_session()there), and use it from:fetchers.py(requests.head/requests.get→get_session().head/.get)scripts/validate_ena_accessions.py(replace the local helper)scripts/classify_hprc_files.pyandscripts/validate_1000g_samples.pyThis gives the production fetch path keep-alive under
_run_parallel, puts one owner on "how this repo does HTTP from a thread pool", and is the natural place to hang a sharedRetry/backoff policy later (that can be a separate decision — the helper is useful without it).Notes
requests.Sessiondoesn't guarantee for concurrent use; keep-alive is per worker, so handshake count = worker count.fetchers.pytouches the classification hot path, so verify golden/e2e outputs are unchanged.Origin: /simplify altitude review on PR #332.