downloader: add request timeout and per-year error isolation

requests.get() had no timeout at all - found for real in CI, where a
single unresponsive request to CBS's site stalled a run for 7+ minutes
with no way to recover. Adds a (connect, read) timeout, and makes
download_all() isolate failures per-year (logged and skipped) so one
bad/slow year doesn't abort the whole batch - maximizing how much data
is actually available afterward, consistent with the CI workflow's
"download or fall back to a viable checkpoint" design.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: akariv

Lamas/lamas/downloader.py

@@ -19,6 +19,12 @@
 P_LIBUD2 = {2021}              # -> p_libud_{4-digit year}.xlsx
 
 
+# (connect timeout, read timeout) in seconds - without this, a slow/unresponsive connection to
+# CBS's site can hang a download indefinitely (found for real: a CI run stalled 7+ minutes on a
+# single request with no timeout set at all).
+REQUEST_TIMEOUT = (10, 60)
+
+
 def download_excel(year, downloads_dir=DOWNLOADS_DIR):
     out_filename = f'{year}' + ('.xlsx' if year >= XLSX_YEAR else '.xls')
     if year in P_LIBUD:
@@ -31,7 +37,7 @@ def download_excel(year, downloads_dir=DOWNLOADS_DIR):
     out_filename = f'{downloads_dir}/lamas-muni-{out_filename}'
     if not os.path.exists(out_filename):
         logger.info('Downloading %s -> %s', url, out_filename)
-        r = requests.get(url, stream=True)
+        r = requests.get(url, stream=True, timeout=REQUEST_TIMEOUT)
         assert r.status_code == 200, f'Failed to download {url}'
         with open(out_filename, 'wb') as f:
             shutil.copyfileobj(r.raw, f)
@@ -40,8 +46,14 @@ def download_excel(year, downloads_dir=DOWNLOADS_DIR):
 
 
 def download_all(downloads_dir=DOWNLOADS_DIR, min_year=MIN_YEAR, max_year=MAX_YEAR):
+    """Downloads every configured year, isolated per-year: one year timing out or failing
+    (network hiccup, a URL pattern change) is logged and skipped rather than aborting the whole
+    batch, so as many years as possible are still available afterward."""
     os.makedirs(downloads_dir, exist_ok=True)
-    return dict(
-        (year, download_excel(year, downloads_dir))
-        for year in range(min_year, max_year + 1)
-    )
+    filenames = {}
+    for year in range(min_year, max_year + 1):
+        try:
+            filenames[year] = download_excel(year, downloads_dir)
+        except Exception as e:
+            logger.error('Failed to download year %s: %s', year, e)
+    return filenames

Lamas/tests/test_downloader.py

@@ -1,6 +1,6 @@
 import io
 
-from lamas.downloader import download_excel
+from lamas.downloader import REQUEST_TIMEOUT, download_all, download_excel
 
 
 class FakeResponse:
@@ -9,14 +9,26 @@ def __init__(self):
         self.raw = io.BytesIO(b'fake-content')
 
 
-def _patch_get(monkeypatch, calls):
-    def fake_get(url, stream=True):
+def _patch_get(monkeypatch, calls, kwargs_seen=None):
+    def fake_get(url, stream=True, timeout=None):
         calls.append(url)
+        if kwargs_seen is not None:
+            kwargs_seen.append({'stream': stream, 'timeout': timeout})
         return FakeResponse()
     monkeypatch.setattr('lamas.downloader.requests.get', fake_get)
     monkeypatch.setattr('lamas.downloader.shutil.copyfileobj', lambda *a, **k: None)
 
 
+def test_download_passes_a_timeout(monkeypatch, tmp_path):
+    # A hung/unresponsive connection to CBS's site must not block forever - found for real in
+    # CI, where a request with no timeout stalled a run for 7+ minutes.
+    calls, kwargs_seen = [], []
+    _patch_get(monkeypatch, calls, kwargs_seen)
+    download_excel(2020, str(tmp_path))
+    assert kwargs_seen[0]['timeout'] == REQUEST_TIMEOUT
+    assert kwargs_seen[0]['timeout'] is not None
+
+
 def test_p_libud_years_use_two_digit_pattern(monkeypatch, tmp_path):
     calls = []
     _patch_get(monkeypatch, calls)
@@ -57,3 +69,18 @@ def test_skips_download_if_file_already_exists(monkeypatch, tmp_path):
     result = download_excel(2020, str(tmp_path))
     assert calls == []
     assert result == str(out)
+
+
+def test_download_all_isolates_a_failing_year(monkeypatch, tmp_path):
+    # One year timing out or erroring (network hiccup, a URL pattern change) must not abort the
+    # whole batch - every other year should still download successfully.
+    def flaky_get(url, stream=True, timeout=None):
+        if '2021' in url:
+            raise TimeoutError('simulated hang')
+        return FakeResponse()
+    monkeypatch.setattr('lamas.downloader.requests.get', flaky_get)
+    monkeypatch.setattr('lamas.downloader.shutil.copyfileobj', lambda *a, **k: None)
+
+    filenames = download_all(str(tmp_path), min_year=2020, max_year=2022)
+    assert set(filenames.keys()) == {2020, 2022}
+    assert 2021 not in filenames