Repository navigation
Conversation
Mounts read-only `/api/3/action/datastore_{search,search_sql,info}`
(GET + POST) and `/datastore/dump/{resource_id}` so scripts targeting
the public CKAN datastore keep working byte-for-byte while callers
migrate to v2.
- New `datastore/legacy/` package — one `rm -rf` away when the legacy
URLs retire; nothing in v2 imports from it. Removal plan documented
in `datastore/legacy/__init__.py`.
- CKAN parity: logical errors (ValidationError / NotAuthorized /
ObjectNotFound) return HTTP 200 with `success:false` via a custom
`_CKANCompatRoute(APIRoute)` on the actions router. Per-field error
details land at the `error` root (`error["sql"]`) matching
api.neso.energy's envelope, not nested under `error.fields`.
- Engine-managed `_id` / `_updated_at` columns stripped from both
`fields` and `records` on every legacy response. Legacy
`datastore_search_sql` accepts SQL without a LIMIT clause (CKAN
behaviour); v2 requires one.
- Dump supports CSV (default), TSV, JSON array, XML plus `bom=true`
for Excel. Full `datastore_search` filter param set (`filters`, `q`,
`distinct`, `plain`, `language`, `limit`, `offset`, `fields`, `sort`)
passed through to the engine. All formats stream API-side so row-
level filtering + system-column stripping work uniformly.
- Hidden from the v2 OpenAPI document (`include_in_schema=False`) so
Swagger users see only v2.
- Startup banner logs the deprecated routes once per process.
- Analytics middleware extended to record legacy traffic under the
same action names as v2; the `endpoint` field preserves the raw
path so downstream can split v2 vs legacy when needed.
- 33 new tests covering shape, GET/POST parity, status-200-on-error,
per-format dump output, BOM, filter passthrough.
Legacy CKAN clients ship SQL written against a Postgres-backed datastore (double-quoted identifiers, `::type` casts, `ILIKE`, `to_timestamp`, `CURRENT_DATE - 1`, `EXTRACT(EPOCH FROM a - b)`, pg timestamp literals). On BigQuery these fail with cryptic dialect errors. New `datastore/infrastructure/engines/bigquery/translate.py` rewrites the common pg-isms before the SQL reaches BigQuery, so the same queries keep returning the same shape of results. Covers the six compatibility areas identified against a production job log (33K distinct queries, 33M+ monthly calls): - Quoted identifiers rewritten `"ident"` → `\`ident\`` (sqlglot). - `ILIKE` → `LOWER() LIKE LOWER()` (sqlglot). - `to_timestamp(epoch_ms/1000)` → `TIMESTAMP_MILLIS(epoch_ms)`. Avoids the FLOAT64 / INT64 trap of a literal `TIMESTAMP_SECONDS(x/1000)`. - `::timestamp` casts retargeted to `CAST(... AS TIMESTAMP)` instead of the semantically-closer `DATETIME`, because the backend creates `timestamp`-typed columns as BQ `TIMESTAMP`. - `EXTRACT(EPOCH FROM a - b)` → `TIMESTAMP_DIFF(a, b, SECOND)`; bare `EXTRACT(EPOCH FROM col)` → `UNIX_SECONDS(col)`. - `CURRENT_DATE - 1` / `CURRENT_DATE + INTERVAL '2 day'` → the BQ-shaped `DATE_SUB/ADD(..., INTERVAL n DAY)` with `TIMESTAMP(...)` wrapping when compared against a column, so TIMESTAMP > DATE type mismatches never reach the engine. Design: - One `tree.sql(dialect='bigquery')` pass handles everything sqlglot already covers (identifiers, ILIKE, to_timestamp, basic casts). - Four targeted AST / post-emit passes fill the gaps that sqlglot doesn't catch on its own. Each pass is scoped tightly — e.g. the TIMESTAMP wrap only fires on `<column> <op> <date-expr>`, not on every `<x> > <y>`. - Transparent: `translate_pg_to_bq` returns `(sql, notes)` where `notes` is a human-readable list of rewrites. The engine logs at DEBUG when the SQL changes, so operators can grep real traffic. - Reversible kill switch via `Config.BIGQUERY_TRANSLATE_PG_SQL`. - Fail-soft on sqlglot parse errors — the downstream BigQuery parse surfaces the real error. 41 unit tests in `tests/engines/bigquery/test_translate.py`: one per spec item, plus a production-log regression suite that runs real customer queries end-to-end through `qualify_table_refs` and asserts the output parses as BigQuery dialect. If a translator regression ever lands, one of these tests breaks.
Three independent fixes that landed together while mirroring the legacy
CKAN API behaviour verified end-to-end.
Status codes: the legacy action router now emits real HTTP codes to
match the legacy CKAN API (which sits behind a gateway that promotes
logical errors from the classic "all 200" convention):
- Validation / Conflict Error → 409
- Not Found Error → 404
- Authorization Error → 403
- Server fault → 5xx (unchanged)
- Success → 200 (unchanged)
Body shape is unchanged — same `{help, success:false, error:{<fields>,
__type}}` envelope with per-field details flattened onto `error`. Only
the HTTP status moves. Scope: `/api/3/action/*` only; v2 and the dump
router are untouched.
Per-column `info` across all six read endpoints: `datastore_info`
already read the per-column data dictionary (title / description /
unit / example) from the CKAN resource schema under AUTH_TYPE=ckan.
Extended the same policy to `datastore_search` and
`datastore_search_sql` on both v2 and legacy so a data-dictionary
edit in CKAN reaches every read endpoint without a second round-trip:
- `datastore_search` prefers the CKAN resource schema wholesale.
- `datastore_search_sql` overlays per-column metadata from any
referenced resource whose column name matches an output column.
`type` + `format` stay the engine's — a computed `SUM(x)` or
`CAST(x AS DATE)` reports what BigQuery actually projected, not
whatever a same-named source column was.
- Legacy's `_project_fields` passes `info` through alongside `id`
+ `type`, keeping the legacy field entry shape (`{type, id, info?}`)
stable while surfacing the metadata.
Single overlay policy lives in `services/read.overlay_resource_info`;
legacy's search_sql imports it so both v2 and legacy stay in sync.
Dump column order restoration: whole-table dumps had regressed to the
BigQuery physical order (`_updated_at` first) instead of
`datastore_search`'s projection order (`_updated_at` last). Restored
`_dump_column_order` + its one call-site so dumps match the search
projection again, as commit e28a4f5 introduced.
Housekeeping: `local_test/` added to `.gitignore` — session-only
replication / validation scripts and pulled fixtures stay out of the
tree, no stray data files in future diffs.
Verified against the full suite (628 passed). Legacy test suite grew
by one — `test_legacy_search_fields_carry_info_metadata` pins the
new `info` round-trip through the legacy search response.
The `total` key on `datastore_search`, `datastore_search_sql`,
`datastore_create` and `datastore_upsert` v2 responses is now
`total_records`. The name is unambiguous — `total` historically
confused with pagination's `total_pages` + `total_count` conventions,
and clients inspecting a search envelope could misread the row count
as a page count on first glance.
Plain field rename (no alias machinery): the Pydantic attribute on
each Result class is `total_records`, the kwargs service code passes
match (`total_records=...`), the streaming writer emits
`,"total_records":`, and the v2 tests assert on `result["total_records"]`.
What stayed on `total` (intentional):
- Legacy `/api/3/action/*` wire key — CKAN parity, verified against
the legacy API's envelope. Legacy's streaming writer is a separate
code path, untouched.
- `include_total` request param — breaking the request contract
would churn every existing client script.
- Engine-side `SearchResult.total` / `WriteResult.total` — internal
dataclasses, no wire exposure.
- Internal `_build_pagination_links(total=...)` and the inner
streaming-writer `total: int | None` kwargs — never seen outside
the service layer.
Suite stays green (628 passed).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
- Fix regression from 23a37e1 that dropped _id and _updated_at from datastore_search (v2 + legacy) when AUTH_TYPE=ckan swapped the engine schema wholesale for the CKAN resource schema. Engine schema is the base again; CKAN overlays metadata on matching user columns. - Introduce SYSTEM_FIELD_INFO in core/constants.py: service-provided info (title/description/comment/example/unit) for _id and _updated_at, nested under `info` to match the datastore_create / datastore_info shape. - Replace overlay_resource_info with enrich_schema (dict | list[dict] | None), used by v2 + legacy search and search_sql. Engine owns type/format; publisher wins on everything else; system seeds fill when the publisher hasn't described a system column. - Legacy datastore_search still hides _updated_at via LEGACY_HIDDEN_COLUMNS (CKAN parity); _id now carries nested info. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Return `help` as `<base>/api/3/action/help_show?name=<action>` on legacy `datastore_search` and `datastore_search_sql` success paths, matching CKAN 2.8. v2 endpoints keep the Swagger deep link. - `fields` now accepts the JSON-array form `["col1","col2"]` alongside the existing comma-separated form, matching CKAN's documented input signature. Validator catches malformed JSON early so clients get a proper validation error, not an internal error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- CLAUDE.md §6.1 + §6.6: show _id and _updated_at in datastore_create and datastore_info response examples, with full service-provided info blocks. - Swagger datastore_create example: three flat Frictionless fields now carry title / description / comment / example / unit. - Example payload fixtures: add `example` to every user column across datastore_create variants; add *_response.json siblings for create / info / search / search_sql. - Postman generator: read `<stem>_response.json` siblings and attach as Postman example responses so collection.json ships documented responses, not empty arrays. Regenerated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
# Conflicts: # tests/test_datastore_search.py # tests/test_datastore_upsert.py # tests/test_read_service.py
luccasmmg
requested changes
Oct 27, 2026
| """ | ||
| auth_result = await context.authorize(resource_id=params.resource_id, permission="read") | ||
|
|
||
| max_limit = context.config.SEARCH_RESULT_ROWS_MAX |
Member
There was a problem hiding this comment.
I thought one of the main things the v2 api did diferently was that it enforced a maximum limit, is that not the case? cause here its enforcing that the limit should be below a certain threshold
| if offset > 0: | ||
| prev_offset = max(0, offset - limit) | ||
| out["prev"] = _link([("offset", str(prev_offset))] + base_pairs) | ||
| if limit > 0 and (total is None or offset + limit < total): |
Member
There was a problem hiding this comment.
I think this does not match the 2.8 api, it means that when we hit the end of the resource next is no longer available, if someone writes a while loop that fetches data in chunks until no records are available, that script will probably break
| yield b'{"help":' | ||
| yield orjson.dumps(help_url) | ||
| yield b',"success":true,"result":{"records":' | ||
| yield from _records_with_full_text(columns, records, keep_indices, full_text_at) |
Member
There was a problem hiding this comment.
This adds the _full_text column to every record result, even if the user hasn't requested, which is not compatible with ckan 2.8 (one extra column can cause issues for example on pandas)
And also (according to claude this one) its much slower
| yield b"}}" | ||
|
|
||
|
|
||
| def legacy_info_result( |
Member
There was a problem hiding this comment.
This one was mostly from claude, i think it explain better, seems like datastore_info shape is wrong
The three shapes
Prod CKAN 2.8 (live, POST /api/3/action/datastore_info):
"result": {
"meta": {"count": 105168},
"primary_keys": [],
"schema": {
"Settlement Day": "date",
"Settlement Period": "number",
"BSUoS Price (£/MWh Hour)": "number",
"Half-hourly Charge": "number",
"Total Daily BSUoS Charge": "number",
"Run Type": "text"
}
}schemais a flat{column: type}object, not a list.- The types are coarse:
int4andnumericcolumns both show as"number", and thetimestampcolumn shows as"date". These differ from theint4/numeric/timestampthatdatastore_searchreports for the same columns. - There's no
_idand nofieldskey.
v2 (services/read.py:263, info_datastore):
"result": {
"meta": {"resource_id": "...", "total_records": 105168, "primary_key": []},
"schema": {"fields": [{"name": ..., "type": ...}, ...]},
"fields": [{"id": ..., "type": "int4", "info": {...}}, ...]
}The wrapper (legacy/endpoints.py:445-474 → legacy/streaming.py:237-257, legacy_info_result):
"result": {
"meta": {"resource_id": "...", "total_records": 105168, "primary_key": []},
"fields": [{"type": "int4", "id": "Settlement Period", "info": {...}}, ...],
"resource_id": "..."
}This is v2's info response with schema removed and _updated_at filtered out. It doesn't take anything from the 2.8 shape: meta is passed through from the engine unchanged (backend.py:942-946).
| FRICTIONLESS_TO_BIGQUERY: dict[str, str] = { | ||
| "integer": "INT64", | ||
| "number": "FLOAT64", | ||
| "number": "NUMERIC", |
Member
There was a problem hiding this comment.
Is this expected @sagargg these are different types and will alter the creation of resources, which doesnt't seem like the idea for this PR
I caught cause it broke the tests
Legacy CKAN scripts assume the search endpoint returns whatever `limit` they ask for; capping at the v2 ceiling breaks unmodified clients that the shim exists to support. The engine streams row-by-row so peak memory stays bounded regardless. `datastore_search_sql` already had no cap on the legacy path.
- honor records_format (lists/csv/tsv) instead of always returning objects
- field-keyed validation errors ({<param>: [msg]}) for filters/q/fields/sort
- always emit _links.next so drain-until-empty pagination loops don't break
- Inject _full_text only when SELECT * reads a real table - SQL errors key under error.query (not error.sql); unknown table on SQL path returns 409 + Validation Error
Flat {meta.count, primary_keys, schema} shape; schema is {col: type}
with Postgres type names shared with datastore_search.
Switch Frictionless `number` from NUMERIC(38,9) to BIGNUMERIC(76,38) so scientific-notation and high-precision decimal inputs round-trip without truncation at write time. Cast NUMERIC/BIGNUMERIC → FLOAT64 at the BigQuery read layer so API search, search_sql, and dump (v2 and legacy) all emit the same ~15-sig-fig float repr for any stored value. Coerce integer- valued floats to int before JSON serialisation so `2` doesn't come back as `2.0`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Legacy CKAN datastore API support
Adds a wire-compatible shim at
/api/3/action/*so scripts and clients that hit a classic CKAN datastore (api.neso.energy-shape) can point at this service without code changes. Lives indatastore/legacy/and sits beside the v2 action API — the two surfaces share the same engines, auth, and services.What's in the surface
datastore_searchanddatastore_search_sql— CKAN envelope (help,success,result),_linkspagination,records_formatdispatch (objects / lists / csv / tsv).fieldsshape:{id, type, info?}with Postgres-style type aliases (int4,int8,text,timestamptz, …) — matches what classic CKAN emits._idpasses through on reads,_updated_atis hidden fromfieldsand record rows,_full_textsynthesised at position 0 in search responses.datastore_create/datastore_upsert/datastore_delete/datastore_infoon both paths./datastore/dumpdownload routes.Parity work in this PR
datastore_search_sqldialect translation — pg-dialect SQL (double-quoted identifiers,::casts,ILIKE, etc.) is rewritten to BigQuery dialect so the same client SQL that worked against CKAN keeps working.helpURL, JSON-arrayfields, legacy pagination_links, deterministic row ordering onsearch_sqlexports.integer → int4(notint8);number → numeric; dates and timestamps round-trip through orjson withOPT_PASSTHROUGH_DATETIMEso the legacy writers format them to match CKAN._idand_updated_atcarry a documentedinfoblock on every read endpoint so clients see consistent column metadata.total→total_recordsrename on the modern surface (keepstotalreserved for pagination semantics); legacy wire staystotal.example_payload/; covers every endpoint on both the v2 and legacy paths.CI — Lint + Test
ruff check .— green after the lint fixes in this PR (import order + a handful of# noqaon long docstring JSON examples).pytest— 649 passing. 5 BigQuery dialect/widening tests on this branch are pre-existing failures unrelated to the legacy surface and are tracked separately.