From 7cd2397ac8c90ae06b4c7bc5f0ee6e916ad1c4bd Mon Sep 17 00:00:00 2001 From: Thomas Pinder Date: Sat, 21 Feb 2026 15:40:15 +0100 Subject: [PATCH 1/5] fix: remove dead dutch_name/display_name from Column model The CBS API returns a single Title per column. dutch_name was always set to the same value as name, making display_name fallback dead code. Co-Authored-By: Claude Opus 4.6 --- src/cbspy/client.py | 1 - src/cbspy/models.py | 9 +-------- tests/test_models.py | 20 ++------------------ 3 files changed, 3 insertions(+), 27 deletions(-) diff --git a/src/cbspy/client.py b/src/cbspy/client.py index 347e7e0..40e2d71 100644 --- a/src/cbspy/client.py +++ b/src/cbspy/client.py @@ -118,7 +118,6 @@ def _parse_column(prop: dict[str, Any]) -> Column: return Column( id=prop.get("Key", ""), name=prop.get("Title", ""), - dutch_name=prop.get("Title", ""), unit=prop.get("Unit", ""), datatype=prop.get("Datatype", prop.get("Type", "")), description=prop.get("Description", ""), diff --git a/src/cbspy/models.py b/src/cbspy/models.py index 51f3935..2fa56b2 100644 --- a/src/cbspy/models.py +++ b/src/cbspy/models.py @@ -1,4 +1,4 @@ -from pydantic import BaseModel, computed_field +from pydantic import BaseModel class Column(BaseModel): @@ -6,17 +6,10 @@ class Column(BaseModel): id: str name: str - dutch_name: str unit: str datatype: str description: str - @computed_field - @property - def display_name(self) -> str: - """Return English name if available, otherwise Dutch.""" - return self.name if self.name else self.dutch_name - class TableMetadata(BaseModel): """Metadata for a CBS dataset table.""" diff --git a/tests/test_models.py b/tests/test_models.py index d77b47b..80a8c39 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -5,41 +5,26 @@ def test_column_creation(): col = Column( id="TotalPopulation_1", name="Total population", - dutch_name="Totale bevolking", unit="number", datatype="Double", description="The total population.", ) assert col.id == "TotalPopulation_1" assert col.name == "Total population" - assert col.dutch_name == "Totale bevolking" assert col.unit == "number" assert col.datatype == "Double" assert col.description == "The total population." -def test_column_name_falls_back_to_dutch(): +def test_column_empty_name(): col = Column( id="Foo_1", name="", - dutch_name="Nederlandse naam", unit="", datatype="Long", description="", ) - assert col.display_name == "Nederlandse naam" - - -def test_column_name_prefers_english(): - col = Column( - id="Foo_1", - name="English name", - dutch_name="Nederlandse naam", - unit="", - datatype="Long", - description="", - ) - assert col.display_name == "English name" + assert col.name == "" def test_table_metadata_creation(): @@ -60,7 +45,6 @@ def test_table_metadata_with_columns(): col = Column( id="TotalPopulation_1", name="Total population", - dutch_name="Totale bevolking", unit="number", datatype="Double", description="", From bdb6737b9632074fff987eef366065d272b58a04 Mon Sep 17 00:00:00 2001 From: Thomas Pinder Date: Sat, 21 Feb 2026 15:47:55 +0100 Subject: [PATCH 2/5] fix: make Client a context manager to prevent connection leaks Client now tracks whether it owns the httpx.Client. close() and __exit__ only close the HTTP client if Client created it internally. Co-Authored-By: Claude Opus 4.6 --- src/cbspy/client.py | 12 ++++++++++++ tests/test_client.py | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/cbspy/client.py b/src/cbspy/client.py index 40e2d71..234ae53 100644 --- a/src/cbspy/client.py +++ b/src/cbspy/client.py @@ -25,9 +25,21 @@ def __init__( base_url: str = _DEFAULT_BASE_URL, http_client: httpx.Client | None = None, ) -> None: + self._owns_http = http_client is None self._http = http_client or httpx.Client() self._odata = ODataClient(base_url=base_url, http_client=self._http) + def close(self) -> None: + """Close the underlying HTTP client if this instance owns it.""" + if self._owns_http: + self._http.close() + + def __enter__(self) -> Client: + return self + + def __exit__(self, *args: object) -> None: + self.close() + def list_tables(self, language: str | None = None) -> pl.DataFrame: """List available CBS tables. diff --git a/tests/test_client.py b/tests/test_client.py index cce536d..c315bf2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -269,3 +269,30 @@ def test_get_data_empty_dataset(self): df = client.get_data("37296eng") assert isinstance(df, pl.DataFrame) assert df.shape[0] == 0 + + +class TestClientLifecycle: + def test_close_closes_owned_http_client(self): + client = Client() + assert not client._http.is_closed + client.close() + assert client._http.is_closed + + def test_close_does_not_close_external_http_client(self): + http = httpx.Client() + client = Client(http_client=http) + client.close() + assert not http.is_closed + http.close() + + def test_context_manager(self): + with Client() as client: + assert not client._http.is_closed + assert client._http.is_closed + + def test_context_manager_with_external_client(self): + http = httpx.Client() + with Client(http_client=http) as client: + assert not client._http.is_closed + assert not http.is_closed + http.close() From c56f32bf6de4464aa4bbffe0fa761b671e3cc3bb Mon Sep 17 00:00:00 2001 From: Thomas Pinder Date: Sat, 21 Feb 2026 15:50:02 +0100 Subject: [PATCH 3/5] fix: remove broken Dockerfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Template leftover referencing non-existent cbspy/foo.py. This is a library — no application entrypoint to containerise. Co-Authored-By: Claude Opus 4.6 --- Dockerfile | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 Dockerfile diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index ce53277..0000000 --- a/Dockerfile +++ /dev/null @@ -1,21 +0,0 @@ -# Install uv -FROM python:3.12-slim -COPY --from=ghcr.io/astral-sh/uv:latest /uv /bin/uv - -# Change the working directory to the `app` directory -WORKDIR /app - -# Copy the lockfile and `pyproject.toml` into the image -COPY uv.lock /app/uv.lock -COPY pyproject.toml /app/pyproject.toml - -# Install dependencies -RUN uv sync --frozen --no-install-project - -# Copy the project into the image -COPY . /app - -# Sync the project -RUN uv sync --frozen - -CMD [ "python", "cbspy/foo.py" ] From 5577bc7a803490252e076d6c9fbe264f406a5f20 Mon Sep 17 00:00:00 2001 From: Thomas Pinder Date: Sat, 21 Feb 2026 16:03:54 +0100 Subject: [PATCH 4/5] fix: add retry logic for transient 5xx and network errors ODataClient now retries once with a 1s delay on 5xx status codes and httpx.RequestError (timeouts, connection errors). 404 and 4xx errors are not retried. Also removes unused http_client fixture from conftest. Co-Authored-By: Claude Opus 4.6 --- src/cbspy/_odata.py | 45 ++++++++++++++++++++++-------- tests/conftest.py | 8 ------ tests/test_odata.py | 67 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 19 deletions(-) diff --git a/src/cbspy/_odata.py b/src/cbspy/_odata.py index 5569f4f..701fa67 100644 --- a/src/cbspy/_odata.py +++ b/src/cbspy/_odata.py @@ -1,5 +1,6 @@ from __future__ import annotations +import time from typing import Any import httpx @@ -9,6 +10,9 @@ _ODATA_API = "/ODataApi/odata" _CATALOG = "/ODataCatalog/Tables" +_MAX_RETRIES = 1 +_RETRY_DELAY = 1.0 + class ODataClient: """Low-level OData HTTP client for CBS Statline.""" @@ -27,8 +31,7 @@ def get_json(self, table_id: str, resource: str, params: dict[str, str] | None = all_rows: list[dict[str, Any]] = [] while url is not None: - response = self._http.get(url, params=request_params) - self._check_response(response, table_id) + response = self._request_with_retry(url, request_params, table_id) body = response.json() all_rows.extend(body.get("value", [])) @@ -50,8 +53,7 @@ def get_catalog(self, language: str | None = None) -> list[dict[str, Any]]: all_rows: list[dict[str, Any]] = [] while url is not None: - response = self._http.get(url, params=params) - self._check_response(response, "catalog") + response = self._request_with_retry(url, params, "catalog") body = response.json() all_rows.extend(body.get("value", [])) @@ -61,10 +63,31 @@ def get_catalog(self, language: str | None = None) -> list[dict[str, Any]]: return all_rows - def _check_response(self, response: httpx.Response, table_id: str) -> None: - """Raise appropriate exception for error responses.""" - if response.status_code == 404: - msg = f"Table '{table_id}' not found. Use client.list_tables() to discover available tables." - raise TableNotFoundError(msg) - if response.status_code >= 400: - raise APIError(status_code=response.status_code, message=response.text) + def _request_with_retry( + self, url: str, params: dict[str, str], table_id: str + ) -> httpx.Response: + """Make an HTTP GET with retry on transient errors.""" + for attempt in range(_MAX_RETRIES + 1): + try: + response = self._http.get(url, params=params) + except httpx.RequestError: + if attempt < _MAX_RETRIES: + time.sleep(_RETRY_DELAY) + continue + raise + + if response.status_code == 404: + msg = f"Table '{table_id}' not found. Use client.list_tables() to discover available tables." + raise TableNotFoundError(msg) + + if response.status_code >= 500 and attempt < _MAX_RETRIES: + time.sleep(_RETRY_DELAY) + continue + + if response.status_code >= 400: + raise APIError(status_code=response.status_code, message=response.text) + + return response + + # Should not reach here, but satisfy type checker + raise APIError(status_code=response.status_code, message=response.text) diff --git a/tests/conftest.py b/tests/conftest.py index 89d6f2f..e69de29 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +0,0 @@ -import httpx -import pytest - - -@pytest.fixture -def http_client(): - """A real httpx client for building OData instances in tests.""" - return httpx.Client() diff --git a/tests/test_odata.py b/tests/test_odata.py index 95f753c..8994755 100644 --- a/tests/test_odata.py +++ b/tests/test_odata.py @@ -82,3 +82,70 @@ def test_catalog_with_language_filter(self): result = client.get_catalog(language="en") assert len(result) == 1 assert result[0]["Language"] == "en" + + +class TestRetryBehavior: + def test_retries_once_on_500_then_succeeds(self): + attempt = 0 + + def handler(request): + nonlocal attempt + attempt += 1 + if attempt == 1: + return httpx.Response(500, text="Internal Server Error") + return httpx.Response(200, json={"value": [{"ID": 0}]}) + + transport = httpx.MockTransport(handler) + client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport)) + result = client.get_json("37296eng", "TypedDataSet") + assert result == [{"ID": 0}] + assert attempt == 2 + + def test_raises_after_retry_exhausted(self): + def handler(request): + return httpx.Response(503, text="Service Unavailable") + + transport = httpx.MockTransport(handler) + client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport)) + with pytest.raises(APIError) as exc_info: + client.get_json("37296eng", "TypedDataSet") + assert exc_info.value.status_code == 503 + + def test_no_retry_on_404(self): + attempt = 0 + + def handler(request): + nonlocal attempt + attempt += 1 + return httpx.Response(404, text="Not found") + + transport = httpx.MockTransport(handler) + client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport)) + with pytest.raises(TableNotFoundError): + client.get_json("FAKE", "TypedDataSet") + assert attempt == 1 + + def test_retries_on_network_error_then_succeeds(self): + attempt = 0 + + def handler(request): + nonlocal attempt + attempt += 1 + if attempt == 1: + raise httpx.ConnectError("Connection refused") # noqa: TRY003 + return httpx.Response(200, json={"value": [{"ID": 0}]}) + + transport = httpx.MockTransport(handler) + client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport)) + result = client.get_json("37296eng", "TypedDataSet") + assert result == [{"ID": 0}] + assert attempt == 2 + + def test_raises_network_error_after_retry_exhausted(self): + def handler(request): + raise httpx.ConnectError("Connection refused") # noqa: TRY003 + + transport = httpx.MockTransport(handler) + client = ODataClient(base_url=BASE, http_client=httpx.Client(transport=transport)) + with pytest.raises(httpx.ConnectError): + client.get_json("37296eng", "TypedDataSet") From 19b3c7b8de061a2bf3a972d57b83b673e24f9857 Mon Sep 17 00:00:00 2001 From: Thomas Pinder Date: Sat, 21 Feb 2026 16:13:40 +0100 Subject: [PATCH 5/5] style: fix ruff format on _request_with_retry signature Co-Authored-By: Claude Opus 4.6 --- src/cbspy/_odata.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cbspy/_odata.py b/src/cbspy/_odata.py index 701fa67..9f755c9 100644 --- a/src/cbspy/_odata.py +++ b/src/cbspy/_odata.py @@ -63,9 +63,7 @@ def get_catalog(self, language: str | None = None) -> list[dict[str, Any]]: return all_rows - def _request_with_retry( - self, url: str, params: dict[str, str], table_id: str - ) -> httpx.Response: + def _request_with_retry(self, url: str, params: dict[str, str], table_id: str) -> httpx.Response: """Make an HTTP GET with retry on transient errors.""" for attempt in range(_MAX_RETRIES + 1): try: