Stop the ENCODE sync from aborting partway on a 600-second fetch timeout — Closes #105

[conradbzura]

Summary

Bound the ENCODE metadata download by a configurable budget instead of a 600-second literal, and report how far a load got when the budget expires.

The fetch streams a hundreds-of-megabyte TSV and transforms and inserts each row as it arrives, so the wall clock tracks insert throughput against the target database rather than network latency. aiohttp.ClientTimeout(total=...) covers that entire operation, not inactivity, so ten minutes was a cap on corpus size dressed as a network timeout. On dev the sync reached 230,000 of roughly 810,000 rows and aborted; a local run against containerized MongoDB completed all 809,981, because local inserts finish inside the budget. Dev's earlier count of 242,000 is consistent with the same timeout firing at a different point rather than with any deliberate cap.

The failure does not self-correct. _sync_encode clears the DCC before reloading, so an aborted run leaves fewer documents than it started with — dev is currently at 230,000, down from 242,000.

Closes #105

Proposed changes

Resolve the budget per call

Read ENCODE_METADATA_TIMEOUT_SECONDS, defaulting to one hour, inside fetch_encode_metadata rather than at import. A malformed value then fails the sync that reads it instead of the import of cfdb.services.encode, which would take down every read the API serves over a knob only the sync touches. It also makes the override observable through the public API, so the tests exercise env → parse → request as one chain rather than poking the parser directly.

Reject a non-positive value rather than passing it through: aiohttp treats a non-positive total as no timeout, so a typo reading 0 would silently convert a bounded request into an unbounded one — failing in the direction hardest to notice.

A total budget still scales with corpus size and insert speed. sock_read would bound inactivity instead and not need revisiting as ENCODE grows; src/cfdb/services/drs.py:367 already uses that shape for a streaming download. One hour was the deliberate choice here.

Report how far a timeout got

Catch asyncio.TimeoutError explicitly, log the row count reached and the budget, and re-raise unchanged so the caller's handling is unaffected. The previous handler caught only aiohttp.ClientError, so a timeout surfaced as a bare traceback — and the row count is the one fact separating "the budget is too small" from "the endpoint is down".

Test cases

fetch_encode_metadata had no tests. Its parse loop, header handling, non-200 branch and network-error handler were reachable only through the sync, which replaces the function wholesale with a generator over already-parsed rows. Nothing in the suite modelled a streamed body either, so the stubs are new: the response body is an async iterator of byte lines, and the session records request kwargs, which the sibling 4DN fakes discard.

#	Test Suite	Given	When	Then	Coverage Target
1	tests/test_encode.py	No timeout override	The fetch drains a response	The request is bounded by at least an hour	The regression guard for the 600-second literal
2	tests/test_encode.py	The timeout variable set to two hours	The fetch drains a response	The request is bounded by that value	Env to parse to request as one chain
3	tests/test_encode.py	No timeout variable in the environment	The fetch drains a response	The request is bounded by the one-hour default	The fallback path
4	tests/test_encode.py	The variable set to a non-integer, zero, or a negative	The fetch is drained	It raises naming the variable and issues no request	A non-positive total means no timeout in aiohttp
5	tests/test_encode.py	A header plus two data lines	The fetch drains them	One dict per data line, keyed by column	The parse loop, and that the stub is faithful
6	tests/test_encode.py	A body carrying only a header	The fetch drains it	No rows and no error	An empty corpus is distinguishable from a broken response
7	tests/test_encode.py	Blank lines between and after data lines	The fetch drains them	Only populated rows are yielded	A real TSV ends with a trailing newline
8	tests/test_encode.py	Lines ending with CRLF	The fetch drains them	Values carry no carriage return	A stray carriage return would corrupt every row's last column
9	tests/test_encode.py	A stream that times out after two rows	The fetch drains it	It logs the count reached and re-raises unchanged	The new handler
10	tests/test_encode.py	A metadata endpoint returning HTTP 503	The fetch is drained	It raises naming the status	The previously untested non-200 branch
11	tests/test_encode.py	A request raising a client error	The fetch is drained	It raises naming the network error	The previously untested handler

Suite: 971 passing. Both guards mutation-checked — restoring total=600 fails three tests, ignoring the env var fails four.

Not addressed. The issue's expected behavior includes failing "without having discarded the previous data", which needs a staging-collection cutover rather than a timeout change. This reduces how often that bites without removing it.