Repository navigation
fix: fetch every resource for the deleted entities expectation - #611
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughDataset resource retrieval now filters and orders query results. It fetches results in pages of up to 1,000 rows, combines the pages, and builds the organisation-to-resources cache. Integration tests cover empty active resources and retrieval of 2,500 resources. ChangesDataset resource retrieval
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Fetch as _fetch_resources_for_dataset
participant Reader as _read_datasette_csv
participant Pandas as pandas.read_csv
participant Datasette
loop Until a page has fewer than 1,000 rows
Fetch->>Reader: Query URL with page limit and offset
Reader->>Pandas: Read CSV from query URL
Pandas->>Datasette: Request CSV page
Datasette-->>Pandas: Return CSV page
Pandas-->>Reader: Return page data
Reader-->>Fetch: Return page data
end
Fetch->>Fetch: Combine pages and build cache
Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
What type of PR is this? (check all applicable)
Description
Fixes two very minor problems in how the
count_deleted_entitiesexpectation fetches each organisation's resources from Datasette which result in very slightly off expectation counts:reporting_historic_endpointsrows with no resource (log entries for days an endpoint returned nothing).order by, instead of making one request. The retry loop has moved into_read_datasette_csvso that each page is retried on its own.Why
Blank active resources. The active query keeps rows where
resource_end_dateis blank or null. Log rows with no resource have a nullresource_end_date, so they pass that filter.group by o.entity, rhe.endpointthen picks an arbitrary row per endpoint. When the pick is a blank row, the organisation's active resource is lost, and every entity it supplies is counted as deleted. In prod on 17 Oct this failed central-activities-zone for LBH (entity 2200001), although its resourceb7cd7941…is still active. In the same night's results, organisations with a blank in their own active list made up 9 failing rows in conservation-area, 3 in article-4-direction-area and 2 in tree-preservation-zone. Which row gets picked varies between runs, so the failures come and go.Truncated historic resources. Datasette returns at most 1,000 rows (
max_returned_rows) and gives no error when it cuts a result short. The dataset-wide historic query is over that for listed-building-outline (2,857 rows), conservation-area (2,451), tree-preservation-zone (2,821) and article-4-direction-area (3,321). Organisations whose rows fall past the cut get an empty historic list, so the check silently passes them with 0. Adding_stream=ondoesn't help: Datasette only streams table and view exports, not custom SQL.Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Ran the dataset checkpoint locally against live prod Datasette, using the prod conservation-area sqlite from 17 Oct 01:22Z, with
mainand with this branch:count_deleted_entitiesmainbecause their resources were past the 1,000-row cut. I traced NED: its own resource ended in December 2025 and its entities are missing from its active resources, so it fails under the expectation's existing definition.count_deleted_entitiesnotice tasks after merge, because organisations that were silently skipped now get checked. They are expectation tasks, so they stay off LPA dashboards.mainis installed at runtime by the collection tasks, so this goes live on the next nightly run with no publish step.Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above. Please refer to the Digital Land Testing Guidance for more information.
have not been included
[optional] Are there any post deployment tasks we need to perform?
[optional] Are there any dependencies on other PRs or Work?
Summary by CodeRabbit