diff --git a/changelog.d/obr-efo-fallback.md b/changelog.d/obr-efo-fallback.md new file mode 100644 index 00000000..200a3aa5 --- /dev/null +++ b/changelog.d/obr-efo-fallback.md @@ -0,0 +1 @@ +- Commit the OBR EFO receipts and expenditure workbooks and fall back to them when the download fails. obr.uk serves 403 Forbidden to GitHub Actions runner IPs, and the swallowed error silently dropped 28 OBR targets, calibrating a degraded dataset that failed the reform-impact and target-registry tests (blocking the data upload). Permanent HTTP errors no longer burn the retry budget, and the committed workbooks — following the committed-not-fetched precedent of the HMRC size-of-gain table and ONS demographics — guarantee the full OBR target set builds offline. Update them together with the sources.yaml URLs on each EFO vintage change. diff --git a/policyengine_uk_data/storage/obr_efo/efo_expenditure.xlsx b/policyengine_uk_data/storage/obr_efo/efo_expenditure.xlsx new file mode 100644 index 00000000..8e6f4717 Binary files /dev/null and b/policyengine_uk_data/storage/obr_efo/efo_expenditure.xlsx differ diff --git a/policyengine_uk_data/storage/obr_efo/efo_receipts.xlsx b/policyengine_uk_data/storage/obr_efo/efo_receipts.xlsx new file mode 100644 index 00000000..218f6165 Binary files /dev/null and b/policyengine_uk_data/storage/obr_efo/efo_receipts.xlsx differ diff --git a/policyengine_uk_data/targets/sources/obr.py b/policyengine_uk_data/targets/sources/obr.py index 7baf3be5..ad4e3cf1 100644 --- a/policyengine_uk_data/targets/sources/obr.py +++ b/policyengine_uk_data/targets/sources/obr.py @@ -45,12 +45,40 @@ _DOWNLOAD_RETRY_STATUSES = {429, 500, 502, 503, 504} +# obr.uk serves 403 Forbidden to GitHub Actions runner IPs (observed on the +# 2026-07-21 push builds: every attempt 403'd while the same URLs work from +# residential connections). A 403 is not retryable, and losing the workbook +# silently drops 28 OBR targets and calibrates a degraded dataset. The EFO +# workbooks for the vintage pinned in sources.yaml are therefore committed +# under storage/obr_efo/ — following the committed-not-fetched precedent of +# capital_gains_size_distribution_hmrc.csv and ons_demographics.py — and +# used as a fallback whenever the download fails. Update them together with +# the sources.yaml URLs when a new EFO vintage is adopted. +_EFO_FALLBACKS = { + "receipts": "efo_receipts.xlsx", + "expenditure": "efo_expenditure.xlsx", +} + + +def _fallback_workbook(url: str) -> openpyxl.Workbook | None: + from policyengine_uk_data.storage import STORAGE_FOLDER + + for slug, filename in _EFO_FALLBACKS.items(): + if slug in url: + path = STORAGE_FOLDER / "obr_efo" / filename + if path.exists(): + return openpyxl.load_workbook(path, data_only=False) + return None + + @lru_cache(maxsize=2) def _download_workbook(url: str) -> openpyxl.Workbook: """Download an xlsx from OBR and return an openpyxl workbook. Retries transient HTTP errors (429/5xx) and connection failures with - exponential backoff, honouring a numeric Retry-After header when present. + exponential backoff, honouring a numeric Retry-After header when + present. Falls back to the committed workbook in storage/obr_efo/ when + the download ultimately fails (obr.uk 403s CI runner IPs). """ last_error: Exception | None = None for attempt in range(_DOWNLOAD_MAX_ATTEMPTS): @@ -60,12 +88,13 @@ def _download_workbook(url: str) -> openpyxl.Workbook: except requests.RequestException as e: last_error = e # connection/timeout — retryable else: - if r.status_code not in _DOWNLOAD_RETRY_STATUSES: - r.raise_for_status() + if r.status_code < 400: return openpyxl.load_workbook(io.BytesIO(r.content), data_only=False) last_error = requests.HTTPError( f"{r.status_code} for url: {url}", response=r ) + if r.status_code not in _DOWNLOAD_RETRY_STATUSES: + break # 403 and other permanent errors: don't burn retries retry_after = r.headers.get("Retry-After", "") if retry_after.isdigit(): wait = int(retry_after) @@ -77,6 +106,14 @@ def _download_workbook(url: str) -> openpyxl.Workbook: wait, ) time.sleep(wait) + fallback = _fallback_workbook(url) + if fallback is not None: + logger.warning( + "OBR download %s failed (%s); using committed workbook fallback", + url, + last_error, + ) + return fallback raise last_error diff --git a/policyengine_uk_data/tests/test_obr_efo_fallback.py b/policyengine_uk_data/tests/test_obr_efo_fallback.py new file mode 100644 index 00000000..309e8d7c --- /dev/null +++ b/policyengine_uk_data/tests/test_obr_efo_fallback.py @@ -0,0 +1,96 @@ +"""Tests for the committed EFO workbook fallback. + +obr.uk serves 403 Forbidden to GitHub Actions runner IPs, and losing the +workbooks silently drops 28 OBR targets and calibrates a degraded dataset +(observed on the 2026-07-21 push builds). These tests pin: the fallback +workbooks are committed and parseable, a failed download uses them instead +of raising, and a 403 does not burn the retry budget. +""" + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest +import requests + +from policyengine_uk_data.storage import STORAGE_FOLDER +from policyengine_uk_data.targets.sources import obr + + +@pytest.fixture(autouse=True) +def _clear_workbook_cache(): + obr._download_workbook.cache_clear() + yield + obr._download_workbook.cache_clear() + + +def _forbidden(*args, **kwargs): + return SimpleNamespace(status_code=403, headers={}, content=b"") + + +def test_fallback_workbooks_are_committed_and_parseable(): + for filename in obr._EFO_FALLBACKS.values(): + path = STORAGE_FOLDER / "obr_efo" / filename + assert path.exists(), f"{filename} missing from storage/obr_efo" + receipts = obr._fallback_workbook("https://obr.uk/x/efo-receipts/") + assert receipts is not None + # The receipts sheet lookup must work on the committed vintage. + assert obr._find_receipts_sheet(receipts) is not None + + +def test_403_uses_fallback_without_retrying(): + calls = [] + + def get(*args, **kwargs): + calls.append(args) + return _forbidden() + + with patch.object(obr.requests, "get", side_effect=get): + wb = obr._download_workbook( + "https://obr.uk/download/whatever-forecast-tables-receipts/" + ) + assert wb is not None + assert len(calls) == 1, "403 is permanent; it should not be retried" + + +def test_connection_failure_uses_fallback(): + def get(*args, **kwargs): + raise requests.ConnectionError("no route to obr.uk") + + with ( + patch.object(obr.requests, "get", side_effect=get), + patch.object(obr.time, "sleep", lambda s: None), + ): + wb = obr._download_workbook( + "https://obr.uk/download/whatever-forecast-tables-expenditure/" + ) + assert wb is not None + + +def test_unknown_url_with_failed_download_still_raises(): + with patch.object(obr.requests, "get", side_effect=_forbidden): + with pytest.raises(requests.HTTPError): + obr._download_workbook("https://obr.uk/download/some-other-file/") + + +def test_full_target_set_available_offline(): + """All 34 OBR targets must build from the committed workbooks alone.""" + + def get(*args, **kwargs): + raise requests.ConnectionError("offline") + + with ( + patch.object(obr.requests, "get", side_effect=get), + patch.object(obr.time, "sleep", lambda s: None), + ): + targets = obr.get_targets() + names = {t.name for t in targets} + assert { + "obr/income_tax", + "obr/ni_employee", + "obr/ni_employer", + "obr/ni_self_employed", + "obr/capital_gains_tax", + "obr/vat", + } <= names + assert len(names) >= 30