diff --git a/README.md b/README.md index 5ed5bf3..a30331d 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@staging" ``` # staging's latest release (early access) — pin to the tag the "staging" badge above shows -pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.31-staging" +pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.33-staging" ``` ## Usage @@ -139,7 +139,11 @@ exact same arguments. - `draft2version(draft_path, version_path)` / `publishversion(consumer_view_path, version_path)` — **not implemented yet** (both raise `NotImplementedError`); promoting a draft table into a permanent version and repointing a consumer-facing view at it. +- `createviewas(view_path, sql, overwrite=False, create_target_folder=True)` — `table2view`'s + general form: creates a view from any query body you hand it, not just `SELECT * FROM `. + The SQL is sent through unchanged. - `deleteview(view_path)` — `DROP VIEW IF EXISTS`. +- `deletetable(table_path)` — `DROP TABLE IF EXISTS`; `deleteview`'s counterpart. **Copying/moving data** — always over Arrow Flight SQL, never REST, regardless of `EEA_CATALOG_TRANSPORT` (needs `username=` on `Catalog(...)` for the Flight auth handshake): @@ -156,11 +160,39 @@ exact same arguments. `target_path` actually exists in the catalog before declaring success, rather than trusting a silent CREATE/DROP. +**A whole folder at once** (`catalog/foldertree.py` — orchestration over the operations above, +not one statement each): +- `folderscan(source_path, target_path)` — read-only: every sub-folder, table and view under + `source_path`, mapped to where it would land under `target_path`, with each view's SQL already + rewritten and the views in dependency order. Returns a `FolderTree`. +- `foldercopy(source_path, target_path, dry_run=False, overwrite=False, skip_existing=True, + verify_views=True, copy_metadata=True, tree=None)` — reproduces the subtree and **proves it**: + folders first (empty ones included), then tables via `datacopy` with `COUNT(*)` compared on + both sides, then views recreated from their own SQL with every reference into the moved + subtree remapped to the new location, so the copy is self-contained. Returns a + `FolderTransferReport`; a per-entry failure is reported, not raised. +- `foldermove(source_path, target_path, ..., delete_source_root=True)` — `foldercopy`, and then + removes the source **only if `report.verified`**: views in reverse dependency order, then + tables, then folders deepest-first. Anything less than a clean copy leaves the source exactly + where it is. +- `remap_view_sql(sql, source_path=..., target_path=..., entries=..., context=...)` — the view + rewriter on its own, if you want to see what it would do to one definition. + **Discovery** - `gettablesfrom(schema_path)` — every table/view under `schema_path`, recursing into subfolders; returns full dot-separated paths. +- `getentriesfrom(schema_path)` — the same sweep, but each path carries whether it is a + `"TABLE"` or a `"VIEW"` (`CatalogEntry`), so a folder-level operation knows what each entry is + without a lookup per path. - `gettableitemsfrom(table_path)` — one table/view's schema (`{column: DATA_TYPE}`) and row count, without fetching any actual rows. +- `entryexists(path)` — whether `path` is a table or view Dremio knows about. Fetches no rows. +- `getviewsql(view_path)` — the SQL a view is defined by, as a `ViewDefinition(sql, context)`. + Read from Dremio's catalog entity where it answers (which also carries the `sqlContext` the + SQL was written against), falling back to `INFORMATION_SCHEMA."VIEWS"`, which has the text but + no context. Raises rather than guessing if neither answers. +- `getfoldersfrom(path, recursive=True)` — the folders under `path`, shallowest first. REST-only: + folders are not in `INFORMATION_SCHEMA`, so an *empty* folder can only be found this way. **Wiki & tags** (Dremio's catalog collaboration API — REST-only, no SQL/Flight equivalent): - `getwikifrom(path)` / `setwikito(path, text, tags=None)` / `deletewiki(path)` — read, @@ -382,6 +414,7 @@ import eea_datalakehouse.notebook # registers %catalog/%ingest — no %load_ext %ingest ingest(folder="./bw_2026", target_catalog_path="bwd.reference", data_format="parquet", table_name="water_temperature") +%ingest diagnose # optional: check the queued ingests first; nothing is uploaded %ingest commit(retry=True) ``` @@ -390,6 +423,12 @@ parameters, description — rather than a raw Python signature, since its audien custodian, not necessarily a developer. `%ingest help` does the same for `IngestSession`'s (fewer) methods — handy when you don't remember an exact parameter name mid-notebook. +`%ingest ingest(...)` also takes what a one-off ingest may need, each on that same line: +`upload_mode="proxy"` when S3 is unreachable from the kernel, `csv_options={"delimiter": ";"}`, +`json_options={"format": "array"}`, `table_definition={"columns": [{"name": "value", +"type": "DOUBLE"}]}` for the column types of a CSV or JSON table, and `cleanup_on_failure=True` +so a failed commit deletes its upload instead of leaving it for a retry. + A freshly created session already has a context — it starts at `"catalog"` (the catalog root), not empty — so a relative path works even before the first `use()` call. `use(None)`/`use("")` reset it back to `"catalog"` the same way; `set_context(None)` (the @@ -485,6 +524,7 @@ context itself — raises `CatalogSessionError` if none is set yet: | `sdi/iso.py` | reads UUID, title, edition, dates and series children from ISO 19115-3 | | `catalog/client.py` | `Catalog` — two connections (REST + Flight), every operation as a method | | `catalog/operations.py` | the operations themselves (table2view, datacopy, createfolder, ...), as functions taking an executor and/or a `CatalogRestClient` | +| `catalog/foldertree.py` | `folderscan`/`foldercopy`/`foldermove` — a whole folder at a time, with the view-SQL rewriter (`remap_view_sql`) that keeps a copied subtree self-contained | | `catalog/sql.py` | `SqlExecutor` protocol + REST/Flight implementations, `resolve_executor()` (transport env var) | | `catalog/rest.py` | `CatalogRestClient` — Dremio's native `/api/v3/catalog` for folders, wiki, and tags (none of which have a SQL/Flight equivalent) | | `catalog/retry_state.py` | persisted memory of stalled attempts, for `retry_pending()` | @@ -565,7 +605,7 @@ To pin to one specific release instead, use the exact tag the live badges under %pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.30" # staging's latest release (early access) -%pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.31-staging" +%pip install "git+https://github.com/eeadata/EEALakeHouse.python.git@v0.1.33-staging" ``` Use the `%pip` magic rather than `!pip` — it installs into the kernel the diff --git a/docs/dds-ingest-robustness-client-plan.md b/docs/dds-ingest-robustness-client-plan.md index 1f2c41d..38289a1 100644 --- a/docs/dds-ingest-robustness-client-plan.md +++ b/docs/dds-ingest-robustness-client-plan.md @@ -1,6 +1,9 @@ # DDS ingest robustness — the client half (DI-12) — plan -> Status: **plan only — nothing implemented yet.** Date: 2026-10-16. +> Status: **PY-1 … PY-7 implemented** (2026-10-16). Reviewed against the code on +> 2026-10-19 (§5); the follow-up tasks **PY-8 … PY-13 implemented** the same day, +> PY-11 with the scope agreed with Oskar. Open: the §5.1 repair reaching `main` +> (PR #97, left to the user and Oskar). Date: 2026-10-16. Companion plan for **this** repository. The design, the server work and the release schedule live with DDS: @@ -37,7 +40,7 @@ repeats the ⚠ note): | Task | What changes for an existing script | Mitigation | |---|---|---| -| PY-1 | with `upload_mode=auto`, a network failure on the presigned upload now retries through DDS (`/stage`) instead of raising | only where the script fails today; `upload_mode=direct` on the server keeps the old behaviour | +| PY-1 | when the server says `upload_mode: "auto"` (DDS's default) and `FolderIngest` keeps its default `upload_mode="auto"`, a network failure on the presigned upload now retries through DDS (`/stage`) instead of raising | only where the script fails today; `FolderIngest(upload_mode="direct")`, or `direct` on the server, keeps the old behaviour | | PY-2 | an upload that fails part-way now cancels the session (its landed files are deleted) before the exception is raised | nothing is lost: an incomplete upload cannot be retried anyway (`retry` re-runs only the load) | | PY-6 | CSV/JSON ingests without `table_definition` now send locally inferred columns | the inferred schema is printed; `infer_schema=False` restores today's behaviour | @@ -45,10 +48,13 @@ repeats the ⚠ note): ## 2. Tasks -### PY-1 Honour `upload_mode`, with automatic `/stage` fallback — R1 +### PY-1 Honour `upload_mode`, with automatic `/stage` fallback — R1 — **DONE** -**Today:** the client ignores the `upload_mode` that `begin` returns and has no -automatic fallback (G10 in `Documentation/data-digestion.md`). +**Before this task** (corrected 2026-10-19): `FolderIngest` already honoured the +server's `upload_mode: "proxy"` hint and took its own +`upload_mode="auto" | "direct" | "proxy"` — Oskar Esparza, `774691a`, 2026-10-08. +This plan was written on a branch that did not have that commit yet. What was +missing was the automatic fallback (G10 in `Documentation/data-digestion.md`). - Implement the contract's upload-mode rule, the same text the extension's `uploadOne` implements: `proxy` → always `IngestClient.stage`; `direct` → @@ -61,7 +67,18 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). - **Verify:** tests with an unreachable presigned URL under each mode; a file over the multipart threshold in proxy mode. -### PY-2 Cancel when an upload fails; small fixes — R1 +> Landed `9d924f7`. The merge with `774691a` on `main` (`101b09c`) dropped part of +> Oskar's code; repaired the same day twice over (§5.1), and since `staging` was +> merged into `eea_bliki` the code is Oskar's repair, `ea142ca`. +> `BeginResult.upload_mode` reads the hint from the top level or from inside `s3`, +> and no hint (an older server) reads as `"direct"`. `_effective_upload_mode` +> decides: `FolderIngest(upload_mode="direct"|"proxy")` wins, and its default +> `"auto"` follows the server. The fallback happens when that comes to `"auto"` — +> both sides say so. DDS sends `upload_mode` on every begin, `"auto"` unless +> configured, so the fallback is on by default. The `/stage` size cap is checked +> before uploading since PY-10. + +### PY-2 Cancel when an upload fails; small fixes — R1 — **DONE** - `FolderIngest.run()` (`folder.py:218-234`) has no failure handling. On an upload-stage failure, call `IngestClient.cancel(session_id)` before raising @@ -76,7 +93,13 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). - **Verify:** an upload that fails on file 3 of 5 leaves no session and no objects (against a mocked DDS that records the `DELETE`). -### PY-3 Optional table definition — R2 +> Landed `9d924f7`. As built: any upload failure cancels the session, and the +> uploads still queued are cancelled at the first failure; the PUT comment is fixed; +> `.ndjson` is in the format table (`formats.py`). Under +> `IngestSession(retry=True)` the cancel hid the upload error until PY-8; the +> client guide describes it since PY-12. + +### PY-3 Optional table definition — R2 — **DONE** - A `TableDefinition` / `ColumnDef` dataclass in `dds_ingestion/models.py`, matching the contract object of DI-12.1b (`version`, `columns` with optional @@ -98,7 +121,10 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). definition → only set fields sent; against a server without `definition_applied` → the definition goes on commit. -### PY-4 Show the plan, and `dry_run` — R2 +> Landed `6a4faaf`. `IngestClient.retry(definition=…)` for DI-12.6 followed in +> PY-9. + +### PY-4 Show the plan, and `dry_run` — R2 — **DONE** - When `begin` returns a `plan` (DI-12.2), print it before uploading: placement, table path, storage path, conflict mode, and the definition report. @@ -110,7 +136,13 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). - **Verify:** plan printed when present, nothing printed when absent; `dry_run` against old and new server. -### PY-5 Failure text, `retryable`, `cleanup_on_failure` — R3 +> Landed `6a4faaf`. As built: the dry run is a separate `FolderIngest.dry_run()` / +> `IngestClient.dry_run()` returning a `DryRunResult`, not a `dry_run=` flag (a dry +> run has no session id, and `BeginResult.session_id` stays a `str`). Against a +> server without dry runs it raises `UnsupportedByServerError`, a subclass of +> `IngestApiError`, so callers that catch `IngestApiError` need no change (rule 4). + +### PY-5 Failure text, `retryable`, `cleanup_on_failure` — R3 — **DONE** - Show the server's failure fields verbatim: `error`, `failed_stage`, `retryable`, and the error body's `message`, in the same format as the @@ -125,7 +157,12 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). - **Verify:** attributes populated from a 4xx and from a failed poll; the `cleanup_on_failure` matrix. -### PY-6 CSV/JSON options, inferred columns, local Parquet check — R3/R4 +> Landed `a7f99e9`. As built: `IngestApiError.error_slug` and `.retryable`; a failed +> commit carries the session's own failure text (`StatusResult.describe_failure`) as +> an exception note; `cleanup_on_failure` as planned. `failed_stage` became an +> attribute in PY-13. + +### PY-6 CSV/JSON options, inferred columns, local Parquet check — R3/R4 — **DONE** - One format table in `dds_ingestion/` (extensions, options builder, local schema reader), replacing `folder.py:45` plus the scattered checks; the client @@ -147,7 +184,18 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). caught locally; one end-to-end editable CSV run against the real-Dremio `local-stack` profile (DI-12.15) that used to hit the 501. -### PY-7 `FolderIngest.diagnose()` — R4 +> Landed `a0faf15`. As built: `formats.py` (`FORMATS`, `local_columns`, +> `compare_columns`); `csv_options` / `json_options` are sent only for their own +> format; columns are inferred only for CSV/JSON into an **editable** table, the one +> case where the server builds the table from them; a JSON array cannot be inferred +> and says so. Open: the end-to-end editable CSV run against `local-stack` +> (DI-12.15) has not been done from this client. Fixed 2026-10-19: a CSV whose text is not +> UTF-8 was inferred as `VARBINARY` columns (pyarrow reads such text as binary) and +> sent. Since DDS passes no `encoding` on to Dremio, which reads CSV as UTF-8, the +> client now refuses a non-UTF-8 `csv_options` `encoding`, and a file that is not +> UTF-8, before anything is uploaded. + +### PY-7 `FolderIngest.diagnose()` — R4 — **DONE** - Checks, in order, and prints each result in plain language: DDS reachable, credential valid, WRITE on the target, the storage self-test verdict @@ -156,6 +204,11 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). - **Server:** DI-12.3 (R2). - **Verify:** one test per failing step. +> Landed `c1149d5`. As built: four steps — DDS reachable; credential accepted +> (`GET /ingest/limits`, or `GET /ingest` on a server without it); storage self-test; +> dry run of the folder. WRITE on the target is covered by the dry run rather than +> checked on its own, so against a server without dry runs (before R2) it is skipped. + --- ## 3. Order @@ -170,6 +223,8 @@ automatic fallback (G10 in `Documentation/data-digestion.md`). Each task can be released as soon as it is ready: by rule 3 it also works against the DDS release before the one it needs, with today's behaviour. +The follow-up tasks from the 2026-10-19 review are ordered in §5.5. + ## 4. Definition of done - Every existing call sends the same request bodies as before (golden tests), @@ -181,3 +236,224 @@ the DDS release before the one it needs, with today's behaviour. columns; Parquet columns are checked locally before upload. - `docs/python-client-guide.md` describes each new option, with one example with and one without a table definition. mypy clean; the test suite passes. + +> 2026-10-19: met — the client guide since PY-12 (`f512d2f`). `dry_run` is a +> method rather than an option (PY-4). mypy is clean for the code these tasks +> touched; the remaining errors are in `dds_ingestion/common/dremio_identity.py`. + +--- + +## 5. Review against the code — 2026-10-19 + +Every task was checked against `eea_bliki` after it was brought up to date with +`main` — which now carries Oskar Esparza's `%catalog` / `%ingest` work and +`774691a` — and against the DDS side: the DI-12 plan's §7.2 progress notes and the +server code. Oskar's work takes priority where the two meet. + +### 5.1 First: `main` cannot ingest + +`101b09c`, which merged `eea_bliki` into `main` on 2026-10-19 in GitHub's conflict +editor, dropped twelve lines of `774691a`: + +- `BeginResult.from_json` raises `NameError: s3` on every begin, so + `FolderIngest.run()` fails before uploading anything. The release workflow moved + the `main` and `v0.1.30` tags to that merge, so `pip install …@main` and + `…@v0.1.30` install it. `@staging` (`v0.1.31-staging`) does not contain + PY-1 … PY-6 and is not affected. +- `FolderIngest(upload_mode="proxy")` reported a proxy upload while uploading to S3. +- 12 tests fail on `main`. + +Repaired twice on 2026-10-19, independently: on `eea_bliki` (`60cad54`: Oskar's +`774691a` restored as written, `upload_mode` `None` without a hint) and by Oskar on +`development`, which reached `staging` (`ea142ca`: `_effective_upload_mode`, and +`"direct"` without a hint). The two behave the same; they differ only in what "no +hint" reads as, and Oskar chose `"direct"`. When `staging` was merged into +`eea_bliki`, his version was taken and the later tasks (PY-8, PY-10, PY-13, the CSV +encoding check) were re-applied on top of it. `main` itself still needs one of them +(PR #97 carries `60cad54`). + +### 5.2 Where each task stands + +| Task | State | Found in the review | Follow-up | +|---|---|---|---| +| PY-1 | done; built on `774691a` | `/stage` size cap not checked; not in the client guide | PY-10, PY-12 — done | +| PY-2 | done | hides the upload error under `IngestSession(retry=True)`; not in the guide | PY-8, PY-12 — done | +| PY-3 | done | `retry` cannot send a corrected definition | PY-9 — done | +| PY-4 | done, as a method | — | — | +| PY-5 | done | `failed_stage` only in the note text | PY-13 — done | +| PY-6 | done | end-to-end editable CSV run against `local-stack` not done | §5.7 — open | +| PY-7 | done | — | — | +| §4 | met | `docs/python-client-guide.md` described none of the new options | PY-12 — done | + +Beyond the tasks: `IngestSession.ingest`, behind `%ingest`, passed `FolderIngest` a +fixed set of options, so none of the options above reached a notebook user (PY-11 — +done). + +### 5.3 Follow-up tasks + +#### PY-8 `retry()` after a cancelled upload — **DONE** + +- **Today:** PY-2 cancels the session after a failed upload, and + `FolderIngest.retry()` then raises `IngestStateError("no session to retry — call + run() first")`. `IngestSession.commit(retry=True)` re-raises that error, so + `%ingest` shows it instead of the upload error: an S3 403 reads "no session to + retry" (checked 2026-10-19). +- Remember the upload error that led to the cancel. `retry()` then: + - re-runs `run()` under a new session when that error was transient (a network + error, or a 5xx from S3 or DDS) **and** the cancel succeeded, dropping the + `idempotency_key` as its "anything else" branch already does; + - otherwise raises `IngestStateError` whose message quotes the upload error. A + failed cancel may have left files in place, and re-running a read-only ingest + stored permanently would add a second copy. +- No change to `IngestSession`. +- **⚠ Compatibility:** `retry()` after a failed upload used to raise; after a + transient failure it now uploads again. The cancel deleted whatever had landed, + so nothing is duplicated. +- **Server:** none. +- **Verify:** `IngestSession(retry=True)` after an S3 403 names the 403; after a + network error in `direct` mode the next attempt uploads and commits; after a + failed cancel nothing is re-uploaded. + +> Landed `7dad450`. As built: `run()` records the upload failure, and `retry()` +> looks at it before anything else — also when the cancel failed and a session id +> is left, because that session never got all its files and asking the server +> about it would only say "uploading". "Can pass" (`_may_succeed_on_retry`) is a +> network error, or an `IngestApiError` whose `retryable` is true or, without +> one, whose status is 5xx. Since PY-11, a commit that `cleanup_on_failure` cancelled is +> recorded the same way, so a retry after a cleanup uploads again instead of +> finding no session. + +#### PY-9 `retry` with a corrected table definition (rest of PY-3) — **DONE** + +- `IngestClient.retry(session_id, *, definition=None)` and + `FolderIngest.retry(*, table_definition=None)`: with a definition, the resumed + load uses it (DI-12.6), and a re-upload under a new session sends it on begin. + Without one, the request is exactly today's (golden test). +- For a commit that failed with `schema_required`, retryable since DI-12.5: a retry + with the columns fixes it without uploading again. +- **Server:** DI-12.6 (done, R3). +- **Verify:** `schema_required` → `retry(table_definition=…)` → done; the request + without a definition is unchanged; against a server before DI-12.6 the retry + behaves as today. + +> Landed `9b4f429`. As built: without a definition, `FolderIngest.retry()` still +> calls `client.retry(session_id)` with no keyword, so a client written against +> the old signature — two test fakes in `IngestSession`'s tests are — keeps +> working. The behaviour against a server before DI-12.6 is from that server's +> code (its endpoint takes no body), not a test. + +#### PY-10 Check file sizes against the server's limits (rest of PY-1) — **DONE** + +- DDS has two caps. `begin` refuses a file over the caller's cap (DI-12.13), but + `POST /ingest/stage` has its own `stage_max_bytes` and refuses a file only when + that file arrives — after other files have uploaded, which PY-2 then cancels. The + two caps are equal unless the server sets a separate stage cap. +- Read `GET /ingest/limits?runner=notebook` once per run. In proxy mode, refuse + before uploading anything when a file exceeds `stage_max_bytes`. With the + fallback, do not fall back for such a file: raise the network error with a note + saying why. A 404 (a server before DI-12.11) keeps today's behaviour. +- `diagnose()` already calls `limits()`: show the caps there and flag the files + over them. +- **Server:** DI-12.11, DI-12.13 (done, R4). +- **Verify:** proxy mode with one file over the stage cap uploads nothing; the + fallback is not attempted for such a file; an older server is unchanged. + +> Landed `2a554b2`. As built: proxy mode asks for the limits before uploading; the +> fallback asks only when a file actually falls back, so a plain direct upload +> makes no extra request. Best-effort: a 404, or any failure of the limits +> request, leaves DDS's own check as the only one. `diagnose()` gained a "File +> sizes" step after the credential step: a file over `max_bytes` fails it, one +> over `stage_max_bytes` fails it only with `upload_mode="proxy"`. + +#### PY-11 The new options in `%ingest` — Oskar's decision — **DONE** + +- `IngestSession.ingest` (`dds_ingestion/session.py`) passes `FolderIngest` seven + options: `intent`, `conflict_mode`, `table_name`, `sub_path`, `parallelism`, + `multipart`, `show_progress`. Not `table_definition`, `csv_options`, + `json_options`, `infer_schema`, `cleanup_on_failure` — nor `upload_mode`. +- Proposal: add those six to `IngestSession.ingest` and to `%ingest help`, and + decide whether `dry_run()` and `diagnose()` belong in the notebook layer. +- Why it matters: without `table_definition`, a CSV/JSON ingest into an editable + table whose columns cannot be inferred has no way through `%ingest`. It fails at + commit today, and at begin once `DDS_INGEST_SCHEMA_AT_BEGIN` is on. +- **Owner:** Oskar Esparza — it is his module; nothing changes there without him. +- **Server:** none. + +> Landed `e1c262d` and `a02a561`. Scope set on 2026-10-19 after talking to Oskar: +> keep `IngestSession`'s logic and queue-then-commit concept, and add only what a +> one-off user can give on the **one** `%ingest` line — what cannot be, stays out. +> As built: `upload_mode`, `csv_options`, `json_options`, `table_definition` (a +> dict on that line) and `cleanup_on_failure` are forwarded — the last at the +> user's request, since for most one-off ingests a retry will not help and a failed +> commit should leave nothing behind; `commit(retry=True)` then uploads a +> cleaned-up ingest again (PY-8). `infer_schema` is left out: turning inference off +> only makes DDS ask for the columns, which `table_definition` gives anyway. One +> verb, `%ingest diagnose`, runs +> `FolderIngest.diagnose()` for every queued ingest and keeps the queue; no +> `dry_run` verb, since `diagnose` ends with one. + +#### PY-12 Client guide — the open part of §4 — **DONE** + +- `docs/python-client-guide.md` describes: `upload_mode` (`FolderIngest`'s choice, + the server's hint, the fallback); cancel on upload failure; `table_definition`, + with one example with and one without; `csv_options` / `json_options` / + `infer_schema`; `cleanup_on_failure`; `dry_run()`; `diagnose()`; reading + `error_slug`, `retryable` and the failure note. +- The DI-12 plan (§7.2) lists the `upload_mode` and cancel parts as open since R1. + +> Landed `f512d2f`, covering PY-8, PY-9, PY-10 and PY-13 as well. As built: Part 1's +> reference tables also caught up with PY-3 … PY-7 (models, `IngestClient` +> endpoints, errors), and the "S3 is unreachable" recipe uses +> `upload_mode="proxy"` instead of driving `/stage` by hand. + +#### PY-13 Small fixes — **DONE** + +- `failed_stage` as an attribute on the exception from a failed commit (PY-5 named + it; today it is only in the note). +- `IngestOutcome.files_staged`: how many files went through DDS, so a fallback is + visible. `IngestOutcome.upload_mode` stays as it is. + +> Landed `a5900d8`. + +### 5.4 Not planned yet + +- **Dremio job ids in the failure text** (DI-12.18, R5): only together with the + extension, which does not show them either — DI-12.14 keeps one failure text for + both clients. +- **A local schema-drift warning** before a read-only append, like PY-6's Parquet + check: after DI-12.22 (R6b). +- **Undo for `IngestSession`** once DDS can delete a read-only table (DI-12.21, R6b); + its module docstring notes that it is missing. +- **Non-UTF-8 CSV that actually loads.** Today the client refuses it (PY-6 note). + Either DDS honours `csv_options['encoding']` — server work: it accepts the field + and uses it nowhere — or the client converts the files to UTF-8 before `begin`, + which changes their sizes and so has to happen before the file list is sent. + +### 5.5 Order + +| Step | What | Why first | State | +|---|---|---|---| +| 1 | §5.1 to `main`, with Oskar | every Python ingest from `@main` fails today | on `eea_bliki` (`60cad54`); PR #97 into `main` is open, and merging it is left to the user and Oskar | +| 2 | PY-8, PY-12, PY-13 | fix and document what is already shipped | done 2026-10-19 | +| 3 | PY-9, PY-10 | use server features that are already deployed | done 2026-10-19 | +| 4 | PY-11 | when Oskar agrees | done 2026-10-19 | + +Steps 2 and 3 were built in the order PY-8, PY-13, PY-9, PY-10, PY-12, so the guide +documents all of them at once. + +### 5.6 Coordination + +- Oskar Esparza reviews the §5.1 repair (PR #97), which restores his `774691a`. + PY-11's scope was agreed with him. +- The DI-12 plan (§7.2) cites PY-7 as `8d79542`; that commit was rebased before it + was pushed and is `c1149d5`. + +### 5.7 Release notes + +- Before `DDS_INGEST_SCHEMA_AT_BEGIN` is turned on (DI-12 plan §7.3), the kernels + also need a Python release with PY-3 and PY-6, from after the §5.1 repair: a script + that sends its columns only on commit (`IngestClient.commit(definition=…)`) is + refused at begin. +- After each deploy: one ingest per format through this client into the production + test area (DI-12 plan §7.1 rule 4), and PY-6's editable-CSV run against + `local-stack`. diff --git a/docs/dev-notes.md b/docs/dev-notes.md index 04f01d5..d3bd917 100644 --- a/docs/dev-notes.md +++ b/docs/dev-notes.md @@ -450,6 +450,26 @@ rule, destructive operations should echo the fully resolved path before acting. #### 8. Proposed: `datacopy` should accept a folder and copy everything in it +> **Status: done** — implemented as `catalog/foldertree.py` +> (`folderscan` / `foldercopy` / `foldermove`), taking §8a's option 1: separate +> verbs named for the folder, not an overloaded `datacopy`. What §8 left for the +> team to decide was decided as follows, and each is documented where it lives: +> +> | Question | What was built | +> |---|---| +> | Views | Recreated **as views**, with every reference into the moved subtree rewritten to the new location (`remap_view_sql`), so the copy is self-contained. Never materialised into tables. `getentriesfrom` adds `TABLE_TYPE` as suggested, as a sibling of `gettablesfrom` rather than a change to its return type. | +> | Depth | `cp -r`, stated: the whole subtree, empty folders included (found over REST, since `INFORMATION_SCHEMA` cannot see them). | +> | Partial failure | Per-entry, not per-operation: each entry gets its own derived idempotency key (`"::"`), a failure is reported rather than raised, and `skip_existing=True` makes a re-run skip what already landed with a matching row count. | +> | `overwrite=True` at folder scale | Per colliding entry, never "replace the target folder". | +> | Parallelism | Not done, deliberately — the Flight-client concern in §8 is unresolved, and the module docstring says so. | +> | Return type | `FolderTransferReport`, per entry, in the spirit of `IngestOutcome`. | +> | `is_folder` best-effort | A failed REST folder listing degrades to rebuilding the tree from the entry paths, with a note on the report saying empty folders will not be reproduced — rather than failing or silently losing them. | +> | Should `datamove` get the same treatment? | Yes, and its partial-failure story is answered by never deleting anything until the copy has verified: `foldermove` is `foldercopy` + delete, gated on `report.verified`. | +> +> `datacopy`/`datamove` are unchanged, so nothing released is broken. §9 (the +> naming sweep) is still open — the new verbs follow the existing +> lowercase-run-together style rather than pre-empting that decision. + **Position to put to the team** (proposed here, not yet discussed with them): a custodian should be able to point `datacopy` at a dataset folder and have every table inside it copied, rather than making one call per table. diff --git a/docs/notebook-facade-for-data-scientists.md b/docs/notebook-facade-for-data-scientists.md index 4b0b238..a06b414 100644 --- a/docs/notebook-facade-for-data-scientists.md +++ b/docs/notebook-facade-for-data-scientists.md @@ -201,6 +201,7 @@ the second call in a session. import eea_datalakehouse.notebook %ingest ingest(folder="./bw_2026", target_catalog_path="bwd.reference", data_format="parquet") +%ingest diagnose # optional: check what is queued; uploads nothing, keeps the queue %ingest commit(retry=True) %catalog set_tags(".water_temperature", ["reviewed"]) diff --git a/docs/python-client-guide.md b/docs/python-client-guide.md index 53e08ef..68af0e5 100644 --- a/docs/python-client-guide.md +++ b/docs/python-client-guide.md @@ -37,12 +37,16 @@ behaviour behind those snippets. - [The setup cell](#the-setup-cell) - [Ingest recipes](#ingest-recipes) - [Catalog recipes](#catalog-recipes) + - [Move a whole folder](#move-a-whole-folder-somewhere-else) - [End to end, in one cell](#end-to-end-in-one-cell) - [Part 1 — `dds_ingestion`](#part-1--dds_ingestion) - [The transfer model](#the-transfer-model) - [`FolderIngest`](#folderingest) - [`intent` — what a transfer leaves behind](#intent--what-a-transfer-leaves-behind) - [`sub_path` — read-only data that accumulates](#sub_path--read-only-data-that-accumulates) + - [Table definitions, CSV/JSON options, inferred columns](#table-definitions-csvjson-options-inferred-columns) + - [`upload_mode` — how the bytes travel](#upload_mode--how-the-bytes-travel) + - [Checking a transfer first: `dry_run()` and `diagnose()`](#checking-a-transfer-first-dry_run-and-diagnose) - [Managing, resuming and abandoning a transfer](#managing-resuming-and-abandoning-a-transfer) - [`IngestOutcome`, `scan_folder`, `ingest_folder`](#ingestoutcome-scan_folder-ingest_folder) - [`IngestClient` — one method per endpoint](#ingestclient--one-method-per-endpoint) @@ -55,6 +59,7 @@ behaviour behind those snippets. - [`Catalog` and its two transports](#catalog-and-its-two-transports) - [`idempotency_key` and the retry contract](#idempotency_key-and-the-retry-contract) - [Operation reference](#operation-reference) + - [Whole folders](#whole-folders--catalogfoldertree) - [Executors (`catalog.sql`)](#executors-catalogsql) - [`CatalogRestClient` (`catalog.rest`)](#catalogrestclient-catalogrest) - [`retry_state`](#retry_state) @@ -133,7 +138,12 @@ fewest lines that actually work. | …and keep the handle, so a failure is resumable | `job = FolderIngest(..., **_dds); job.run()` | | add this year to a table that grows | `ingest(..., intent="read_only", table_name="t", sub_path="2026")` | | redo one year only | `ingest(..., sub_path="2026", conflict_mode="replace")` | +| CSV / JSON into an editable table | `ingest("./csv", "path.to.target", data_format="csv", intent="editable")` — columns inferred | +| …with the column types I choose | `ingest(..., table_definition=TableDefinition(columns=(ColumnDef("v", "DOUBLE"),)))` | +| check a transfer before starting it | `job.diagnose()` · `job.dry_run()` | | resume a failed transfer | `job.retry()` | +| …after fixing the columns | `job.retry(table_definition=…)` | +| S3 is unreachable from this kernel | `FolderIngest(..., upload_mode="proxy", **_dds).run()` | | pick a transfer up in a later kernel | `FolderIngest.attach(session_id, ..., **_dds)` | | abandon a transfer | `job.cancel()` | | see my transfers | `IngestClient(**_dds).list_sessions(state="failed")` | @@ -141,6 +151,9 @@ fewest lines that actually work. | turn an ingested table into a view | `catalog.table2view(view, source, idempotency_key=KEY)` | | …for every table in a folder | `for p in catalog.gettablesfrom(SRC, …): catalog.table2view(…)` | | copy / move a table | `catalog.datacopy(a, b, …)` · `catalog.datamove(a, b, …)` | +| **move a whole folder** — subfolders, tables and views | `foldermove(catalog, src, dst, idempotency_key=KEY)` | +| …see what that would do, first | `foldermove(…, dry_run=True)` → `print(report.summary())` | +| copy a folder without removing the source | `foldercopy(catalog, src, dst, idempotency_key=KEY)` | | list everything under a folder | `catalog.gettablesfrom("bwd", idempotency_key=KEY)` | | schema + row count, no rows fetched | `catalog.gettableitemsfrom(path, idempotency_key=KEY)` | | create / delete a folder | `catalog.createfolder(p, …)` · `catalog.deletefolder(p, …)` | @@ -245,6 +258,47 @@ ingest("./bw_2026", "water_management_resources.bathing_water.bwd.reference", table_name="water_temperature", sub_path="2026", conflict_mode="replace") ``` +### CSV or JSON into an editable table + +An editable table is built from column types. For CSV and JSON the client reads +every file locally, infers them, prints them (`Inferred columns: …`) and sends +them with the transfer — nothing to write: + +```python +out = ingest("./stations_csv", "biodiversity.uploads", data_format="csv", intent="editable") +``` + +To choose the types yourself, pass a table definition — and parsing hints, if +the files need them: + +```python +from eea_datalakehouse.dds_ingestion import ColumnDef, TableDefinition + +columns = TableDefinition(columns=( + ColumnDef("station_id", "VARCHAR"), + ColumnDef("measured_at", "TIMESTAMP"), + ColumnDef("value", "DOUBLE"), +)) +out = ingest("./stations_csv", "biodiversity.uploads", data_format="csv", intent="editable", + table_definition=columns, csv_options={"delimiter": ";"}) +``` + +A JSON *array* file cannot be inferred — pass its columns this way. Details: +[Table definitions](#table-definitions-csvjson-options-inferred-columns). + +### Check a transfer before you start it + +```python +job = FolderIngest("./my_data", "biodiversity.uploads", data_format="parquet", **_dds) +job.diagnose() # one ✓ / ✗ line per check, ending with a dry run of this folder +job.dry_run() # only the dry run: what commit would do +``` + +Neither uploads anything or opens a session. `diagnose()` says, in plain +language, which of DDS, your credential, the file sizes, DDS's storage and the +target is the problem — see +[Checking a transfer first](#checking-a-transfer-first-dry_run-and-diagnose). + ### The transfer failed — resume it without re-uploading ```python @@ -259,8 +313,25 @@ except Exception: `retry()` decides from the server's own report. It re-runs just the load step when the staged files survived, starts a fresh session when they did not, and **raises rather than guessing** for a permanently-stored transfer that a blind -re-run would duplicate. The full decision table is -[here](#managing-resuming-and-abandoning-a-transfer). +re-run would duplicate. An *upload* failure has already cancelled the session +(the error's note says so); `retry()` then uploads again if the failure can pass +— a network error, a 5xx — and otherwise raises with the upload error. The full +decision table is [here](#managing-resuming-and-abandoning-a-transfer). + +### The load failed on its columns — fix them and resume + +A commit that failed with `schema_required` or `schema_mismatch` (the error's +`error_slug`) needs no new upload — only the right columns: + +```python +from eea_datalakehouse.dds_ingestion import ColumnDef, TableDefinition + +job.retry(table_definition=TableDefinition(columns=(ColumnDef("value", "DOUBLE"),))) +``` + +The corrected definition replaces the stored one and only the load re-runs. It +needs a DDS server with DI-12.6; an older one ignores the definition and the +retry fails as before. ### Pick a transfer up in a later kernel @@ -326,23 +397,21 @@ dataflow level and below are created. ### S3 is unreachable from this kernel — send the bytes through the server -The fallback when presigned uploads cannot connect. Slower, since every byte -transits DDS: +Every file goes through DDS (`POST /api/v1/ingest/stage`) instead of to the +presigned S3 targets. Slower, since every byte transits DDS: ```python -from pathlib import Path -from eea_datalakehouse.dds_ingestion import scan_folder - -folder = Path("./my_data") -with IngestClient(**_dds) as c: - begin = c.begin(target_catalog_path="biodiversity.uploads", intent="read_only", - data_format="parquet", conflict_mode="fail", - files=scan_folder(folder, "parquet")) - for t in begin.s3.uploads: - c.stage(begin.session_id, t.rel_path, (folder / t.rel_path).read_bytes()) - print(c.commit(session_id=begin.session_id)) +job = FolderIngest("./my_data", "biodiversity.uploads", data_format="parquet", + upload_mode="proxy", **_dds) +out = job.run() +print(out.upload_mode, out.files_staged) # "proxy", every file ``` +Usually you do not need to ask. With the default `upload_mode="auto"` the client +follows the server, which can say `proxy` itself; and when it says `auto` (DDS's +default), a file whose presigned upload fails at the network layer goes through +DDS on its own. See [`upload_mode`](#upload_mode--how-the-bytes-travel). + ### Tell staged from permanent Whether your uploaded files *are* the table matters before you re-run anything: @@ -445,6 +514,61 @@ the target is verified to exist before it reports success: catalog.datamove("bwd.draft.bw", "bwd.versions.v2026", idempotency_key=KEY) ``` +### Move a whole folder somewhere else + +Sub-folders, tables and views together — the views rewritten so the copy points +at its own tables rather than back at the originals. Look before you leap: + +```python +from eea_datalakehouse.catalog import foldermove + +report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", + idempotency_key=KEY, dry_run=True) +print(report.summary()) # every folder, table (with its row count) and rewritten view +``` + +Nothing was written. When the plan reads right, drop `dry_run`: + +```python +report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", + idempotency_key=KEY, on_progress=print) +print(report.summary()) +report.verified, report.source_deleted +``` + +What it actually does, in this order: + +1. **Folders** first, shallowest first — including empty ones, which + `INFORMATION_SCHEMA` cannot see and only the REST catalog knows about. +2. **Tables** with `datacopy`, then `COUNT(*)` **on both sides**. A table whose + counts disagree fails the whole move. +3. **Views** recreated from their own SQL, with every reference into the moved + subtree remapped to the new location, in dependency order (a view over + another view is created after it). They are counted on both sides too. +4. **Only then** the source is taken apart — views, tables, folders + deepest-first, then the source folder itself. + +**If anything fails, the source is left exactly where it is** and +`report.verified` is `False`. Fix what the report names and run it again: +tables that already landed with a matching count are skipped, so a re-run is +cheap and safe. To copy without ever removing the source, call `foldercopy` +with the same arguments. + +Worth knowing: + +- **`overwrite=False` by default.** A target table that already exists with a + *different* row count fails that entry rather than being silently accepted. + Pass `overwrite=True` to replace it. +- **`report.summary()` is the thing to read** — per entry: what happened, both + row counts, and any note about how a view's SQL was rewritten. +- **The view rewrite is textual**, not a parse of Dremio's SQL dialect. It + never touches string literals or comments, and only rewrites chains it can + resolve. Read the rewritten definitions in a dry run before applying a + subtree of complicated views. +- **Dremio wiki and tags travel with each entry** (`copy_metadata=True`), + best-effort — a wiki that will not copy is a note, not a failure. The **DDS** + `meta.json` sidecar is a different system and is *not* carried. + ### Delete a view `DROP VIEW IF EXISTS` — safe to re-run, and it fails loudly rather than dropping @@ -606,12 +730,18 @@ begin → upload (all files, in parallel) → commit server validates the target *here*, so an unauthorised or non-existent catalog folder fails before a single byte moves. It answers with an [`S3Plan`](#models): a bucket, a key prefix, and one presigned - [`UploadTarget`](#models) per file. + [`UploadTarget`](#models) per file — and, from a server with contract v0.8, + a [`BeginPlan`](#models) saying what commit will do, which `run()` shows + before uploading. 2. **upload** — each file's bytes go straight from your machine to object - storage using the target exactly as issued. Small files use a presigned POST - policy (or a legacy presigned PUT); large ones use S3 multipart with a + storage using the target exactly as issued. Small files use a presigned PUT + (older servers issue a POST policy); large ones use S3 multipart with a presigned URL per part. The DDS bearer token is **never** sent on these - requests — each presigned URL carries its own auth. + requests — each presigned URL carries its own auth. Where S3 cannot be + reached from your kernel the bytes go through DDS instead — see + [`upload_mode`](#upload_mode--how-the-bytes-travel). If an upload fails, + `run()` cancels the session before raising, so the files that did land are + deleted rather than left behind. 3. **`commit`** — the server loads what landed into the table. This is where the Dremio work happens (`CREATE TABLE` + `COPY INTO` on a managed catalog), so it is the step most likely to fail, and the one @@ -635,7 +765,13 @@ FolderIngest( parallelism=4, idempotency_key=None, multipart=None, + table_definition=None, # TableDefinition | dict + cleanup_on_failure=False, + csv_options=None, # dict, sent only with data_format="csv" + json_options=None, # dict, sent only with data_format="json" + infer_schema=True, show_progress=True, + upload_mode="auto", # "auto" | "direct" | "proxy" client=None, # inject an IngestClient (tests, custom transport) base_url=None, # else DDS_BASE_URL creds=None, # else _DREMIO_USER / _DREMIO_PWD @@ -654,7 +790,12 @@ FolderIngest( | `parallelism` | Concurrent uploads (a `ThreadPoolExecutor`). Must be ≥ 1 — `ValueError` otherwise. | | `idempotency_key` | Passed to `begin` so a repeated call replays the same session rather than starting a second one. **Dropped automatically** when `retry()` has to start over, or the server would just replay the failed session. | | `multipart` | `True`/`False` forces the server's multipart decision; `None` leaves it to the server. | -| `show_progress` | `False` suppresses the progress bar entirely. | +| `table_definition` | The table's column types (and, for later servers, partitioning and sorting) — a `TableDefinition` or a dict in the contract's shape. See [Table definitions](#table-definitions-csvjson-options-inferred-columns). | +| `cleanup_on_failure` | After a commit failure the server *could* retry, cancel the session — deleting its uploaded files — instead of keeping it for `retry()`. Default `False`: kept. With it, a `retry()` afterwards uploads again under a new session. | +| `csv_options` / `json_options` | Parsing hints sent on begin, each only for its own format. CSV: `delimiter`, `quote`, `header` — DDS also accepts `encoding` but passes it on to nothing, so anything other than UTF-8 is refused (see [CSV must be UTF-8](#table-definitions-csvjson-options-inferred-columns)). JSON: `format`, `"lines"` or `"array"`. Inference uses them too. | +| `infer_schema` | For CSV/JSON into an `editable` table without a `table_definition`: infer the columns locally from every file and send them. `False` turns that off. | +| `show_progress` | `False` suppresses the progress bar, and the plan, inferred columns and `diagnose()` lines that are otherwise printed. | +| `upload_mode` | `"auto"` follows the server; `"direct"` uploads only to S3; `"proxy"` sends every file through DDS. `ValueError` otherwise. See [`upload_mode`](#upload_mode--how-the-bytes-travel). | | `client` / `base_url` / `creds` | Injection points. Pass `client=` and the object never builds its own (and never closes yours). Otherwise `base_url` defaults to `DDS_BASE_URL` and `creds` to `load_creds()`. | Credentials live in the `IngestClient`, never on `FolderIngest` itself. @@ -663,12 +804,14 @@ Credentials live in the `IngestClient`, never on `FolderIngest` itself. | Method | Does | |---|---| -| `run() -> IngestOutcome` | The whole `begin → upload → commit` flow. Leaves the client open so you can still `status()`/`retry()`/`cancel()`. | +| `run() -> IngestOutcome` | The whole `begin → upload → commit` flow. Cancels the session if an upload fails. Leaves the client open so you can still `status()`/`retry()`/`cancel()`. | | `begin() -> BeginResult` | Step 1 alone. Raises `FileNotFoundError` if the scan found no files of `data_format`. Sets `session_id`. | +| `dry_run() -> DryRunResult` | Every check `begin` runs, and what commit would do — **nothing uploaded, no session**. See [Checking a transfer first](#checking-a-transfer-first-dry_run-and-diagnose). | +| `diagnose() -> Diagnosis` | Plain-language checks, one per line, ending with a dry run. See [Checking a transfer first](#checking-a-transfer-first-dry_run-and-diagnose). | | `commit(*, multipart_etags=None) -> CommitResult` | Step 3 alone. `IngestStateError` if nothing has begun. | | `status() -> StatusResult` | Server-side state of this transfer. | | `estimate() -> EstimateResult` | Row count + size class of the staged data, between upload and commit. | -| `retry() -> IngestOutcome` | Resume or re-run — see [below](#managing-resuming-and-abandoning-a-transfer). | +| `retry(*, table_definition=None) -> IngestOutcome` | Resume or re-run, optionally with corrected columns — see [below](#managing-resuming-and-abandoning-a-transfer). | | `cancel() -> None` | Delete the staged data and drop the session. Does **not** drop a table an earlier successful commit already created. Clears `session_id`. | | `close() -> None` | Release the HTTP client, but only if this object created it. | | `session_id` (property) | `None` until `begin()` runs or `attach()` supplies one. | @@ -731,6 +874,119 @@ FolderIngest( nothing: `scan_folder` keeps sub-folders and the server preserves the original `rel_path`. +## Table definitions, CSV/JSON options, inferred columns + +A table definition says what the table should look like. Every field is +optional, and only the fields you set are sent: + +```python +from eea_datalakehouse.dds_ingestion import ColumnDef, TableDefinition + +TableDefinition( + columns=(ColumnDef("station_id", "VARCHAR"), ColumnDef("value", "DOUBLE")), + partition_by=(), # accepted; the plan says whether this server applies it + sort_by=(), + version=None, # None → version 1 +) +``` + +A dict in the contract's shape works anywhere a `TableDefinition` does: +`{"columns": [{"name": "value", "type": "DOUBLE"}]}`. Types are Dremio's +(`VARCHAR`, `BIGINT`, `DOUBLE`, `DECIMAL(10,2)`, `TIMESTAMP`, …). +`ColumnDef.nullable` and `description` are accepted by the server but not +applied yet. + +**Where it goes.** On `begin`. A server with contract v0.8 answers with a plan +(`BeginResult.plan`); `run()` prints `plan.describe()` — the sentence the +JupyterLab extension shows — and logs a warning for each part of the definition +this server does not apply (`plan.definition_ignored`). A server without plans +ignored it, so it is sent again on `commit`. + +**When you give none — inferred columns.** For CSV or JSON into an `editable` +table (the one case where the server builds the table from columns), the client +reads every file with pyarrow, widens the types across files, prints them as +`Inferred columns: …`, and sends them as the definition. `csv_options` and +`json_options` are used for that reading too. A file that cannot be inferred — a +JSON *array*, for one — logs a warning and sends nothing; the server then asks +for the columns. `infer_schema=False` turns inference off. Parquet needs none: +its files carry their own schema. + +**CSV must be UTF-8.** Dremio reads CSV as UTF-8 and DDS passes no encoding on +to it, so a Latin-1 or Windows-1252 file would load garbled — and inference alone +would read its text columns as binary. The client refuses such a file before +anything is uploaded: a `csv_options` `encoding` other than UTF-8 (however +spelled: `utf8`, `UTF-8` are fine), or, while inferring columns, a file that is +not valid UTF-8, which the error names. Save the files as UTF-8 first. + +**Parquet with columns given.** They are checked against the files' own footers +before anything is uploaded, with the server's comparison rules. A mismatch +raises `IngestApiError` (422, `error_slug="schema_mismatch"`) in the server's +wording, and `where` says it was the local check. The server checks again at +commit and stays the authority. + +**After a failure.** `retry(table_definition=…)` sends corrected columns with +the retry — see [Managing](#managing-resuming-and-abandoning-a-transfer). + +## `upload_mode` — how the bytes travel + +Two parties decide: `FolderIngest(upload_mode=…)` and the server's hint in +`BeginResult.upload_mode` (DDS's `DDS_INGEST_UPLOAD_MODE`). + +| `FolderIngest(upload_mode=…)` | the server says | the files go | +|---|---|---| +| `"auto"` (default) | `"proxy"` | all through DDS (`POST /api/v1/ingest/stage`) | +| `"auto"` | `"auto"` — DDS's default | to S3; a file whose upload fails at the network layer goes through DDS instead | +| `"auto"` | `"direct"` — also what no hint (an older server) reads as | to S3 only | +| `"direct"` | anything | to S3 only | +| `"proxy"` | anything | all through DDS | + +- **Only a network-layer failure falls back** — an unreachable endpoint, DNS, TLS, + a timeout. An HTTP error from S3 is a signing or permission problem that DDS + would not fix, so it is raised. +- **`/stage` has its own size cap.** DDS checks it only when each file arrives, + so the client asks first (`GET /api/v1/ingest/limits`, at most once per run and + only when a file may go through DDS). With `"proxy"`, a file over the cap is + refused before anything is uploaded. With the fallback, such a file is not + sent through DDS, and the network error carries a note saying why. A server + without that endpoint is not checked locally. +- A file the server planned as multipart goes through DDS whole. +- `IngestOutcome.upload_mode` is `"proxy"` when every file went through DDS, else + `"direct"`; `IngestOutcome.files_staged` counts the files that went through + DDS, so a fallback shows. + +## Checking a transfer first: `dry_run()` and `diagnose()` + +Neither uploads anything or leaves a session behind. + +**`dry_run() -> DryRunResult`** runs every check `begin` runs — privileges, the +target folder, storage, collisions, the table definition — and returns what +commit would do: `plan` (a `BeginPlan`), `key_prefix` and `collision`. The plan +is printed too. It raises what a real `begin` would raise. A DDS server without +dry runs would open a real session instead; the client cancels it straight away +and raises `UnsupportedByServerError`. + +**`diagnose() -> Diagnosis`** runs these checks in order and prints one line +each (`✓` passed, `✗` failed, `–` skipped, with the reason): + +| Step | Checks | Skipped when | +|---|---|---| +| DDS reachable | `GET /health` | — if it fails, every step after it is skipped | +| Credential accepted | `GET /api/v1/ingest/limits`, which needs a login (`GET /api/v1/ingest` on a server without it) | — | +| File sizes | each file against `max_bytes` (what begin accepts) and `stage_max_bytes` (what `/stage` accepts; failing only with `upload_mode="proxy"`) | DDS reported no limits | +| Storage write test | DDS's own S3 write test (`POST /storage-selftest`) | no verdict yet, or an older server | +| Dry run of this folder | everything `dry_run()` checks, including WRITE on the target | a server without dry runs | + +``` +✓ DDS reachable: the service answers +✓ Credential accepted: DDS accepts your Dremio credential +✓ File sizes: within DDS's limits (largest: sub/b.parquet, 7 bytes) +✓ Storage write test: DDS can write to its S3 storage +✓ Dry run of this folder: Importing as a new editable table at bio/uploads/t. +``` + +`Diagnosis.steps` holds one `DiagnosisStep(name, ok, detail)` per line, `ok` +being `None` for a skipped step, and `Diagnosis.ok` is true when no step failed. + ## Managing, resuming and abandoning a transfer ```python @@ -752,8 +1008,22 @@ with FolderIngest(folder="./my_data", target_catalog_path="catalog.theme.sub", | failed, not resumable, staged placement | Opens a fresh session and uploads the folder again, dropping any `idempotency_key` first. | | `done` | `IngestStateError` — already succeeded, nothing to retry. | | `pending` / `uploading` / `committing` | `IngestStateError` — wait for it to finish. | +| — this object's own `run()` cancelled the session: after a failed **upload**, or after a failed commit with `cleanup_on_failure=True` | Uploads again under a new session when the failure can pass — a network error, a 5xx, or DDS saying `retryable` — and the cancel went through. Otherwise `IngestStateError`, quoting the error. The server is not asked: that session is gone, or never got all its files. | | no session at all | `IngestStateError`. | +**With corrected columns.** `retry(table_definition=…)` replaces the +definition this transfer was given. A resumed load uses it without uploading +again — the fix for a `schema_required` or `schema_mismatch` failure (DI-12.6; a +server before that ignores it) — and an upload under a new session sends it on +`begin`. + +**What a failure leaves behind.** A failed *upload* cancels the session before +the error reaches you, deleting the files that did land; the error's note says +so, or says the cancel failed and the server's sweep will remove them. A failed +*commit* keeps the session for `retry()` when the server says it can be retried, +unless `cleanup_on_failure=True`; one it cannot retry the server has already +cleaned up. + > ⚠ **A transfer whose files are stored permanently is never re-uploaded > blindly.** There the upload landed in the table's own folder and stayed, so a > fresh session would add a *second* copy: the server numbers an incoming name @@ -796,6 +1066,8 @@ class IngestOutcome: files_skipped: int etags: dict[str, str] # rel_path -> single-shot ETag (multipart entries are not here) resumed: bool = False + upload_mode: str = "direct" # "proxy" when every file went through DDS + files_staged: int = 0 # files that went through DDS: all with "proxy", the fallbacks otherwise ``` ```python @@ -827,16 +1099,20 @@ IngestClient(base_url, creds, *, timeout=60.0, http_client=None) | Method | Endpoint / action | |---|---| -| `begin(*, target_catalog_path, intent, data_format, conflict_mode, files, table_name=None, sub_path=None, idempotency_key=None, multipart=None)` | `POST /api/v1/ingest/begin` → `BeginResult` | +| `begin(*, target_catalog_path, intent, data_format, conflict_mode, files, table_name=None, sub_path=None, idempotency_key=None, multipart=None, definition=None, csv_options=None, json_options=None)` | `POST /api/v1/ingest/begin` → `BeginResult`. A server that took `definition` returns `BeginResult.plan`. Only the fields that were set are sent. | +| `dry_run(*, …)` | `POST /api/v1/ingest/begin` with `dry_run: true` → `DryRunResult`. Takes `begin`'s arguments except `idempotency_key` and `multipart`. Against a server without dry runs: cancels the session it opened and raises `UnsupportedByServerError`. | | `commit(*, session_id, definition=None, multipart_etags=None)` | `POST /api/v1/ingest/commit` → `CommitResult` | | `get_status(session_id)` | `GET /api/v1/ingest/{id}` → `StatusResult` | | `list_sessions(state=None)` | `GET /api/v1/ingest` → `list[StatusResult]`. `state` filters server-side: `"active"`, `"failed"`, `"all"`. | -| `retry(session_id)` | `POST /api/v1/ingest/{id}/retry` → `StatusResult` | +| `retry(session_id, *, definition=None)` | `POST /api/v1/ingest/{id}/retry` → `StatusResult`. With `definition`, the body `{"definition": …}` replaces the stored one (DI-12.6); without it, no body, as before. | | `cancel(session_id)` | `DELETE /api/v1/ingest/{id}` | | `estimate(session_id)` | `POST /api/v1/ingest/estimate` → `EstimateResult`. On a managed catalog the count comes from staged Parquet footers; CSV/JSON have none and report `0`. | | `stage(session_id, rel_path, data)` | `POST /api/v1/ingest/stage` → `StageResult`. Uploads one file **through the server** — the fallback when the S3 endpoint is not reachable from this kernel. Slower (the bytes transit the server), so prefer `upload_file`. | -| `upload_file(target, data)` | To S3, not DDS. Presigned POST policy (form `fields` first, `file` part **last**, so S3 honours the policy conditions) or legacy presigned PUT. Returns the `ETag` header or `None`. | +| `upload_file(target, data)` | To S3, not DDS. Presigned PUT with the target's signed `headers` — the normal path since contract v0.7 — or, from an older server, a POST policy (form `fields` first, `file` part **last**, so S3 honours the policy conditions). Returns the `ETag` header or `None`. An `httpx.TransportError` (unreachable, TLS, timeout) passes through unchanged. | | `upload_file_multipart(target, data)` | To S3. Splits `data` into `len(target.parts)` chunks (ceil division; last part takes the remainder) and `PUT`s each to its presigned part URL. Returns `[{"part_number", "etag"}, …]` for `commit`'s `multipart_etags`. Raises if S3 omits an ETag for any part. | +| `health()` | `GET /health` (no login); raises unless DDS answers. | +| `limits(runner="notebook")` | `GET /api/v1/ingest/limits` → dict: `max_bytes` (the largest file begin accepts from this kind of client), `stage_max_bytes` (through `/stage`), `multipart_threshold`. Needs a login. | +| `storage_selftest()` | `POST /storage-selftest` (no login) → dict; `storage_write_test.ok` says whether DDS can write to its S3. | | `close()` | Closes the `httpx.Client` **only if** the client created it. Also a context manager. | Both upload methods enforce `target.max_bytes` locally (a `413` @@ -856,7 +1132,11 @@ working when the server adds fields. | `UploadPart` | `part_number`, `url` | | `UploadTarget` | `rel_path`, `url`, `method="POST"`, `fields`, `headers`, `max_bytes`, `upload_id`, `parts`; `is_multipart` (true when `upload_id` **and** `parts` are set) | | `S3Plan` | `bucket`, `key_prefix`, `uploads: tuple[UploadTarget, ...]` | -| `BeginResult` | `session_id`, `status`, `s3: S3Plan`, `collision` | +| `BeginResult` | `session_id`, `status`, `s3: S3Plan`, `collision`, `upload_mode` (the server's hint; `"direct"` when it sends none), `plan: BeginPlan \| None` | +| `BeginPlan` | `placement`, `table_path`, `storage_path`, `action` (`"create"`, `"append"`, `"replace"`, `"fail"`), `definition_source`, `definition_applied`, `definition_ignored`; `describe()` — one sentence, the extension's wording | +| `DryRunResult` | `plan: BeginPlan`, `key_prefix`, `collision` | +| `TableDefinition` | `columns: tuple[ColumnDef, ...]`, `partition_by`, `sort_by`, `version`; `as_payload()` sends only what was set | +| `ColumnDef` | `name`, `type` (a Dremio type), `nullable`, `description` | | `CommitResult` | `session_id`, `status`, `table_path`, `record_count`, `storage_path` | | `Progress` | `files_done`, `files_total`, `step` | | `StatusResult` | `status`, `progress`, `raw` — plus the properties below | @@ -883,6 +1163,9 @@ client does not model yet is still reachable. | `error` | why it failed, including the stage (`"Dremio load failed: …"`, `"S3 upload failed: …"`). `None` unless failed. | | `is_terminal` | status is `done`, `failed` or `cancelled` | | `is_resumable` | failed **and** the server's own `resumable` flag is truthy | +| `failed_stage` | where a failed transfer broke: `"upload"` or `"load"` | +| `error_code` | the failure's slug, e.g. `"schema_required"` (contract v0.8) | +| `describe_failure()` | one sentence on the failure — stage, slug, reason, and what a retry would do — worded exactly as the JupyterLab Transfers panel words it | Lifecycle: `pending` → `uploading` → `committing` → `done`, or `failed` / `cancelled`. @@ -920,9 +1203,22 @@ RuntimeError ├── IngestStateError asked for locally-impossible state, before any HTTP call └── IngestApiError a DDS call returned non-success ├── StorageUnavailableError DDS cannot write to its own S3 (503) + ├── UnsupportedByServerError the DDS server predates the feature (e.g. dry runs) └── S3UploadError a presigned upload failed — NOT a DDS error ``` +**What every `IngestApiError` tells you**, besides its message: + +| Attribute | Meaning | +|---|---| +| `error_slug` | DDS's machine-readable `error`, e.g. `schema_required`, `schema_mismatch`; `""` when the body had none | +| `retryable` | whether repeating — for a failed transfer, `retry()` — can succeed unchanged; `None` from a server before contract v0.8 | +| `failed_stage` | on a failed commit from `FolderIngest`: `"upload"` or `"load"`, from the session; `None` otherwise | + +A failed commit also carries the session's own failure text as an exception +note — `StatusResult.describe_failure()`, the sentence the Transfers panel +shows. A failed upload carries a note on what happened to its session. + **`IngestApiError`** carries `status_code`, `message` and `where` — the call that failed, e.g. `POST /api/v1/ingest/begin`. `where` is in the message because a transfer makes several calls to two different systems, and "API error 500" alone @@ -947,9 +1243,14 @@ an unreachable endpoint, a gateway error. It adds `rel_path` and `url`, and its message names both, so you can tell "your upload never landed" from "the Document Service refused the request". +**`UnsupportedByServerError`** — the DDS server is older than the feature a call +needs (e.g. `dry_run()`). A subclass of `IngestApiError`, so code that catches +that keeps working. + **`IngestStateError`** is raised locally, before any HTTP call — committing a transfer that never began, retrying one that is still running, cancelling -nothing. +nothing, or retrying an upload that would fail the same way (its message quotes +the upload error). ## Notebook helpers (`dds_ingestion.common`) @@ -1138,10 +1439,20 @@ created. Nothing in the library fans an operation out over a folder's contents to convert a whole folder, loop over `gettablesfrom`: [recipe](#turn-every-table-in-a-folder-into-views-in-another-folder). +**`createviewas(view_path, sql, *, overwrite=False, create_target_folder=True)`** +— `table2view`'s general form: a view over any query body you hand it, not just +`SELECT * FROM `. `sql` is the body only (no `CREATE VIEW` prefix) and is +sent through unchanged — quoting and correctness are yours, since the text +usually comes back out of Dremio itself. `overwrite=True` drops whatever is +actually at `view_path` first, detecting whether it is a table or a view. + **`deleteview(view_path)`** — `DROP VIEW IF EXISTS`. Idempotent. If `view_path` is actually a table, this fails loudly rather than dropping the wrong kind of entry. +**`deletetable(table_path)`** — `DROP TABLE IF EXISTS`; `deleteview`'s +counterpart, failing just as loudly on the wrong kind of entry. + **`draft2version(draft_path, version_path)`** and **`publishversion(consumer_view_path, version_path)`** — **not implemented**; both raise `NotImplementedError`. See [Not implemented yet](#not-implemented-yet). @@ -1182,6 +1493,65 @@ Shared behaviour: for a no-result-rows DDL statement are unverified here — so it checks `target_path` actually exists in the catalog before declaring success. +### Whole folders — `catalog.foldertree` + +`folderscan` / `foldercopy` / `foldermove` (also methods on `Catalog`). These +are orchestration over the operations above rather than one statement each, so +they live in their own module — see its docstring for the assumptions they rest +on, and the [recipe](#move-a-whole-folder-somewhere-else) for how to drive them. + +**`folderscan(catalog, source_path, target_path) -> FolderTree`** — read-only. +Every sub-folder, table and view under `source_path`, mapped to where it would +land, with each view's SQL already rewritten and the views in dependency order. +Raises if the two paths are the same, or if either is inside the other. + +```python +@dataclass(frozen=True, slots=True) +class TreeEntry: + source_path: str + target_path: str + kind: Literal["FOLDER", "TABLE", "VIEW"] + view_sql: str | None # views: as Dremio stores it + view_context: tuple[str, ...] # views: its sqlContext, if known + remapped_sql: str | None # views: what will actually be created + depends_on: tuple[str, ...] # views: what it selects from, in this subtree + notes: tuple[str, ...] +``` + +**`foldercopy(catalog, source_path, target_path, *, dry_run=False, +overwrite=False, skip_existing=True, verify_views=True, copy_metadata=True, +qualify_external=True, tree=None, on_progress=None) -> FolderTransferReport`** +— reproduces the subtree and proves it. Folders first (empty ones included), +then tables via `datacopy` with `COUNT(*)` compared on both sides, then views +recreated from their own SQL with references into the subtree remapped. A +per-entry failure is recorded, not raised — the sweep finishes so you see the +whole picture. + +`skip_existing=True` is what makes a re-run cheap: a target table already there +**with the source's row count** is reported `skipped`. One with a *different* +count fails that entry rather than being silently accepted. + +**`foldermove(catalog, source_path, target_path, *, delete_source_root=True, +…)`** — `foldercopy`, then removal of the source **only if `report.verified`**: +views in reverse dependency order, then tables, then folders deepest-first. +`deletefolder(cascade=True)` is deliberately not used — a folder that will not +delete because something unexpected is inside it is a fact worth surfacing. + +**`FolderTransferReport`** — `folders` / `tables` / `views` are lists of +`EntryResult(entry, status, source_rows, target_rows, sql, error, notes)`, where +`status` is `planned` / `created` / `copied` / `skipped` / `failed`. +`report.verified` is the green light, `report.failures` is what to fix, +`report.deleted` and `report.delete_errors` say what came off the source, and +`report.summary()` prints the lot. + +**`remap_view_sql(sql, *, source_path, target_path, entries, context=(), +external_exists=None) -> RemapResult`** — the view rewriter on its own. Textual, +never a parse of Dremio's dialect: it skips string literals and comments, and +rewrites only identifier chains it can resolve — against a known entry, against +the source root, or against the view's own `sqlContext`. Its docstring lists the +four rules in order, and the one corner it can get wrong. + + ### Discovery **`gettablesfrom(schema_path) -> list[str]`** — every table and view under @@ -1191,6 +1561,35 @@ since SQL `LIKE`'s `%` matches across dots), with `_`/`%` in real folder names escaped so an underscore is not read as a wildcard. Full paths rather than bare names because two subfolders can each hold a table of the same name. +**`getentriesfrom(schema_path) -> list[CatalogEntry]`** — the same sweep, with +`TABLE_TYPE` selected as well, so each path carries whether it is a `"TABLE"` or +a `"VIEW"`. Sorted by path. This is what a folder-level operation needs: copying +a view with `CREATE TABLE … AS SELECT` silently materialises it into a table. + +**`entryexists(path) -> bool`** — whether `path` is a table or view Dremio knows +about, via `INFORMATION_SCHEMA` (so REST or Flight both answer). Fetches no rows, +so it costs nothing on a large table. Unlike `CatalogRestClient.exists`, it does +not see folders, spaces or sources. + +**`getviewsql(view_path) -> ViewDefinition`** — the SQL a view is defined by. + +```python +@dataclass(frozen=True, slots=True) +class ViewDefinition: + sql: str # the query body, as Dremio stores it + context: tuple[str, ...] # the sqlContext it was written against; () = unknown +``` + +Two sources, better one first: Dremio's own catalog entity (which carries the +`sqlContext` as well as the text), then `INFORMATION_SCHEMA."VIEWS"` (text only, +so `context` comes back empty — meaning *unknown*, not *none*). Raises if +neither answers, rather than letting a caller replace a view with a guess. + +**`getfoldersfrom(path, *, recursive=True) -> list[str]`** — the folders under +`path`, shallowest first (the order they have to be created in on the other +side). REST-only: folders are not in `INFORMATION_SCHEMA`, so an **empty** folder +is invisible to `gettablesfrom`/`getentriesfrom` and can only be found this way. + **`gettableitemsfrom(table_path) -> TableInfo`** — one table or view's schema and row count, **without fetching any rows**. diff --git a/pyproject.toml b/pyproject.toml index bbe97be..ba6850e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "EEADataLakehouse" -version = "0.1.31" +version = "0.1.33" description = "Data preparation and validation utilities for the EEA data lakehouse pipeline." readme = "README.md" requires-python = ">=3.11" diff --git a/src/eea_datalakehouse/catalog/__init__.py b/src/eea_datalakehouse/catalog/__init__.py index 3f41a1e..eb44604 100644 --- a/src/eea_datalakehouse/catalog/__init__.py +++ b/src/eea_datalakehouse/catalog/__init__.py @@ -5,12 +5,19 @@ catalog.draft2version("bwd.draft.bw", "bwd.versions.v1", idempotency_key="bwd-v2025_1") table2view, draft2version, publishversion, datacopy, datamove, deleteview, -gettablesfrom, gettableitemsfrom, getwikifrom, gettagsfrom, setwikito, +deletetable, createviewas, gettablesfrom, getentriesfrom, gettableitemsfrom, +entryexists, getviewsql, getfoldersfrom, getwikifrom, gettagsfrom, setwikito, deletewiki, setmeta2wiki, getmetafromwiki, settagsto, deletetags, createfolder, deletefolder — over REST (the default) or Arrow Flight (opt in via EEA_CATALOG_TRANSPORT=flight in .env), with retry-later error handling for a Dremio engine that's still starting up. +A whole folder at a time — sub-folders, tables and views together — lives in +`foldertree`: `folderscan` plans it, `foldercopy` reproduces it and proves +the copy by row count, `foldermove` does that and then removes the source, +but only once the copy verified. Views are rewritten to point at the copies +rather than back at the originals (`remap_view_sql`). + The module-level functions in `operations` (table2view(executor, ...) etc.) are what `Catalog`'s methods delegate to — call them directly if you'd rather manage the executor yourself. @@ -20,22 +27,41 @@ from .client import Catalog from .errors import CatalogAuthError, CatalogOperationError, EngineStartingError +from .foldertree import ( + EntryResult, + FolderTransferReport, + FolderTree, + RemapResult, + TreeEntry, + foldercopy, + foldermove, + folderscan, + remap_view_sql, +) from .operations import ( + CatalogEntry, TableInfo, VersionResult, + ViewDefinition, createfolder, createversion, + createviewas, datacopy, datamove, deletefolder, + deletetable, deletetags, deleteview, deletewiki, draft2version, + entryexists, + getentriesfrom, + getfoldersfrom, getmetafromwiki, gettableitemsfrom, gettablesfrom, gettagsfrom, + getviewsql, getwikifrom, publishversion, retry_pending, @@ -68,6 +94,7 @@ "Catalog", "CatalogAuthError", "CatalogCommitError", + "CatalogEntry", "CatalogListing", "CatalogOperationError", "CatalogRestClient", @@ -75,27 +102,43 @@ "CatalogSessionError", "CommitReport", "EngineStartingError", + "EntryResult", "FlightSqlExecutor", + "FolderTransferReport", + "FolderTree", + "RemapResult", "RestSqlExecutor", "SqlExecutor", "SqlResult", "TableInfo", + "TreeEntry", "VersionResult", + "ViewDefinition", "createfolder", "createversion", + "createviewas", "datacopy", "datamove", "deletefolder", + "deletetable", "deletetags", "deleteview", "deletewiki", "draft2version", + "entryexists", + "foldercopy", + "foldermove", + "folderscan", + "getentriesfrom", + "getfoldersfrom", "gettableitemsfrom", "gettablesfrom", "gettagsfrom", + "getviewsql", "getwikifrom", "publishversion", "getmetafromwiki", + "remap_view_sql", "resolve_executor", "retry_pending", "setmeta2wiki", diff --git a/src/eea_datalakehouse/catalog/client.py b/src/eea_datalakehouse/catalog/client.py index 1e7d9bb..cc3caf5 100644 --- a/src/eea_datalakehouse/catalog/client.py +++ b/src/eea_datalakehouse/catalog/client.py @@ -37,8 +37,9 @@ import pyarrow as pa -from . import operations, retry_state -from .operations import TableInfo, VersionResult +from . import foldertree, operations, retry_state +from .foldertree import FolderTransferReport, FolderTree, ProgressCallback +from .operations import CatalogEntry, TableInfo, VersionResult, ViewDefinition from .rest import CatalogRestClient from .sql import ( DEFAULT_TIMEOUT, @@ -323,6 +324,48 @@ def open_flight_channel(self) -> FlightSqlExecutor: each (see `FlightSqlExecutor.fork`). The caller closes it.""" return self._flight_executor.fork() + def getentriesfrom(self, schema_path: str, *, idempotency_key: str) -> list[CatalogEntry]: + return operations.getentriesfrom( + self._executor, schema_path, idempotency_key=idempotency_key + ) + + def entryexists(self, path: str, *, idempotency_key: str) -> bool: + return operations.entryexists(self._executor, path, idempotency_key=idempotency_key) + + def getviewsql(self, view_path: str, *, idempotency_key: str) -> ViewDefinition: + return operations.getviewsql( + self._executor, + view_path, + catalog_rest=self._catalog_rest, + idempotency_key=idempotency_key, + ) + + def createviewas( + self, + view_path: str, + sql: str, + *, + overwrite: bool = False, + create_target_folder: bool = True, + idempotency_key: str, + ) -> SqlResult: + return operations.createviewas( + self._executor, + view_path, + sql, + overwrite=overwrite, + create_target_folder=create_target_folder, + catalog_rest=self._catalog_rest, + idempotency_key=idempotency_key, + ) + + def getfoldersfrom( + self, path: str, *, recursive: bool = True, idempotency_key: str + ) -> list[str]: + return operations.getfoldersfrom( + self._catalog_rest, path, recursive=recursive, idempotency_key=idempotency_key + ) + def getwikifrom(self, path: str, *, idempotency_key: str) -> str: return operations.getwikifrom(self._catalog_rest, path, idempotency_key=idempotency_key) @@ -394,6 +437,92 @@ def deletefolder(self, path: str, *, cascade: bool = False, idempotency_key: str idempotency_key=idempotency_key, ) + # --- whole folders: scan / copy / move ------------------------------- + # These are orchestration over the operations above rather than one SQL + # statement each, so they live in `foldertree` and are exposed here for + # a caller who already has a Catalog. See that module's docstring for + # what a move actually does, and in what order. + + def folderscan( + self, + source_path: str, + target_path: str, + *, + idempotency_key: str, + qualify_external: bool = True, + on_progress: ProgressCallback | None = None, + ) -> FolderTree: + return foldertree.folderscan( + self, + source_path, + target_path, + idempotency_key=idempotency_key, + qualify_external=qualify_external, + on_progress=on_progress, + ) + + def foldercopy( + self, + source_path: str, + target_path: str, + *, + idempotency_key: str, + dry_run: bool = False, + overwrite: bool = False, + skip_existing: bool = True, + verify_views: bool = True, + copy_metadata: bool = True, + qualify_external: bool = True, + tree: FolderTree | None = None, + on_progress: ProgressCallback | None = None, + ) -> FolderTransferReport: + return foldertree.foldercopy( + self, + source_path, + target_path, + idempotency_key=idempotency_key, + dry_run=dry_run, + overwrite=overwrite, + skip_existing=skip_existing, + verify_views=verify_views, + copy_metadata=copy_metadata, + qualify_external=qualify_external, + tree=tree, + on_progress=on_progress, + ) + + def foldermove( + self, + source_path: str, + target_path: str, + *, + idempotency_key: str, + dry_run: bool = False, + overwrite: bool = False, + skip_existing: bool = True, + verify_views: bool = True, + copy_metadata: bool = True, + qualify_external: bool = True, + delete_source_root: bool = True, + tree: FolderTree | None = None, + on_progress: ProgressCallback | None = None, + ) -> FolderTransferReport: + return foldertree.foldermove( + self, + source_path, + target_path, + idempotency_key=idempotency_key, + dry_run=dry_run, + overwrite=overwrite, + skip_existing=skip_existing, + verify_views=verify_views, + copy_metadata=copy_metadata, + qualify_external=qualify_external, + delete_source_root=delete_source_root, + tree=tree, + on_progress=on_progress, + ) + def retry_pending(self, idempotency_key: str) -> SqlResult | list[Any]: pending = retry_state.get(idempotency_key) if pending is None: diff --git a/src/eea_datalakehouse/catalog/foldertree.py b/src/eea_datalakehouse/catalog/foldertree.py new file mode 100644 index 0000000..68233c0 --- /dev/null +++ b/src/eea_datalakehouse/catalog/foldertree.py @@ -0,0 +1,1110 @@ +"""Move or copy a whole folder — sub-folders, tables and views — from one +place in the Dremio catalog to another. + + from eea_datalakehouse.catalog import Catalog, foldermove + + with Catalog(DREMIO_URL, DREMIO_TOKEN, username=DREMIO_USERNAME) as catalog: + report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key=KEY) + print(report.summary()) + +`foldercopy` reproduces the source subtree under the target and proves the +copy landed. `foldermove` is `foldercopy` plus removal of the source — and it +removes **nothing** unless the copy verified clean (`report.verified`). That +order — copy, verify, then delete — is the whole point: a folder move across a +Community-Edition Dremio has no transaction behind it, so the only safe +sequencing is to leave the source standing until the target is proven. + +What each kind of entry needs is different, and that is why this is more than +a loop over `datacopy`: + +* **Folders** are reproduced first, shallowest first, so nothing lands in a + folder that does not exist yet. Empty ones are included — they are invisible + to `INFORMATION_SCHEMA` and only Dremio's REST catalog knows about them. +* **Tables** are copied with `datacopy` (``CREATE TABLE … AS SELECT``, over + Arrow Flight) and then **counted on both sides**: ``COUNT(*)`` on the source + and on the target, which have to agree. A table whose counts disagree fails + the whole move. +* **Views** are *not* copied as data — a ``CREATE TABLE … AS SELECT`` over a + view silently materialises it into a table, which is a different kind of + thing from the original. Each view is recreated from its own SQL with every + reference into the moved subtree **remapped to the new location** (see + `remap_view_sql`), so the copied folder is self-contained and still points at + its own tables rather than back at the ones about to be deleted. Views are + created in dependency order, so a view selecting from another view in the + same subtree is created after it. + +Assumptions worth checking against your actual Dremio deployment before +relying on this — in the same spirit as `operations`' own list: + +* **A view's SQL is read from Dremio's catalog entity where possible** + (`getviewsql`), because that carries the ``sqlContext`` the SQL was written + against as well as the text. Where that lookup is rejected — some source + types answer a nested by-path lookup with a 400, see + `CatalogRestClient.is_folder` — it falls back to + ``INFORMATION_SCHEMA."VIEWS"``, which has the text but no context. With no + context, an unqualified reference inside a view cannot be resolved and is + left exactly as it was; `remap_view_sql` says so in its notes rather than + guessing. +* **The rewrite is textual**, not a parse of Dremio's SQL dialect. It skips + string literals and comments, and only rewrites identifier chains it can + resolve to something it knows about (see `remap_view_sql` for the exact + rules and the one corner it can get wrong). A dry run prints every rewritten + definition — read them before turning `dry_run` off on a subtree of + complicated views. +* **Nothing here is parallel.** `datacopy` is Flight-only and + `FlightSqlExecutor` holds one client with a lazily-cached auth header whose + initialisation is unguarded; this repo has not verified concurrent statement + execution on one Flight client. N tables copy one after another. +* **Re-running is expected to be cheap.** With the default + `skip_existing=True`, a target table that is already there and already has + the source's row count is left alone and reported as ``skipped`` — so a run + interrupted halfway (a cold engine, a dropped kernel) is resumed by simply + running it again, rather than by unpicking what already made it. +* **Deletion is explicit, never a cascade.** The source is taken apart in the + order it was built: views (reverse dependency order), then tables, then + folders deepest-first. `deletefolder`'s own `cascade=True` is deliberately + not used — a folder that will not delete because something unexpected is + still inside it is a fact worth surfacing, not something to bulldoze. +""" + +from __future__ import annotations + +import re +from collections.abc import Callable, Iterator +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Literal + +from .errors import CatalogOperationError +from .operations import _quote_path + +if TYPE_CHECKING: # pragma: no cover - typing only + from .client import Catalog + +EntryKind = Literal["FOLDER", "TABLE", "VIEW"] +EntryStatus = Literal["planned", "created", "copied", "skipped", "failed"] + +ProgressCallback = Callable[[str], None] + +# Identifier chain segment: a double-quoted identifier (with "" as an escaped +# quote inside it) or a bare one. Digits can't start a bare identifier, which +# is what keeps `1.5` from being read as a two-segment chain. +_SQL_IDENT = re.compile(r'"(?:[^"]|"")*"|[A-Za-z_][A-Za-z0-9_$]*') + +# Keywords a dataset reference can follow. Only used to decide whether an +# unresolved chain is worth one existence check to see if it was written +# relative to a context this move is dropping — never to decide a rewrite that +# the chain itself already justifies. +_REFERENCE_KEYWORDS = frozenset({"FROM", "JOIN", "INTO", "TABLE", "UPDATE"}) + +# Enough of Dremio's SQL vocabulary to keep the "unqualified reference" +# warning below from firing on every keyword in an ordinary SELECT. It only +# gates a note; nothing is ever rewritten on the strength of this list. +_SQL_WORDS = frozenset({ + "ALL", "AND", "AS", "ASC", "AVG", "BETWEEN", "BY", "CASE", "CAST", "COALESCE", "COUNT", + "CROSS", "CURRENT_DATE", "CURRENT_TIMESTAMP", "DESC", "DISTINCT", "ELSE", "END", "ESCAPE", + "EXISTS", "EXTRACT", "FALSE", "FROM", "FULL", "GROUP", "HAVING", "IN", "INNER", "INTERVAL", + "INTO", "IS", "JOIN", "LEFT", "LIKE", "LIMIT", "MAX", "MIN", "NATURAL", "NOT", "NULL", + "NULLIF", "OFFSET", "ON", "OR", "ORDER", "OUTER", "OVER", "PARTITION", "RIGHT", "ROW", + "ROWS", "SELECT", "SET", "SUBSTRING", "SUM", "TABLE", "THEN", "TRIM", "TRUE", "UNION", + "UPDATE", "USING", "VALUES", "WHEN", "WHERE", "WITH", +}) + + +@dataclass(frozen=True, slots=True) +class TreeEntry: + """One thing to reproduce at the target: a folder, a table or a view.""" + + source_path: str + target_path: str + kind: EntryKind + #: Views only — the SQL Dremio holds for the view, exactly as stored. + view_sql: str | None = None + #: Views only — the ``sqlContext`` that SQL was written against, if known. + view_context: tuple[str, ...] = () + #: Views only — the rewritten SQL that will be used at the target. + remapped_sql: str | None = None + #: Views only — source paths inside this subtree the view selects from. + depends_on: tuple[str, ...] = () + #: Anything the caller should read before trusting this entry. + notes: tuple[str, ...] = () + + @property + def name(self) -> str: + return self.source_path.rsplit(".", 1)[-1] + + +@dataclass(frozen=True, slots=True) +class FolderTree: + """What `folderscan` found under a source folder, already mapped to targets. + + `views` is in **dependency order**: a view that selects from another view + in this same subtree comes after it, so creating them in list order works. + `folders` is shallowest first, for the same reason. + """ + + source_path: str + target_path: str + folders: tuple[TreeEntry, ...] = () + tables: tuple[TreeEntry, ...] = () + views: tuple[TreeEntry, ...] = () + notes: tuple[str, ...] = () + + @property + def entries(self) -> tuple[TreeEntry, ...]: + """Every entry, in the order a copy has to create them.""" + return self.folders + self.tables + self.views + + def __len__(self) -> int: + return len(self.entries) + + +@dataclass(slots=True) +class EntryResult: + """What actually happened to one `TreeEntry`.""" + + entry: TreeEntry + status: EntryStatus = "planned" + source_rows: int | None = None + target_rows: int | None = None + sql: str | None = None + error: str | None = None + notes: list[str] = field(default_factory=list) + + @property + def counts_match(self) -> bool: + """Both sides counted, and they agree. False while either is unknown.""" + return ( + self.source_rows is not None + and self.target_rows is not None + and self.source_rows == self.target_rows + ) + + +@dataclass(slots=True) +class FolderTransferReport: + """The outcome of a `foldercopy` / `foldermove`, entry by entry. + + `verified` is the green light: every folder reproduced, every table copied + *and* counted equal on both sides, every view created, and nothing failed. + `foldermove` deletes the source only when it is true. + """ + + source_path: str + target_path: str + dry_run: bool = False + folders: list[EntryResult] = field(default_factory=list) + tables: list[EntryResult] = field(default_factory=list) + views: list[EntryResult] = field(default_factory=list) + deleted: list[str] = field(default_factory=list) + delete_errors: list[tuple[str, str]] = field(default_factory=list) + notes: list[str] = field(default_factory=list) + source_deleted: bool = False + + @property + def results(self) -> list[EntryResult]: + return self.folders + self.tables + self.views + + @property + def failures(self) -> list[EntryResult]: + return [result for result in self.results if result.status == "failed"] + + @property + def verified(self) -> bool: + """Whether the target is a proven-good copy of the source. + + A dry run is never verified — nothing was written, so there is nothing + to have proven. Tables must have matching counts on both sides; views + must have been created or deliberately skipped, and have matching + counts too wherever they were checked (`verify_views`). + """ + if self.dry_run or not self.results: + return False + if self.failures: + return False + if any(result.status == "planned" for result in self.results): + return False + for result in self.tables: + if not result.counts_match: + return False + for result in self.views: + if result.source_rows is not None and not result.counts_match: + return False + return True + + def summary(self) -> str: + """A readable report — what moved, what it counted, what went wrong.""" + head = "PLAN" if self.dry_run else "RESULT" + lines: list[str] = [f"{head}: {self.source_path} -> {self.target_path}"] + for label, results in ( + ("folders", self.folders), + ("tables", self.tables), + ("views", self.views), + ): + if not results: + continue + lines.append(f"\n{label} ({len(results)}):") + for result in results: + mark = {"failed": "x", "planned": "-", "skipped": "="}.get(result.status, "+") + counts = "" + if result.source_rows is not None or result.target_rows is not None: + src = "?" if result.source_rows is None else f"{result.source_rows:,}" + tgt = "?" if result.target_rows is None else f"{result.target_rows:,}" + # "->" while a side is still unknown (a plan, say) — only a + # count that was actually taken on both sides can disagree. + both_known = result.source_rows is not None and result.target_rows is not None + agree = "==" if result.counts_match else ("!=" if both_known else "->") + counts = f" [{src} {agree} {tgt} rows]" + lines.append( + f" {mark} {result.entry.source_path} -> {result.entry.target_path}" + f" ({result.status}){counts}" + ) + lines.extend(f" note: {note}" for note in result.notes) + if result.error: + lines.append(f" error: {result.error}") + if self.deleted: + lines.append(f"\nremoved from the source ({len(self.deleted)}):") + lines.extend(f" - {path}" for path in self.deleted) + if self.delete_errors: + lines.append("\ncould not remove:") + lines.extend(f" x {path}: {error}" for path, error in self.delete_errors) + if self.notes: + lines.append("\nnotes:") + lines.extend(f" {note}" for note in self.notes) + tail = f"\nverified: {self.verified}" + if self.dry_run: + tail += " (dry run — nothing was written)" + else: + tail += f" source removed: {self.source_deleted}" + lines.append(tail) + return "\n".join(lines) + + +# -------------------------------------------------------------------------- +# paths +# -------------------------------------------------------------------------- + + +def _segments(path: str) -> list[str]: + return [segment for segment in path.split(".") if segment] + + +def _lower(segments: list[str] | tuple[str, ...]) -> list[str]: + return [segment.lower() for segment in segments] + + +def _remap_path(path: str, source_path: str, target_path: str) -> str: + """`path`, with its `source_path` prefix swapped for `target_path`.""" + remainder = _segments(path)[len(_segments(source_path)) :] + return ".".join(_segments(target_path) + remainder) + + +def _is_inside(path: str, ancestor: str) -> bool: + """Whether `path` is `ancestor` itself, or something under it (case-insensitively).""" + path_segments = _lower(_segments(path)) + ancestor_segments = _lower(_segments(ancestor)) + return path_segments[: len(ancestor_segments)] == ancestor_segments + + +def _unquote_ident(raw: str) -> str: + if raw.startswith('"') and raw.endswith('"') and len(raw) >= 2: + return raw[1:-1].replace('""', '"') + return raw + + +# -------------------------------------------------------------------------- +# view SQL rewriting +# -------------------------------------------------------------------------- + + +@dataclass(frozen=True, slots=True) +class _Token: + """One piece of a SQL string: passthrough text, or an identifier chain.""" + + kind: Literal["text", "chain"] + raw: str + segments: tuple[str, ...] = () + + +def _iter_sql_chains(sql: str) -> Iterator[_Token]: + """Walk `sql`, yielding a `_Token` per piece: text, or an identifier chain. + + String literals (``'…'``, with ``''`` as an escaped quote), line comments + (``-- …``) and block comments (``/* … */``) are passed through as text and + never looked inside — the one hard rule that keeps a path-shaped string + from being rewritten as if it were a reference. + + A *chain* is one or more identifiers joined by dots with no whitespace + between them: ``a.b``, ``"my space"."tbl"``, ``t.col``. Whitespace around + a dot ends the chain, which is a deliberate simplification — Dremio's own + stored view SQL does not write references that way. + """ + index, length = 0, len(sql) + buffer: list[str] = [] + while index < length: + char = sql[index] + if char == "'": + cursor = index + 1 + while cursor < length: + if sql[cursor] == "'": + if cursor + 1 < length and sql[cursor + 1] == "'": + cursor += 2 + continue + cursor += 1 + break + cursor += 1 + buffer.append(sql[index:cursor]) + index = cursor + continue + if sql.startswith("--", index): + cursor = sql.find("\n", index) + cursor = length if cursor == -1 else cursor + buffer.append(sql[index:cursor]) + index = cursor + continue + if sql.startswith("/*", index): + cursor = sql.find("*/", index + 2) + cursor = length if cursor == -1 else cursor + 2 + buffer.append(sql[index:cursor]) + index = cursor + continue + match = _SQL_IDENT.match(sql, index) + if match is None: + buffer.append(char) + index += 1 + continue + + segments: list[str] = [] + cursor = index + while True: + match = _SQL_IDENT.match(sql, cursor) + if match is None: + break + segments.append(_unquote_ident(match.group(0))) + cursor = match.end() + if cursor < length and sql[cursor] == "." and _SQL_IDENT.match(sql, cursor + 1): + cursor += 1 + continue + break + if buffer: + yield _Token("text", "".join(buffer)) + buffer = [] + yield _Token("chain", sql[index:cursor], tuple(segments)) + index = cursor + if buffer: + yield _Token("text", "".join(buffer)) + + +@dataclass(frozen=True, slots=True) +class RemapResult: + """`remap_view_sql`'s answer: the new SQL, and what it had to touch.""" + + sql: str + #: Source paths inside the moved subtree this SQL selects from. + references: tuple[str, ...] = () + #: Anything the caller should read before trusting the rewrite. + notes: tuple[str, ...] = () + + +def remap_view_sql( + sql: str, + *, + source_path: str, + target_path: str, + entries: dict[str, str], + context: tuple[str, ...] = (), + external_exists: Callable[[str], bool] | None = None, +) -> RemapResult: + """Rewrite a view's SQL so it points at the copied subtree, not the original. + + `entries` maps a **lower-cased** source path to its target path — every + table and view being moved. `context` is the view's own ``sqlContext``. + + Each identifier chain outside a string literal or comment is resolved, in + this order, and the first rule that answers wins: + + 1. **A known entry.** The longest prefix of the chain that is one of + `entries` becomes that entry's target path — so ``bwd.draft.t.col`` + rewrites the ``bwd.draft.t`` part and leaves ``.col`` alone. + 2. **Anything else under the source root.** A chain starting with + `source_path` has that prefix swapped for `target_path`. This is the + safety net for something the scan did not enumerate (a table created + between the scan and the rewrite). + 3. **Context-relative, into the subtree.** With a `context`, the chain is + retried as ``context + chain``; if that resolves under rule 1 or 2, the + chain is replaced with the **fully qualified** target path. This is + what keeps a view that says ``FROM sales`` working: after the move it + no longer has that context, so the reference is spelled out. + 4. **Context-relative, out of the subtree.** A chain that follows + ``FROM``/``JOIN``/``INTO``/``TABLE``/``UPDATE`` and still has not + resolved is checked once against `external_exists` as + ``context + chain``; if it really is there, it is spelled out in full + so it survives losing the context. Skipped entirely without an + `external_exists`. + + Anything else is left exactly as written. + + **The corner it can get wrong.** Rule 2 is prefix matching, so if + `source_path` is a single segment (a whole space) and the SQL aliases a + table with that same name, ``alias.column`` looks like a path into the + source. Moving a whole space is not the normal case, and moving a folder + (two segments or more) cannot hit it — but it is why a dry run prints + every rewritten definition. + """ + source_segments = _segments(source_path) + target_segments = _segments(target_path) + source_lower = _lower(source_segments) + context_segments = list(context) + + references: set[str] = set() + notes: list[str] = [] + probed: dict[str, bool] = {} + + def resolve_known( + segments: list[str], *, allow_root_prefix: bool = True + ) -> tuple[int, list[str]] | None: + """(segments consumed, replacement) under rules 1 and 2, or None. + + `allow_root_prefix=False` drops rule 2, leaving only an exact match + against a known entry. Rule 3 needs that: prepending a context that is + itself inside the source tree makes *every* chain — every keyword + included — start with the source root, so the fallback would rewrite + `SELECT` as eagerly as a real reference. The enumeration is complete + for the subtree, so an exact match is all a context-relative + reference into it ever needs. + """ + for count in range(len(segments), 0, -1): + key = ".".join(_lower(segments[:count])) + if key in entries: + references.add(key) + return count, _segments(entries[key]) + if not allow_root_prefix: + return None + lowered = _lower(segments) + if len(lowered) >= len(source_lower) and lowered[: len(source_lower)] == source_lower: + return len(segments), target_segments + segments[len(source_segments) :] + return None + + out: list[str] = [] + previous_keyword = "" + for token in _iter_sql_chains(sql): + if token.kind == "text": + out.append(token.raw) + stripped = token.raw.strip() + if stripped and stripped[-1] == ",": + previous_keyword = "," + continue + + segments = list(token.segments) + replacement: list[str] | None = None + consumed = len(segments) + + resolved = resolve_known(segments) + if resolved is not None: + consumed, replacement = resolved + elif context_segments: + relative = resolve_known(context_segments + segments, allow_root_prefix=False) + if relative is not None: + # The match covered the context as well, so only what is left + # over of the chain itself counts as consumed — `sales.region` + # against a context that resolves `sales` keeps `.region`. + consumed = max(relative[0] - len(context_segments), 1) + replacement = relative[1] + elif external_exists is not None and previous_keyword in _REFERENCE_KEYWORDS: + candidate = ".".join(context_segments + segments) + if candidate not in probed: + probed[candidate] = external_exists(candidate) + if probed[candidate]: + consumed, replacement = len(segments), context_segments + segments + notes.append( + f"{'.'.join(segments)!r} was relative to the view's context — " + f"written out in full as {candidate!r}" + ) + + if replacement is None: + out.append(token.raw) + else: + out.append(_quote_path(".".join(replacement))) + # Whatever of the chain the rule did not consume (a column, say) + # is appended back unchanged. + if consumed < len(segments): + out.append("." + ".".join(f'"{segment}"' for segment in segments[consumed:])) + + previous_keyword = segments[0].upper() if len(segments) == 1 else "" + + if not context_segments: + unqualified = [ + token + for token in _iter_sql_chains(sql) + if token.kind == "chain" + and len(token.segments) == 1 + and token.segments[0].upper() not in _SQL_WORDS + ] + if unqualified: + notes.append( + "the view's sqlContext is unknown, so any unqualified reference in its " + "SQL was left as written — check it resolves from the new location" + ) + + return RemapResult(sql="".join(out), references=tuple(sorted(references)), notes=tuple(notes)) + + +# -------------------------------------------------------------------------- +# scanning +# -------------------------------------------------------------------------- + + +def _key(idempotency_key: str, operation: str, path: str) -> str: + """One idempotency key per entry, derived from the caller's own. + + A folder move is N operations, and `retry_state` remembers one attempt per + key — so sharing a single key across them would leave only the last stall + remembered. Deriving ``"::"`` keeps every step + separately resumable with `catalog.retry_pending(...)`, and keeps a re-run + of the same job landing on the same keys. + """ + return f"{idempotency_key}:{operation}:{path}" + + +def _emit(on_progress: ProgressCallback | None, message: str) -> None: + if on_progress is not None: + on_progress(message) + + +def _order_views(views: list[TreeEntry]) -> list[TreeEntry]: + """Views sorted so a view comes after every view in this subtree it selects + from. A dependency cycle (which Dremio would not have let you create) is + left in scan order rather than raising.""" + by_path = {entry.source_path.lower(): entry for entry in views} + state: dict[str, str] = {} + ordered: list[TreeEntry] = [] + + def visit(entry: TreeEntry) -> None: + key = entry.source_path.lower() + if state.get(key) in ("done", "visiting"): + return + state[key] = "visiting" + for dependency in entry.depends_on: + other = by_path.get(dependency.lower()) + if other is not None and other.source_path != entry.source_path: + visit(other) + state[key] = "done" + ordered.append(entry) + + for entry in views: + visit(entry) + return ordered + + +def folderscan( + catalog: Catalog, + source_path: str, + target_path: str, + *, + idempotency_key: str, + qualify_external: bool = True, + on_progress: ProgressCallback | None = None, +) -> FolderTree: + """Everything under `source_path`, mapped to where it would land under + `target_path`. Reads only — nothing is created, changed or deleted. + + Views come back with their SQL already rewritten (`TreeEntry.remapped_sql`) + and in dependency order, so the plan a dry run prints is exactly what an + apply would run. + + Raises `CatalogOperationError` if the two paths are the same, if either is + inside the other (a move into your own subtree has no fixed point), or if + `source_path` cannot be read at all. + """ + if not _segments(source_path) or not _segments(target_path): + raise CatalogOperationError("both a source and a target path are required") + if _lower(_segments(source_path)) == _lower(_segments(target_path)): + raise CatalogOperationError(f"source and target are the same folder: {source_path!r}") + if _is_inside(target_path, source_path): + raise CatalogOperationError( + f"target {target_path!r} is inside source {source_path!r} — " + "a folder cannot be moved into itself" + ) + if _is_inside(source_path, target_path): + raise CatalogOperationError( + f"source {source_path!r} is inside target {target_path!r} — " + "that would copy a folder onto its own parent" + ) + + notes: list[str] = [] + + _emit(on_progress, f"scanning {source_path} …") + entries = catalog.getentriesfrom( + source_path, idempotency_key=_key(idempotency_key, "getentriesfrom", source_path) + ) + + folder_paths: list[str] = [] + try: + folder_paths = catalog.getfoldersfrom( + source_path, idempotency_key=_key(idempotency_key, "getfoldersfrom", source_path) + ) + except CatalogOperationError as exc: + if not entries: + raise CatalogOperationError(f"could not read {source_path!r}: {exc}") from exc + notes.append( + f"folders could not be listed over the REST catalog ({exc}) — the tree was " + "rebuilt from the paths of the tables and views found, so any empty folder " + "under the source will not be reproduced" + ) + + # Every folder an entry sits in has to exist at the target whether or not + # the REST listing found it, so derive those too and merge. + derived: set[str] = set() + source_depth = len(_segments(source_path)) + for entry in entries: + segments = _segments(entry.path) + for depth in range(source_depth + 1, len(segments)): + derived.add(".".join(segments[:depth])) + all_folders = sorted(set(folder_paths) | derived, key=lambda path: (path.count("."), path)) + + folders = tuple( + TreeEntry( + source_path=path, + target_path=_remap_path(path, source_path, target_path), + kind="FOLDER", + ) + for path in all_folders + ) + tables = tuple( + TreeEntry( + source_path=entry.path, + target_path=_remap_path(entry.path, source_path, target_path), + kind="TABLE", + ) + for entry in entries + if entry.kind == "TABLE" + ) + + # The whole map has to be known before any view can be rewritten — a view + # may select from a table listed after it. + path_map = {entry.source_path.lower(): entry.target_path for entry in (*tables, *folders)} + canonical = {entry.source_path.lower(): entry.source_path for entry in (*tables, *folders)} + view_entries = [entry for entry in entries if entry.kind == "VIEW"] + for entry in view_entries: + path_map[entry.path.lower()] = _remap_path(entry.path, source_path, target_path) + canonical[entry.path.lower()] = entry.path + + def external_exists(path: str) -> bool: + return catalog.entryexists(path, idempotency_key=_key(idempotency_key, "entryexists", path)) + + views: list[TreeEntry] = [] + for entry in view_entries: + entry_target = _remap_path(entry.path, source_path, target_path) + entry_notes: list[str] = [] + view_sql: str | None = None + view_context: tuple[str, ...] = () + remapped: str | None = None + references: tuple[str, ...] = () + try: + definition = catalog.getviewsql( + entry.path, idempotency_key=_key(idempotency_key, "getviewsql", entry.path) + ) + view_sql, view_context = definition.sql, definition.context + result = remap_view_sql( + view_sql, + source_path=source_path, + target_path=target_path, + entries=path_map, + context=view_context, + external_exists=external_exists if qualify_external else None, + ) + remapped = result.sql + references = tuple(canonical.get(ref, ref) for ref in result.references) + entry_notes.extend(result.notes) + except CatalogOperationError as exc: + entry_notes.append(f"could not read this view's definition: {exc}") + views.append( + TreeEntry( + source_path=entry.path, + target_path=entry_target, + kind="VIEW", + view_sql=view_sql, + view_context=view_context, + remapped_sql=remapped, + depends_on=references, + notes=tuple(entry_notes), + ) + ) + + tree = FolderTree( + source_path=source_path, + target_path=target_path, + folders=folders, + tables=tables, + views=tuple(_order_views(views)), + notes=tuple(notes), + ) + _emit( + on_progress, + f"found {len(tree.folders)} folder(s), {len(tree.tables)} table(s), " + f"{len(tree.views)} view(s) under {source_path}", + ) + return tree + + +# -------------------------------------------------------------------------- +# copying +# -------------------------------------------------------------------------- + + +def _row_count(catalog: Catalog, path: str, idempotency_key: str) -> int | None: + """``COUNT(*)`` for `path`, or None if it is not there at all.""" + try: + return catalog.gettableitemsfrom( + path, idempotency_key=_key(idempotency_key, "gettableitemsfrom", path) + ).row_count + except CatalogOperationError: + return None + + +def _carry_metadata(catalog: Catalog, entry: TreeEntry, idempotency_key: str) -> list[str]: + """Copy the wiki (and, for tables/views, Dremio's tags) across. + + Best-effort by design: metadata is descriptive, and losing a wiki is not a + reason to fail a move that otherwise verified. Whatever went wrong comes + back as notes on the entry instead. + + Not covered: the DDS ``meta.json`` sidecar the JupyterLab extension shows + under *⚙ Domain*. That lives in the Document Service, not in Dremio, and + is a different system's copy to make. + """ + problems: list[str] = [] + try: + wiki = catalog.getwikifrom( + entry.source_path, + idempotency_key=_key(idempotency_key, "getwikifrom", entry.source_path), + ) + except CatalogOperationError: + wiki = "" # no wiki, or nothing readable — normal, nothing to carry + if wiki: + try: + catalog.setwikito( + entry.target_path, + wiki, + idempotency_key=_key(idempotency_key, "setwikito", entry.target_path), + ) + except CatalogOperationError as exc: + problems.append(f"wiki not carried over: {exc}") + + if entry.kind == "FOLDER": + return problems # Dremio's tags exist only on tables and views + try: + tags = catalog.gettagsfrom( + entry.source_path, + idempotency_key=_key(idempotency_key, "gettagsfrom", entry.source_path), + ) + except CatalogOperationError: + return problems + if tags: + try: + catalog.settagsto( + entry.target_path, + tags, + idempotency_key=_key(idempotency_key, "settagsto", entry.target_path), + ) + except CatalogOperationError as exc: + problems.append(f"tags not carried over: {exc}") + return problems + + +def foldercopy( + catalog: Catalog, + source_path: str, + target_path: str, + *, + idempotency_key: str, + dry_run: bool = False, + overwrite: bool = False, + skip_existing: bool = True, + verify_views: bool = True, + copy_metadata: bool = True, + qualify_external: bool = True, + tree: FolderTree | None = None, + on_progress: ProgressCallback | None = None, +) -> FolderTransferReport: + """Reproduce the whole `source_path` subtree under `target_path`, and prove it. + + The source is left completely untouched — this is the copy half of + `foldermove`, and useful on its own for taking a snapshot of a folder. + + * `dry_run=True` writes nothing and returns the plan, with each table's + source row count filled in so you can see what a real run would move. + * `overwrite=True` replaces whatever is already at the target, per entry. + * `skip_existing=True` (the default) leaves a target table that is already + there **and already has the source's row count** alone, reporting it as + ``skipped`` — which is what makes an interrupted run resumable by just + running it again. A target that exists with a *different* count fails + the entry rather than being silently accepted, and so does any occupied + target with `skip_existing=False`: without `overwrite=True`, nothing + already at the target is ever destroyed. + * `verify_views=True` (the default) also counts each copied view on both + sides. That is the only real proof a rewritten definition resolves to + the same data, so leave it on unless the views are expensive to run. + * `copy_metadata=True` carries each entry's wiki and tags across + (best-effort — see `_carry_metadata`). + * `tree=` skips the scan and applies a `FolderTree` you already have — + the "look at the plan, then run it" flow, without paying for a second + scan or risking a plan that differs from the one you read. + + Returns a `FolderTransferReport`. A per-entry failure does not raise: the + whole point is to finish the sweep and report, so one bad table does not + hide the state of the other forty. + """ + if tree is None: + tree = folderscan( + catalog, + source_path, + target_path, + idempotency_key=idempotency_key, + qualify_external=qualify_external, + on_progress=on_progress, + ) + + report = FolderTransferReport( + source_path=tree.source_path, + target_path=tree.target_path, + dry_run=dry_run, + notes=list(tree.notes), + ) + report.folders = [EntryResult(entry=entry) for entry in tree.folders] + report.tables = [EntryResult(entry=entry) for entry in tree.tables] + report.views = [ + EntryResult(entry=entry, sql=entry.remapped_sql, notes=list(entry.notes)) + for entry in tree.views + ] + + if dry_run: + for result in report.tables: + result.source_rows = _row_count(catalog, result.entry.source_path, idempotency_key) + for result in report.views: + if result.entry.remapped_sql is None: + result.status = "failed" + result.error = "no usable view definition — see the notes on this entry" + return report + + for result in report.folders: + entry = result.entry + try: + catalog.createfolder( + entry.target_path, + create_parents=True, + idempotency_key=_key(idempotency_key, "createfolder", entry.target_path), + ) + result.status = "created" + _emit(on_progress, f"folder {entry.target_path}") + except CatalogOperationError as exc: + result.status = "failed" + result.error = str(exc) + _emit(on_progress, f"FAILED folder {entry.target_path}: {exc}") + + for result in report.tables: + entry = result.entry + try: + result.source_rows = _row_count(catalog, entry.source_path, idempotency_key) + existing_rows = _row_count(catalog, entry.target_path, idempotency_key) + # Something is already at the target. `overwrite=False` never + # destroys it — not even to "finish" the copy — so the only two + # ways past here are accepting it (skip_existing, and only when + # the counts already agree) or being told to replace it. + if existing_rows is not None and not overwrite: + result.target_rows = existing_rows + if skip_existing and existing_rows == result.source_rows: + result.status = "skipped" + _emit(on_progress, f"table {entry.target_path} (there already, counts agree)") + continue + result.status = "failed" + result.error = ( + f"{entry.target_path!r} already exists with {existing_rows} row(s) and the " + f"source has {result.source_rows} — pass overwrite=True to replace it" + ) + _emit(on_progress, f"FAILED table {entry.target_path}: {result.error}") + continue + catalog.datacopy( + entry.source_path, + entry.target_path, + overwrite=overwrite, + create_target_folder=True, + idempotency_key=_key(idempotency_key, "datacopy", entry.target_path), + ) + result.target_rows = _row_count(catalog, entry.target_path, idempotency_key) + result.status = "copied" + _emit( + on_progress, + f"table {entry.target_path} " + f"({result.source_rows} -> {result.target_rows} rows)", + ) + except CatalogOperationError as exc: + result.status = "failed" + result.error = str(exc) + _emit(on_progress, f"FAILED table {entry.target_path}: {exc}") + + for result in report.views: + entry = result.entry + if entry.remapped_sql is None: + result.status = "failed" + result.error = "no usable view definition — see the notes on this entry" + _emit(on_progress, f"FAILED view {entry.target_path}: {result.error}") + continue + try: + already_there = catalog.entryexists( + entry.target_path, + idempotency_key=_key(idempotency_key, "entryexists", entry.target_path), + ) + if already_there and skip_existing and not overwrite: + result.status = "skipped" + _emit(on_progress, f"view {entry.target_path} (there already)") + else: + # `overwrite` is passed through as given, so an occupied target + # with overwrite=False fails here rather than being replaced. + catalog.createviewas( + entry.target_path, + entry.remapped_sql, + overwrite=overwrite, + create_target_folder=True, + idempotency_key=_key(idempotency_key, "createviewas", entry.target_path), + ) + result.status = "created" + _emit(on_progress, f"view {entry.target_path}") + except CatalogOperationError as exc: + result.status = "failed" + result.error = str(exc) + _emit(on_progress, f"FAILED view {entry.target_path}: {exc}") + + if verify_views: + for result in report.views: + if result.status == "failed": + continue + try: + result.source_rows = _row_count(catalog, result.entry.source_path, idempotency_key) + result.target_rows = _row_count(catalog, result.entry.target_path, idempotency_key) + except CatalogOperationError as exc: + result.status = "failed" + result.error = f"the copied view would not run: {exc}" + continue + if not result.counts_match: + result.status = "failed" + result.error = ( + f"row counts disagree — source {result.source_rows}, " + f"target {result.target_rows}; the rewritten definition is not equivalent" + ) + _emit(on_progress, f"FAILED view {result.entry.target_path}: {result.error}") + + if copy_metadata: + for result in report.results: + if result.status in ("failed", "planned"): + continue + result.notes.extend(_carry_metadata(catalog, result.entry, idempotency_key)) + + return report + + +# -------------------------------------------------------------------------- +# moving +# -------------------------------------------------------------------------- + + +def foldermove( + catalog: Catalog, + source_path: str, + target_path: str, + *, + idempotency_key: str, + dry_run: bool = False, + overwrite: bool = False, + skip_existing: bool = True, + verify_views: bool = True, + copy_metadata: bool = True, + qualify_external: bool = True, + delete_source_root: bool = True, + tree: FolderTree | None = None, + on_progress: ProgressCallback | None = None, +) -> FolderTransferReport: + """Move the whole `source_path` subtree to `target_path`. + + `foldercopy` first — folders, tables (row-counted on both sides), views + (rewritten to point at the copies) — and then, **only if + `report.verified` came back true**, the source is taken apart: views in + reverse dependency order, then tables, then folders deepest-first, then + `source_path` itself unless `delete_source_root=False`. + + Anything less than a clean copy leaves the source exactly where it is, and + says so in `report.notes`. Nothing is ever removed on a `dry_run`. + + All of `foldercopy`'s keyword arguments mean the same thing here. + """ + report = foldercopy( + catalog, + source_path, + target_path, + idempotency_key=idempotency_key, + dry_run=dry_run, + overwrite=overwrite, + skip_existing=skip_existing, + verify_views=verify_views, + copy_metadata=copy_metadata, + qualify_external=qualify_external, + tree=tree, + on_progress=on_progress, + ) + + if dry_run: + report.notes.append( + "dry run — the source would be removed only after the copy above verified clean" + ) + return report + + if not report.verified: + plural = "y" if len(report.failures) == 1 else "ies" + report.notes.append( + f"the copy did not verify ({len(report.failures)} failed entr{plural}) — " + f"{source_path!r} was left exactly as it was. Fix what failed and run this " + "again; entries that already landed are skipped." + ) + _emit(on_progress, f"source {source_path} left untouched — the copy did not verify") + return report + + _emit(on_progress, f"copy verified — removing {source_path}") + + def remove(path: str, delete: Callable[[str], object]) -> None: + try: + delete(path) + report.deleted.append(path) + _emit(on_progress, f"removed {path}") + except CatalogOperationError as exc: + report.delete_errors.append((path, str(exc))) + _emit(on_progress, f"FAILED to remove {path}: {exc}") + + def drop_view(path: str) -> None: + catalog.deleteview(path, idempotency_key=_key(idempotency_key, "deleteview", path)) + + def drop_table(path: str) -> None: + catalog.deletetable(path, idempotency_key=_key(idempotency_key, "deletetable", path)) + + def drop_folder(path: str) -> None: + catalog.deletefolder(path, idempotency_key=_key(idempotency_key, "deletefolder", path)) + + for result in reversed(report.views): + remove(result.entry.source_path, drop_view) + for result in report.tables: + remove(result.entry.source_path, drop_table) + # Deepest first: a folder has to be empty before it will delete, and + # cascade is deliberately not used here — see the module docstring. + for result in sorted( + report.folders, key=lambda item: item.entry.source_path.count("."), reverse=True + ): + remove(result.entry.source_path, drop_folder) + if delete_source_root: + remove(source_path, drop_folder) + + report.source_deleted = not report.delete_errors + if report.delete_errors: + report.notes.append( + "the copy verified and the target is complete, but part of the source could not " + "be removed (listed above) — it is safe to re-run this, or to delete what is " + "left by hand" + ) + return report diff --git a/src/eea_datalakehouse/catalog/operations.py b/src/eea_datalakehouse/catalog/operations.py index e198763..fbdb026 100644 --- a/src/eea_datalakehouse/catalog/operations.py +++ b/src/eea_datalakehouse/catalog/operations.py @@ -206,6 +206,27 @@ def _entry_exists(executor: SqlExecutor, path: str, *, idempotency_key: str) -> return len(rows) > 0 +def entryexists( + executor: SqlExecutor, + path: str, + *, + idempotency_key: str, +) -> bool: + """Whether `path` is a table or view Dremio knows about. + + The public form of the existence check `table2view`/`datacopy`/`datamove` + already run internally. Transport-agnostic (`INFORMATION_SCHEMA`, so REST + or Flight both answer) — unlike `CatalogRestClient.exists`, which goes + through the REST catalog API and also sees folders, spaces and sources. + Fetches no rows from `path` itself, so it costs nothing on a large table. + """ + try: + return _entry_exists(executor, path, idempotency_key=idempotency_key) + except EngineStartingError as exc: + retry_state.record(idempotency_key, "entryexists", path, str(exc), params={"path": path}) + raise + + def _entry_kind(executor: SqlExecutor, path: str, *, idempotency_key: str) -> Literal["TABLE", "VIEW"]: """Whether `path` is a Dremio TABLE or VIEW, via INFORMATION_SCHEMA's own ``TABLE_TYPE`` column — doubles as an existence check. @@ -1198,6 +1219,237 @@ def gettableitemsfrom( return TableInfo(schema=schema, row_count=row_count) +@dataclass(frozen=True, slots=True) +class CatalogEntry: + """One table or view under a schema: its full dot-path, and which it is.""" + + path: str + kind: Literal["TABLE", "VIEW"] + + +def getentriesfrom( + executor: SqlExecutor, + schema_path: str, + *, + idempotency_key: str, +) -> list[CatalogEntry]: + """`gettablesfrom`, but each path carries whether it is a TABLE or a VIEW. + + Same single query over the whole subtree (see `gettablesfrom` for the + `LIKE` reasoning), with ``TABLE_TYPE`` selected as well — so a caller + copying a folder knows per entry what it is without one `_entry_kind` + round-trip per path. It matters: copying a view with + ``CREATE TABLE … AS SELECT`` silently materialises it into a table, + which is a different kind of thing from the original. + + Sorted by path, so two runs enumerate in the same order. + """ + like_pattern = f"{_like_escape(schema_path)}.%" + sql = ( + 'SELECT "TABLE_SCHEMA", "TABLE_NAME", "TABLE_TYPE" FROM INFORMATION_SCHEMA."TABLES" ' + f"WHERE \"TABLE_SCHEMA\" = {_quote_literal(schema_path)} " + f"OR \"TABLE_SCHEMA\" LIKE {_quote_literal(like_pattern)} ESCAPE '\\'" + ) + rows = _fetch_step( + executor, + sql, + operation="getentriesfrom", + target=schema_path, + idempotency_key=idempotency_key, + params={"schema_path": schema_path}, + ) + entries = [ + CatalogEntry( + path=f"{row['TABLE_SCHEMA']}.{row['TABLE_NAME']}", + kind="VIEW" if "VIEW" in str(row.get("TABLE_TYPE", "")).upper() else "TABLE", + ) + for row in rows + ] + return sorted(entries, key=lambda entry: entry.path) + + +@dataclass(frozen=True, slots=True) +class ViewDefinition: + """A view's own SQL, and the catalog context it was written against. + + `context` is the ``sqlContext`` Dremio stores beside the SQL — the schema + an unqualified ``FROM sales`` inside `sql` resolves against. It is empty + when the definition came from `INFORMATION_SCHEMA."VIEWS"`, which has no + equivalent column: an empty `context` means *unknown*, not *none*. + """ + + sql: str + context: tuple[str, ...] = () + + +def getviewsql( + executor: SqlExecutor, + view_path: str, + *, + catalog_rest: CatalogRestClient | None = None, + idempotency_key: str, +) -> ViewDefinition: + """The SQL the view at `view_path` is defined by. + + Two sources, in this order — they answer with different amounts of the + truth, so the better one is tried first: + + 1. `catalog_rest.get_view_definition` (Dremio's catalog entity), which + carries the SQL **and** its ``sqlContext``. Skipped without a + `catalog_rest`, and fallen through if the lookup is rejected — some + Dremio source types answer a nested by-path lookup with a 400 (see + `CatalogRestClient.is_folder`). + 2. ``INFORMATION_SCHEMA."VIEWS"``, which has ``VIEW_DEFINITION`` but no + context — so the result's `context` is empty and any unqualified + reference inside the SQL cannot be resolved from it. + + Raises `CatalogOperationError` if neither answers — a view whose + definition cannot be read is not something a caller should quietly + replace with a guess. + """ + if catalog_rest is not None: + try: + definition = catalog_rest.get_view_definition(view_path) + except CatalogOperationError: + definition = None + if definition is not None: + sql, context = definition + retry_state.clear(idempotency_key) + return ViewDefinition(sql=sql, context=tuple(context)) + + schema_path = _parent_path(view_path) + if schema_path is None: + raise ValueError(f"{view_path!r} has no containing schema") + leaf = view_path.rsplit(".", 1)[-1] + sql_query = ( + 'SELECT "VIEW_DEFINITION" FROM INFORMATION_SCHEMA."VIEWS" ' + f"WHERE \"TABLE_SCHEMA\" = {_quote_literal(schema_path)} " + f"AND \"TABLE_NAME\" = {_quote_literal(leaf)}" + ) + rows = _fetch_step( + executor, + sql_query, + operation="getviewsql", + target=view_path, + idempotency_key=idempotency_key, + params={"view_path": view_path}, + ) + definition_sql = str(rows[0].get("VIEW_DEFINITION", "")) if rows else "" + if not definition_sql.strip(): + raise CatalogOperationError( + f"could not read the view definition of {view_path!r} — " + "it is not a view, or Dremio reports no SQL for it" + ) + return ViewDefinition(sql=definition_sql, context=()) + + +def createviewas( + executor: SqlExecutor, + view_path: str, + sql: str, + *, + overwrite: bool = False, + create_target_folder: bool = True, + catalog_rest: CatalogRestClient | None = None, + idempotency_key: str, +) -> SqlResult: + """Create the view at `view_path` from an arbitrary `sql` query body. + + `table2view`'s general form: that one always writes + ``SELECT * FROM ``, this one takes whatever query you hand it — + which is what a folder copy needs, since a copied view has to select + from the *copied* tables, not the originals (see + `eea_datalakehouse.catalog.foldertree`). + + `sql` is the query body only (no ``CREATE VIEW`` prefix) and is sent + through unchanged — quoting and correctness are the caller's, since the + text usually comes back out of Dremio itself. + + `overwrite=False` (the default) raises `CatalogOperationError` if + anything already sits at `view_path`, rather than letting Dremio answer + with its own less specific error. `overwrite=True` drops whatever is + actually there first, detecting whether it is a table or a view — the + same reasoning as `datacopy`'s own `check_or_drop_target`. + """ + params: dict[str, Any] = { + "view_path": view_path, + "sql": sql, + "overwrite": overwrite, + "create_target_folder": create_target_folder, + } + + def ensure_target_folder() -> None: + if not create_target_folder: + return + parent = _parent_path(view_path) + if parent is None: + return + if catalog_rest is None: + raise CatalogOperationError( + "createviewas(create_target_folder=True) needs catalog_rest= " + "to check/create the target folder" + ) + catalog_rest.ensure_folder_path(parent) + + def check_or_drop_target() -> None: + if not _entry_exists(executor, view_path, idempotency_key=idempotency_key): + return + if not overwrite: + raise CatalogOperationError( + f"target {view_path!r} already exists — pass overwrite=True to replace it" + ) + existing_kind = _entry_kind(executor, view_path, idempotency_key=idempotency_key) + executor.execute( + f"DROP {existing_kind} IF EXISTS {_quote_path(view_path)}", + idempotency_key=idempotency_key, + ) + + def create_view() -> SqlResult: + return executor.execute( + f"CREATE VIEW {_quote_path(view_path)} AS {sql}", idempotency_key=idempotency_key + ) + + return _run_actions( + [ensure_target_folder, check_or_drop_target, create_view], + operation="createviewas", + target=view_path, + idempotency_key=idempotency_key, + params=params, + ) + + +def getfoldersfrom( + catalog_rest: CatalogRestClient, + path: str, + *, + recursive: bool = True, + idempotency_key: str, +) -> list[str]: + """Dot-paths of the folders under `path`, shallowest first. + + REST-only (see module docstring) — folders are not in + `INFORMATION_SCHEMA`, which only knows about tables and views, so an + *empty* folder is invisible to `gettablesfrom`/`getentriesfrom` and can + only be found this way. + + Breadth-first, which is the order they have to be created in on the + other side. `path` itself is not included. + """ + try: + folders = catalog_rest.list_folders(path, recursive=recursive) + except EngineStartingError as exc: + retry_state.record( + idempotency_key, + "getfoldersfrom", + path, + str(exc), + params={"path": path, "recursive": recursive}, + ) + raise + retry_state.clear(idempotency_key) + return folders + + def getwikifrom( catalog_rest: CatalogRestClient, path: str, @@ -1674,6 +1926,11 @@ def delete_dataset(dataset_path: str) -> None: "deletetable": deletetable, "gettablesfrom": gettablesfrom, "gettableitemsfrom": gettableitemsfrom, + "entryexists": entryexists, + "getentriesfrom": getentriesfrom, + "getviewsql": getviewsql, + "createviewas": createviewas, + "getfoldersfrom": getfoldersfrom, "getwikifrom": getwikifrom, "gettagsfrom": gettagsfrom, "setwikito": setwikito, diff --git a/src/eea_datalakehouse/catalog/rest.py b/src/eea_datalakehouse/catalog/rest.py index d0b3344..7f14605 100644 --- a/src/eea_datalakehouse/catalog/rest.py +++ b/src/eea_datalakehouse/catalog/rest.py @@ -230,6 +230,74 @@ def is_table_or_view(self, path: str) -> bool: return False return entity is not None and entity.get("entityType") == "dataset" + def children(self, path: str) -> list[dict[str, Any]]: + """Dremio's own child JSON for every entity directly inside `path`. + + Each child carries at least ``path`` (a list of segments), ``id`` and + ``type``; a container also carries ``containerType`` (see + `_child_is_folder`). Raises `CatalogOperationError` if `path` does not + exist — unlike `is_folder`/`is_table_or_view`, which swallow a failed + lookup, because "I could not read this folder" and "this folder is + empty" are not the same answer for a caller that is about to copy or + delete its contents. + """ + entity = self._lookup_by_path(path) + if entity is None: + raise CatalogOperationError(f"{path!r} does not exist") + return list(entity.get("children") or []) + + def list_folders(self, path: str, *, recursive: bool = True) -> list[str]: + """Dot-paths of the folders inside `path`, shallowest first. + + `path` itself is not included. `recursive=True` (the default) walks + the whole subtree; `False` returns only the direct children. Ordering + is breadth-first, which is the order a caller has to *create* them in + on the other side — a parent before anything under it. + + Same unverified assumption as `delete_folder`'s cascade: a folder + child is recognised by ``type="CONTAINER"`` / + ``containerType="FOLDER"``. + """ + found: list[str] = [] + queue = [path] + while queue: + current = queue.pop(0) + for child in self.children(current): + if not self._child_is_folder(child): + continue + child_path = ".".join(child.get("path") or []) + if not child_path: + continue + found.append(child_path) + if recursive: + queue.append(child_path) + return found + + def get_view_definition(self, path: str) -> tuple[str, list[str]] | None: + """``(sql, sql_context)`` for the view at `path`, or None if `path` is + not a view (or Dremio's entity carries no SQL for it). + + Dremio's catalog entity for a virtual dataset carries both the SQL + *as written* and the ``sqlContext`` it was written against — and the + context is what makes an unqualified ``FROM sales`` inside that SQL + resolvable. `INFORMATION_SCHEMA."VIEWS"` has the definition but not + the context, which is why this REST route is preferred where it + answers (see `getviewsql`). + + Best-effort about *reachability*, not about correctness: a lookup + Dremio rejects outright (the 400 described in `is_folder`) surfaces as + `CatalogOperationError`, so the caller can fall back rather than + silently copy a view with no definition. + """ + entity = self._lookup_by_path(path) + if entity is None: + raise CatalogOperationError(f"{path!r} does not exist") + sql = entity.get("sql") + if not sql: + return None + context = [str(segment) for segment in (entity.get("sqlContext") or [])] + return str(sql), context + def get_wiki(self, path: str) -> str: """The Dremio wiki text attached to the catalog entity at `path`. diff --git a/src/eea_datalakehouse/dds_ingestion/client.py b/src/eea_datalakehouse/dds_ingestion/client.py index 1d84b91..61ef9bd 100644 --- a/src/eea_datalakehouse/dds_ingestion/client.py +++ b/src/eea_datalakehouse/dds_ingestion/client.py @@ -71,10 +71,14 @@ class IngestApiError(RuntimeError): ``schema_required``; ``""`` when the body carried none) and ``retryable`` whether repeating the request — for a failed transfer, ``retry`` — can succeed unchanged (``None`` from a server predating contract v0.8). + ``failed_stage`` is where a failed commit broke, ``"upload"`` or ``"load"``, + as the session reports it; ``FolderIngest`` sets it, and it is ``None`` + everywhere else. """ error_slug: str = "" retryable: bool | None = None + failed_stage: str | None = None def __init__(self, status_code: int, message: str, *, where: str | None = None) -> None: super().__init__( @@ -353,6 +357,31 @@ def get_status(self, session_id: str) -> StatusResult: self._raise_for_status(resp, f"GET /api/v1/ingest/{session_id}") return StatusResult.from_json(resp.json()) + def health(self) -> None: + """Raise unless DDS answers its liveness probe (``GET /health``, no login).""" + resp = self._http.get(f"{self._base_url}/health") + self._raise_for_status(resp, "GET /health") + + def limits(self, runner: str = "notebook") -> dict[str, Any]: + """The ingest size limits for this kind of client (``GET /api/v1/ingest/limits``, + contract v0.8). Needs a login, so it also proves the credential works.""" + resp = self._http.get( + f"{self._base_url}/api/v1/ingest/limits", + params={"runner": runner}, + headers=self._auth_headers, + ) + self._raise_for_status(resp, "GET /api/v1/ingest/limits") + result: dict[str, Any] = resp.json() + return result + + def storage_selftest(self) -> dict[str, Any]: + """Re-run DDS's S3 write test and return its verdict (``POST /storage-selftest``, + no login). ``storage_write_test.ok`` says whether ingest can write.""" + resp = self._http.post(f"{self._base_url}/storage-selftest") + self._raise_for_status(resp, "POST /storage-selftest") + result: dict[str, Any] = resp.json() + return result + def list_sessions(self, state: str | None = None) -> list[StatusResult]: """Your ingest sessions (``GET /api/v1/ingest``). @@ -368,7 +397,12 @@ def list_sessions(self, state: str | None = None) -> list[StatusResult]: rows = payload if isinstance(payload, list) else payload.get("sessions", []) return [StatusResult.from_json(row) for row in rows] - def retry(self, session_id: str) -> StatusResult: + def retry( + self, + session_id: str, + *, + definition: TableDefinition | Mapping[str, Any] | None = None, + ) -> StatusResult: """Re-run a failed session's last stage (``POST /api/v1/ingest/{id}/retry``). Only a ``failed`` session can be retried, and only the Dremio load stage @@ -376,9 +410,16 @@ def retry(self, session_id: str) -> StatusResult: (see :attr:`StatusResult.is_resumable`). A session whose upload never completed, or whose failure a re-run cannot fix, raises :class:`IngestApiError` telling you to upload again. + + ``definition`` (contract v0.8, DI-12.6) replaces the stored table + definition before the re-run — the fix for a ``schema_required`` or + ``schema_mismatch`` failure without uploading again. Without it the + request has no body, exactly as before. """ + payload = definition_payload(definition) resp = self._http.post( f"{self._base_url}/api/v1/ingest/{session_id}/retry", + json={"definition": payload} if payload is not None else None, headers=self._auth_headers, ) self._raise_for_status(resp, f"POST /api/v1/ingest/{session_id}/retry") diff --git a/src/eea_datalakehouse/dds_ingestion/folder.py b/src/eea_datalakehouse/dds_ingestion/folder.py index 85f65f8..b12212f 100644 --- a/src/eea_datalakehouse/dds_ingestion/folder.py +++ b/src/eea_datalakehouse/dds_ingestion/folder.py @@ -17,6 +17,7 @@ from __future__ import annotations import logging +import threading from collections.abc import Mapping from concurrent.futures import ThreadPoolExecutor, as_completed from dataclasses import dataclass, field @@ -26,9 +27,9 @@ import httpx import pyarrow as pa -from .client import IngestApiError, IngestClient +from .client import IngestApiError, IngestClient, UnsupportedByServerError from .credentials import DremioCreds, load_base_url, load_creds -from .formats import FORMATS, compare_columns, local_columns +from .formats import FORMATS, compare_columns, first_non_utf8, is_utf8, local_columns from .models import ( BeginPlan, BeginResult, @@ -60,6 +61,27 @@ class IngestStateError(RuntimeError): """ +@dataclass(frozen=True, slots=True) +class DiagnosisStep: + """One check of :meth:`FolderIngest.diagnose`; ``ok`` is ``None`` when skipped.""" + + name: str + ok: bool | None + detail: str + + +@dataclass(slots=True) +class Diagnosis: + """What :meth:`FolderIngest.diagnose` found, one step per check, in order.""" + + steps: list[DiagnosisStep] = field(default_factory=list) + + @property + def ok(self) -> bool: + """Whether every check that ran passed.""" + return all(step.ok is not False for step in self.steps) + + @dataclass(slots=True) class IngestOutcome: """Summary returned by :meth:`FolderIngest.run` and :meth:`FolderIngest.retry`. @@ -76,6 +98,34 @@ class IngestOutcome: etags: dict[str, str] = field(default_factory=dict) resumed: bool = False upload_mode: str = "direct" # how the files went: "direct" (presigned S3) or "proxy" (DDS) + # How many of files_uploaded went through DDS (/stage): all of them with + # "proxy", the network-error fallbacks with "direct" (PY-13). + files_staged: int = 0 + + +@dataclass(frozen=True, slots=True) +class _FailedRun: + """Why the last :meth:`FolderIngest.run` stopped and cancelled its session — + an upload failure (PY-2), or a commit failure with ``cleanup_on_failure`` — + kept for :meth:`FolderIngest.retry` (PY-8).""" + + stage: str # "upload" | "commit" + error: Exception + session_id: str + cancelled: bool # whether the cancel went through + + +def _may_succeed_on_retry(err: Exception) -> bool: + """Whether a failed upload can succeed unchanged on a second attempt: a + network-layer error, or a 5xx from S3 or DDS. DDS's own ``retryable`` flag + decides where it gives one.""" + if isinstance(err, httpx.TransportError): + return True + if isinstance(err, IngestApiError): + if err.retryable is not None: + return err.retryable + return err.status_code >= 500 + return False def scan_folder(folder: Path, data_format: DataFormat) -> list[FileSpec]: @@ -178,7 +228,8 @@ def __init__( # begin; the begin plan says which parts this server uses. self.table_definition = table_definition # DI-12.26: after a commit failure the server could retry, drop the - # session (and its uploaded files) instead of keeping it for retry(). + # session (and its uploaded files) instead of keeping it for retry(); + # a retry() then uploads again under a new session. self.cleanup_on_failure = cleanup_on_failure # Parsing hints sent on begin (the server already accepts them). self.csv_options = csv_options @@ -196,6 +247,17 @@ def __init__( # command below works from. self._session_id: str | None = None self._begin: BeginResult | None = None + # Set when run() cancelled its session after a failure; what retry() + # goes by once there is no session left (PY-8). + self._failed_run: _FailedRun | None = None + # The files the current run sent through DDS (/stage), by proxy or by + # fallback; appended to from the upload threads (list.append is atomic). + self._staged: list[str] = [] + # The largest file POST /ingest/stage accepts (PY-10): asked of DDS at + # most once per run, and only when a file may go through it. + self._stage_cap: int | None = None + self._stage_cap_asked = False + self._stage_cap_lock = threading.Lock() # The client owns the credentials; if the caller did not inject one we # build it from the kernel environment. Creds never leave the client. @@ -256,11 +318,13 @@ def run(self) -> IngestOutcome: :meth:`status`, :meth:`retry` or :meth:`cancel` the transfer. Use it as a context manager (or call :meth:`close`) to release it. """ + self._failed_run = None begin = self.begin() try: etags, multipart_etags, uploaded, skipped = self._upload_all(begin) except Exception as err: - self._cancel_after_failure(err) + cancelled = self._cancel_after_failure(err) + self._failed_run = _FailedRun("upload", err, begin.session_id, cancelled) raise try: commit = self.commit(multipart_etags=multipart_etags or None) @@ -274,6 +338,7 @@ def run(self) -> IngestOutcome: files_skipped=skipped, etags=etags, upload_mode="proxy" if self._uses_proxy(begin) else "direct", + files_staged=len(self._staged), ) def _effective_upload_mode(self, begin: BeginResult) -> str: @@ -296,6 +361,7 @@ def begin(self) -> BeginResult: The server validates the target here — an unauthorised or non-existent catalog folder fails now, before a single byte is uploaded. """ + self._check_csv_encoding_option() files = scan_folder(self.folder, self.data_format) if not files: raise FileNotFoundError( @@ -331,6 +397,7 @@ def dry_run(self) -> DryRunResult: real begin would raise, or :class:`UnsupportedByServerError` against a DDS server that predates dry runs. """ + self._check_csv_encoding_option() files = scan_folder(self.folder, self.data_format) if not files: raise FileNotFoundError( @@ -377,6 +444,13 @@ def _definition_to_send( try: columns = local_columns(self.folder, rel_paths, self.data_format, options) except (ValueError, pa.ArrowException) as exc: + # A file that is not UTF-8 would load garbled, not just untyped: say so. + bad = first_non_utf8(self.folder, rel_paths) if self.data_format == "csv" else None + if bad is not None: + raise self._encoding_error( + f"{bad!r} is not valid UTF-8, and Dremio reads CSV as UTF-8, so its " + "text would load garbled: save it as UTF-8 and run again." + ) from exc logger.warning( "could not infer the %s columns locally (%s); the server will ask for " "them — pass table_definition=…", @@ -391,6 +465,26 @@ def _definition_to_send( print(f"Inferred columns: {summary}") return inferred + def _check_csv_encoding_option(self) -> None: + """Refuse a CSV ``encoding`` other than UTF-8 before anything starts: DDS + accepts the option but passes no encoding on to Dremio, which reads CSV as + UTF-8, so such a file would load garbled rather than fail.""" + encoding = (self.csv_options or {}).get("encoding") + if self.data_format == "csv" and encoding and not is_utf8(str(encoding)): + raise self._encoding_error( + f"csv_options asks for encoding {encoding!r}, but DDS ignores it and " + "Dremio reads CSV as UTF-8, so the text would load garbled: save the " + "files as UTF-8, drop the encoding option, and run again." + ) + + @staticmethod + def _encoding_error(message: str) -> IngestApiError: + err = IngestApiError( + 400, message, where="local CSV encoding check (nothing was uploaded)" + ) + err.retryable = False + return err + def _check_parquet_columns(self, rel_paths: list[str], given: list[Mapping[str, Any]]) -> None: """Refuse given Parquet columns that contradict the local footers, before anything is uploaded — the same check the server runs at commit (DI-12.1d).""" @@ -408,6 +502,108 @@ def _check_parquet_columns(self, rel_paths: list[str], given: list[Mapping[str, err.retryable = False raise err + def diagnose(self) -> Diagnosis: + """Check, in plain language, whether this transfer would work — without + starting it (PY-7). Nothing is uploaded and no session is created. + + In order: DDS reachable; the credential accepted; the files against DDS's + size limits (PY-10); DDS's own S3 write test; and a dry run of this exact + folder, which covers the privileges on the target, the target folder, + collisions and the table definition. A check that cannot run (DDS + unreachable, a server predating it) is reported as skipped, with the + reason, instead of failing the ones after it. + """ + result = Diagnosis() + + def add(name: str, ok: bool | None, detail: str) -> None: + result.steps.append(DiagnosisStep(name, ok, detail)) + if self.show_progress: + mark = {True: "✓", False: "✗", None: "–"}[ok] + print(f"{mark} {name}: {detail}") + + try: + self._client.health() + except (httpx.HTTPError, IngestApiError) as exc: + add("DDS reachable", False, str(exc)) + for name in ( + "Credential accepted", + "File sizes", + "Storage write test", + "Dry run of this folder", + ): + add(name, None, "skipped: DDS is not reachable") + return result + add("DDS reachable", True, "the service answers") + + limits: dict[str, Any] | None = None + try: + limits = self._client.limits("notebook") + add("Credential accepted", True, "DDS accepts your Dremio credential") + except IngestApiError as exc: + if exc.status_code == 404: # a server predating GET /ingest/limits + try: + self._client.list_sessions() + add("Credential accepted", True, "DDS accepts your Dremio credential") + except IngestApiError as inner: + add("Credential accepted", False, inner.message) + else: + add("Credential accepted", False, exc.message) + + add("File sizes", *self._check_file_sizes(limits)) + + try: + verdict = self._client.storage_selftest().get("storage_write_test") or {} + if verdict.get("ok"): + add("Storage write test", True, "DDS can write to its S3 storage") + elif verdict: + failures = "; ".join(str(f) for f in verdict.get("failures") or []) or "failed" + add("Storage write test", False, failures) + else: + add("Storage write test", None, "skipped: no verdict reported yet") + except IngestApiError as exc: + add("Storage write test", None, f"skipped: {exc.message}") + + try: + dry = self.dry_run() + add("Dry run of this folder", True, dry.plan.describe()) + except UnsupportedByServerError: + add("Dry run of this folder", None, "skipped: this DDS server has no dry run") + except (IngestApiError, FileNotFoundError, ValueError) as exc: + add("Dry run of this folder", False, str(getattr(exc, "message", exc))) + return result + + def _check_file_sizes(self, limits: Mapping[str, Any] | None) -> tuple[bool | None, str]: + """:meth:`diagnose`'s size step (PY-10): this folder's files against the + caps DDS reports — ``max_bytes`` for any file of this caller, which begin + enforces, and ``stage_max_bytes`` for a file sent through DDS.""" + if limits is None: + return None, "skipped: DDS did not report its size limits" + files = scan_folder(self.folder, self.data_format) + if not files: + return None, f"skipped: no {self.data_format} files under {self.folder}" + + def over(cap: object) -> list[str]: + if not isinstance(cap, int): + return [] + return [f"{f.rel_path} ({f.size:,} bytes)" for f in files if f.size > cap] + + max_bytes, stage_max = limits.get("max_bytes"), limits.get("stage_max_bytes") + if too_big := over(max_bytes): + return False, f"larger than DDS accepts ({max_bytes:,} bytes): {', '.join(too_big)}" + if too_big_to_stage := over(stage_max): + names = ", ".join(too_big_to_stage) + if self.upload_mode == "proxy": + return False, ( + f"larger than DDS accepts through /stage ({stage_max:,} bytes), where " + f"upload_mode='proxy' sends every file: {names}" + ) + return True, ( + f"fine for a direct upload, but larger than DDS accepts through /stage " + f"({stage_max:,} bytes), so these cannot fall back to it: {names}" + ) + largest = max(files, key=lambda f: f.size) + return True, f"within DDS's limits (largest: {largest.rel_path}, {largest.size:,} bytes)" + def _report_plan(self, plan: BeginPlan) -> None: """Show what commit will do: printed when ``show_progress`` (the same switch as the progress bar), always logged; the parts of the table @@ -463,9 +659,17 @@ def cancel(self) -> None: self._client.cancel(self._session_id) self._session_id = None - def retry(self) -> IngestOutcome: + def retry( + self, *, table_definition: TableDefinition | Mapping[str, Any] | None = None + ) -> IngestOutcome: """Re-run this transfer from the step that failed. + ``table_definition`` replaces the one this transfer was given — the fix + for a commit that failed on its columns (``schema_required``, + ``schema_mismatch``). A resumed load uses it without uploading again + (DI-12.6; a server before that ignores it), and a new upload sends it on + begin. + Two cases, decided by the server's own report of where it broke: * **The load failed** (``CREATE TABLE`` / ``COPY INTO``) and the staged @@ -477,6 +681,12 @@ def retry(self) -> IngestOutcome: staged copy is gone. A fresh session is opened and the folder is uploaded again. Any ``idempotency_key`` is dropped for that attempt, or the server would just replay the failed session. + * **This object's own** :meth:`run` **cancelled its session** — after a + failed upload (PY-2), or after a failed commit with + ``cleanup_on_failure`` — so there is none left to resume. The folder is + uploaded again under a new session if the failure can pass (a network + error, a 5xx, DDS saying ``retryable``) and the cancel went through; + otherwise this raises :class:`IngestStateError` quoting the error. **A transfer whose files are stored permanently is never re-uploaded blindly.** There the upload landed in the table's own folder and stayed, @@ -490,6 +700,12 @@ def retry(self) -> IngestOutcome: Raises :class:`IngestStateError` if there is nothing to retry — no session, one that is still running, or one that already succeeded. """ + if table_definition is not None: + self.table_definition = table_definition + if self._failed_run is not None: + # That session was cancelled, or never got all its files: the server + # has nothing to resume (PY-8). + return self._rerun_after_cancel(self._failed_run) if self._session_id is None: raise IngestStateError("no session to retry — call run() first") status = self._client.get_status(self._session_id) @@ -504,7 +720,12 @@ def retry(self) -> IngestOutcome: ) if status.is_resumable: logger.info("resuming the load step of %s", self._session_id) - resumed = self._client.retry(self._session_id) + # Without a definition, the call is exactly the one made before DI-12.6. + resumed = ( + self._client.retry(self._session_id, definition=table_definition) + if table_definition is not None + else self._client.retry(self._session_id) + ) return IngestOutcome( begin=self._begin, commit=CommitResult.from_json(resumed.raw), @@ -532,6 +753,36 @@ def retry(self) -> IngestOutcome: self.idempotency_key = None return self.run() + def _rerun_after_cancel(self, failure: _FailedRun) -> IngestOutcome: + """:meth:`retry` once :meth:`run` cancelled its session after a failure + (PY-8). Uploads again only when a second attempt can succeed and nothing + of the first is left behind; otherwise raises, quoting the error, so a + caller that retries (``IngestSession.commit(retry=True)``) shows the + cause instead of "no session".""" + if not failure.cancelled: + raise IngestStateError( + f"the {failure.stage} failed ({failure.error}) and session {failure.session_id} " + "could not be cancelled, so files it uploaded may still be in place; " + "uploading again could add a second copy. Cancel it with " + "FolderIngest.attach(...).cancel(), or wait for the server's sweep, " + "then run the ingest again." + ) from failure.error + if not _may_succeed_on_retry(failure.error): + raise IngestStateError( + f"the {failure.stage} failed ({failure.error}); its session was " + "cancelled, and running it again would fail the same way. Fix the " + "cause, then run the ingest again." + ) from failure.error + logger.info( + "the %s of session %s failed (%s); uploading again under a new session", + failure.stage, + failure.session_id, + failure.error, + ) + self._begin = None + self.idempotency_key = None + return self.run() + # -- upload phase ----------------------------------------------------- def _after_failed_commit(self, err: IngestApiError) -> None: @@ -539,22 +790,27 @@ def _after_failed_commit(self, err: IngestApiError) -> None: ``cleanup_on_failure``. Best-effort: nothing here replaces ``err``. The note is the session's own failure text (:meth:`StatusResult. - describe_failure`), the same the Transfers panel shows. A failure the - server cannot retry was already cleaned up server-side; a retryable one - is kept for :meth:`retry` unless ``cleanup_on_failure`` is set. + describe_failure`), the same the Transfers panel shows, and + ``err.failed_stage`` its stage. A failure the server cannot retry was + already cleaned up server-side; a retryable one is kept for :meth:`retry` + unless ``cleanup_on_failure`` is set. """ session_id = self._session_id if session_id is None: return try: - err.add_note(self._client.get_status(session_id).describe_failure()) + status = self._client.get_status(session_id) + err.failed_stage = status.failed_stage + err.add_note(status.describe_failure()) except Exception: # noqa: BLE001 — the commit error is what matters logger.debug("could not fetch the failed session's status", exc_info=True) - if self.cleanup_on_failure and err.retryable is True: - self._cancel_after_failure(err) + if self.cleanup_on_failure and err.retryable is True and self._cancel_after_failure(err): + # The upload is gone with the session, so retry() starts over (PY-8). + self._failed_run = _FailedRun("commit", err, session_id, cancelled=True) - def _cancel_after_failure(self, err: Exception) -> None: - """Cancel the session after a failure, and say so on ``err``. + def _cancel_after_failure(self, err: Exception) -> bool: + """Cancel the session after a failure, say so on ``err``, and return + whether the cancel went through. After a failed upload (PY-2): an incomplete client upload cannot be resumed — the server only re-runs the load (:meth:`retry`) — and the @@ -567,7 +823,7 @@ def _cancel_after_failure(self, err: Exception) -> None: """ session_id = self._session_id if session_id is None: - return + return False try: self.cancel() except Exception as cancel_err: # noqa: BLE001 — reported on err instead @@ -576,11 +832,12 @@ def _cancel_after_failure(self, err: Exception) -> None: f"DDS could not cancel session {session_id} ({cancel_err}); the " "server's sweep removes its uploaded files after the session's TTL." ) - else: - err.add_note( - f"Session {session_id} was cancelled and the files it had uploaded " - "were deleted; run the ingest again." - ) + return False + err.add_note( + f"Session {session_id} was cancelled and the files it had uploaded " + "were deleted; run the ingest again." + ) + return True def _already_done(self, session_id: str) -> set[str]: """Return rel_paths already uploaded for this session (resume support). @@ -615,11 +872,15 @@ def _upload_all( ) -> tuple[dict[str, str], list[dict[str, object]], int, int]: targets = list(begin.s3.uploads) proxy = self._uses_proxy(begin) + self._staged = [] + self._stage_cap_asked = False if proxy: logger.info("uploading through DDS (proxy), not to the presigned S3 targets") done = self._already_done(begin.session_id) pending = [t for t in targets if t.rel_path not in done] skipped = len(targets) - len(pending) + if proxy: + self._refuse_files_over_the_stage_cap(pending) # Single-shot ETags (rel_path -> ETag) and multipart commit entries # ({rel_path, upload_id, parts:[{part_number, etag}]}) are collected @@ -682,7 +943,7 @@ def _upload_one( it fails at the **network** layer (unreachable endpoint, DNS, TLS, timeout), sends the file through DDS instead. An HTTP error from S3 is not retried through DDS: that is a signing or permission problem the - proxy would not fix. + proxy would not fix. Nor is a file larger than ``/stage`` accepts (PY-10). """ data = (self.folder / target.rel_path).read_bytes() mode = self._effective_upload_mode(begin) @@ -696,6 +957,13 @@ def _upload_one( except httpx.TransportError as exc: if mode != "auto": raise + cap = self._stage_limit() + if cap is not None and len(data) > cap: + exc.add_note( + f"Not sent through DDS instead: {target.rel_path!r} is {len(data):,} " + f"bytes, more than POST /api/v1/ingest/stage accepts ({cap:,} bytes)." + ) + raise logger.warning( "presigned upload of %s failed at the network layer (%s); " "sending it through DDS instead", @@ -711,8 +979,52 @@ def _stage( S3 with its own credentials. A file the server planned as multipart goes through whole; DDS accepts that at commit (DI-12.10a).""" self._client.stage(session_id, target.rel_path, data) + self._staged.append(target.rel_path) return None, None + def _stage_limit(self) -> int | None: + """The largest file ``POST /api/v1/ingest/stage`` accepts, from ``GET + /api/v1/ingest/limits`` (DI-12.11), asked at most once per run (PY-10). + + ``None`` when DDS does not say — a server before DI-12.11 answers 404 — + or the question fails: this is a check ahead of DDS's own, never a + reason to fail an upload by itself. + """ + with self._stage_cap_lock: + if not self._stage_cap_asked: + self._stage_cap_asked = True + try: + value = self._client.limits("notebook").get("stage_max_bytes") + except Exception: # noqa: BLE001 — best-effort, see the docstring + logger.debug("could not read the ingest limits", exc_info=True) + value = None + self._stage_cap = value if isinstance(value, int) else None + return self._stage_cap + + def _refuse_files_over_the_stage_cap(self, targets: list[UploadTarget]) -> None: + """With ``proxy`` every file goes through ``POST /ingest/stage``, which DDS + caps on its own and checks only when each file arrives (PY-10). Refuse an + oversized file before anything is uploaded, rather than after the files + ahead of it.""" + cap = self._stage_limit() + if cap is None: + return + too_big: list[str] = [] + for target in targets: + size = (self.folder / target.rel_path).stat().st_size + if size > cap: + too_big.append(f"{target.rel_path!r} ({size:,} bytes)") + if not too_big: + return + err = IngestApiError( + 400, + f"DDS accepts files of up to {cap:,} bytes through POST /api/v1/ingest/stage, " + f"which this upload goes through; too large: {', '.join(too_big)}", + where="local size check against GET /api/v1/ingest/limits (nothing was uploaded)", + ) + err.retryable = False + raise err + def ingest_folder( folder: str | Path, diff --git a/src/eea_datalakehouse/dds_ingestion/formats.py b/src/eea_datalakehouse/dds_ingestion/formats.py index 967666a..71780d3 100644 --- a/src/eea_datalakehouse/dds_ingestion/formats.py +++ b/src/eea_datalakehouse/dds_ingestion/formats.py @@ -15,6 +15,7 @@ from __future__ import annotations +import codecs import re from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass @@ -97,7 +98,12 @@ def _csv_schema(path: Path, options: Mapping[str, Any] | None) -> Columns: delimiter=opts.get("delimiter") or ",", quote_char=opts.get("quote") or '"' ) read = pa_csv.ReadOptions(autogenerate_column_names=opts.get("header") is False) - return _columns(pa_csv.read_csv(path, read_options=read, parse_options=parse).schema) + schema = pa_csv.read_csv(path, read_options=read, parse_options=parse).schema + # pyarrow reads text that is not valid UTF-8 as binary, and a CSV has no binary + # columns: without this, a Latin-1 file would become VARBINARY columns. + if any(pa.types.is_binary(f.type) or pa.types.is_large_binary(f.type) for f in schema): + raise ValueError(f"{path.name} is not valid UTF-8") + return _columns(schema) def _json_schema(path: Path, options: Mapping[str, Any] | None) -> Columns: @@ -125,6 +131,31 @@ class ClientFormat: } +def is_utf8(encoding: str) -> bool: + """Whether ``encoding`` names UTF-8, however it is spelled (``utf8``, + ``UTF-8``, ``utf_8``); an unknown name is not.""" + try: + return codecs.lookup(encoding).name == "utf-8" + except LookupError: + return False + + +def first_non_utf8(folder: Path, rel_paths: Sequence[str]) -> str | None: + """The first of the files that is not valid UTF-8, read in 1 MiB chunks; + ``None`` when every one is. Dremio reads CSV as UTF-8, and DDS passes no + encoding on to it.""" + for rel in rel_paths: + decoder = codecs.getincrementaldecoder("utf-8")() + with (folder / rel).open("rb") as fh: + try: + while chunk := fh.read(1 << 20): + decoder.decode(chunk) + decoder.decode(b"", final=True) + except UnicodeDecodeError: + return rel + return None + + def local_columns( folder: Path, rel_paths: Sequence[str], fmt: DataFormat, options: Mapping[str, Any] | None ) -> Columns: diff --git a/src/eea_datalakehouse/dds_ingestion/session.py b/src/eea_datalakehouse/dds_ingestion/session.py index 0e8556f..32a09b7 100644 --- a/src/eea_datalakehouse/dds_ingestion/session.py +++ b/src/eea_datalakehouse/dds_ingestion/session.py @@ -30,6 +30,7 @@ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -37,7 +38,7 @@ from .client import IngestClient from .credentials import DremioCreds from .folder import DEFAULT_PARALLELISM, FolderIngest, IngestOutcome, IngestStateError -from .models import DataFormat, Intent +from .models import DataFormat, Intent, TableDefinition, UploadMode class IngestSessionError(RuntimeError): @@ -124,12 +125,32 @@ def ingest( parallelism: int = DEFAULT_PARALLELISM, multipart: bool | None = None, show_progress: bool = True, + table_definition: TableDefinition | Mapping[str, Any] | None = None, + cleanup_on_failure: bool = False, + csv_options: Mapping[str, Any] | None = None, + json_options: Mapping[str, Any] | None = None, + upload_mode: UploadMode = "auto", ) -> IngestSession: """Queue one folder ingest — see `FolderIngest` for what each argument means. No `idempotency_key` here: unlike the catalog side, a queued ingest's own key would only ever be used once per `commit()`, so there is nothing to encapsulate — `FolderIngest` already defaults - it to `None` (a fresh session every run).""" + it to `None` (a fresh session every run). + + Each of the last five still fits on the one `%ingest` line that queues + the ingest:: + + upload_mode="proxy" # S3 is unreachable from this kernel + csv_options={"delimiter": ";"} # CSV parsing hints + json_options={"format": "array"} # a JSON array rather than JSON lines + table_definition={"columns": [{"name": "value", "type": "DOUBLE"}]} + cleanup_on_failure=True # a failed commit leaves nothing behind + + With `cleanup_on_failure=True`, `commit(retry=True)` uploads a cleaned-up + ingest again under a new session (see `FolderIngest.retry`). + `FolderIngest`'s `infer_schema` is left out on purpose: turning + inference off only makes DDS ask for the columns, which + `table_definition` gives anyway.""" self._pending.append( _QueuedIngest( folder=folder, @@ -143,11 +164,46 @@ def ingest( "parallelism": parallelism, "multipart": multipart, "show_progress": show_progress, + "table_definition": table_definition, + "cleanup_on_failure": cleanup_on_failure, + "csv_options": csv_options, + "json_options": json_options, + "upload_mode": upload_mode, }, ) ) return self + def diagnose(self) -> None: + """Check every queued ingest without running it — `FolderIngest.diagnose` + for each, one printed line per check: DDS reachable, the credential, the + file sizes, DDS's storage, and a dry run of that folder into its target. + Nothing is uploaded and the queue is kept, so a `commit()` afterwards + runs exactly these ingests.""" + if not self._pending: + print("nothing queued — queue an ingest(...) first, then diagnose") + return + for item in self._pending: + print(f"{item.describe()}:") + try: + job = FolderIngest( + item.folder, + item.target_catalog_path, + data_format=item.data_format, + client=self._client, + base_url=self._base_url, + creds=self._creds, + **{**item.kwargs, "show_progress": True}, # the lines are the point + ) + except (OSError, ValueError, RuntimeError) as exc: + # A missing folder or credential is itself the finding. + print(f"✗ cannot check it: {exc}") + continue + try: + job.diagnose() + finally: + job.close() + def commit(self, *, retry: bool = False, max_retries: int = 3) -> IngestCommitReport: """Run every queued ingest, in order. Stops at the first failure — see the module docstring: nothing already succeeded can be rolled diff --git a/src/eea_datalakehouse/notebook/magics.py b/src/eea_datalakehouse/notebook/magics.py index 461b78e..e4f87aa 100644 --- a/src/eea_datalakehouse/notebook/magics.py +++ b/src/eea_datalakehouse/notebook/magics.py @@ -18,6 +18,7 @@ %ingest ingest(folder="./bw_2026", target_catalog_path="bwd.reference", data_format="parquet", table_name="water_temperature") + %ingest diagnose %ingest commit(retry=True) %catalog use("catalog.water_management_resources.bathing_water.bwd") @@ -116,7 +117,7 @@ ), "ingest": ( '%ingest ingest(folder="./data", target_catalog_path="a.b", data_format="parquet")' - " | %ingest commit" + " | %ingest diagnose | %ingest commit" ), "sdi": '%sdi get_xml("") | %sdi resolve_series("")', "gpkg": ( @@ -298,7 +299,18 @@ ), ] _INGEST_HELP = [ - ("ingest", "Queue one folder ingest (see FolderIngest for what each argument means)."), + ( + "ingest", + "Queue one folder ingest (see FolderIngest for what each argument means). " + "upload_mode='proxy' when S3 is unreachable; csv_options={'delimiter': ';'}; " + "table_definition={'columns': [{'name': 'value', 'type': 'DOUBLE'}]} for the column " + "types of a CSV/JSON table.", + ), + ( + "diagnose", + "Check every queued ingest without running it: DDS, your credential, the file " + "sizes, DDS's storage, and a dry run. Nothing is uploaded; the queue is kept.", + ), ("commit", "Run every queued ingest in order; stops at the first failure."), ] _SDI_HELP = [ diff --git a/tests/catalog/conftest.py b/tests/catalog/conftest.py index a9b3220..a2c69c4 100644 --- a/tests/catalog/conftest.py +++ b/tests/catalog/conftest.py @@ -106,6 +106,9 @@ def __init__( self._raise_on_create_folder = raise_on_create_folder self._raise_on_delete_folder = raise_on_delete_folder self._raise_on_exists = raise_on_exists + # Seeded per-test by assignment, since only the folder-tree tests care. + self.view_definitions: dict[str, tuple[str, list[str]]] = {} + self.folder_listing: list[str] = [] def exists(self, path: str) -> bool: if self._raise_on_exists is not None: @@ -183,6 +186,20 @@ def set_tags(self, path: str, tags: list[str]) -> None: raise CatalogOperationError(f"{path!r} does not exist") self._tags[path] = list(tags) + def get_view_definition(self, path: str) -> tuple[str, list[str]] | None: + from eea_datalakehouse.catalog.errors import CatalogOperationError + + if path not in self.existing: + raise CatalogOperationError(f"{path!r} does not exist") + return self.view_definitions.get(path) + + def list_folders(self, path: str, *, recursive: bool = True) -> list[str]: + from eea_datalakehouse.catalog.errors import CatalogOperationError + + if path not in self.existing: + raise CatalogOperationError(f"{path!r} does not exist") + return list(self.folder_listing) + def ensure_folder_path(self, path: str) -> list[str]: from eea_datalakehouse.catalog.errors import CatalogOperationError diff --git a/tests/catalog/test_foldertree.py b/tests/catalog/test_foldertree.py new file mode 100644 index 0000000..40a7dd9 --- /dev/null +++ b/tests/catalog/test_foldertree.py @@ -0,0 +1,688 @@ +"""foldertree — scanning a subtree, rewriting view SQL, copying with proof, +and only then removing the source.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from eea_datalakehouse.catalog import foldertree, retry_state +from eea_datalakehouse.catalog.errors import CatalogOperationError +from eea_datalakehouse.catalog.foldertree import ( + foldercopy, + foldermove, + folderscan, + remap_view_sql, +) +from eea_datalakehouse.catalog.operations import CatalogEntry, TableInfo, ViewDefinition + + +@pytest.fixture(autouse=True) +def _isolated_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Never touch the real ~/.cache retry-state file.""" + state_path = tmp_path / "state.json" + monkeypatch.setattr(retry_state, "DEFAULT_STATE_PATH", state_path) + return state_path + + +class FakeCatalog: + """In-memory stand-in for `Catalog`, holding a whole little catalog. + + `tables` maps a dot-path to a row count, `views` maps one to + ``(sql, context)``, `folders` is a set of folder paths. Everything + `foldertree` calls is a real method here, so a test asserts against the + catalog's *state* afterwards rather than against a list of SQL strings — + which is what makes "did the move actually move it" checkable. + """ + + def __init__( + self, + tables: dict[str, int] | None = None, + views: dict[str, tuple[str, tuple[str, ...]]] | None = None, + folders: set[str] | None = None, + wikis: dict[str, str] | None = None, + tags: dict[str, list[str]] | None = None, + fail_copy_of: set[str] | None = None, + short_copy_of: set[str] | None = None, + ) -> None: + self.tables = dict(tables or {}) + self.views = dict(views or {}) + self.folders = set(folders or set()) + self.wikis = dict(wikis or {}) + self.tags = dict(tags or {}) + self.calls: list[tuple[str, str]] = [] + self.keys: list[str] = [] + self._fail_copy_of = set(fail_copy_of or set()) + self._short_copy_of = set(short_copy_of or set()) + + # -- helpers --------------------------------------------------------- + def _record(self, operation: str, path: str, idempotency_key: str) -> None: + self.calls.append((operation, path)) + self.keys.append(idempotency_key) + + def _under(self, prefix: str, paths) -> list[str]: + return [p for p in paths if p == prefix or p.startswith(f"{prefix}.")] + + # -- discovery ------------------------------------------------------- + def getentriesfrom(self, schema_path: str, *, idempotency_key: str) -> list[CatalogEntry]: + self._record("getentriesfrom", schema_path, idempotency_key) + tables = self._under(schema_path, self.tables) + views = self._under(schema_path, self.views) + entries = [CatalogEntry(path=p, kind="TABLE") for p in tables] + entries += [CatalogEntry(path=p, kind="VIEW") for p in views] + return sorted(entries, key=lambda entry: entry.path) + + def getfoldersfrom( + self, path: str, *, recursive: bool = True, idempotency_key: str + ) -> list[str]: + self._record("getfoldersfrom", path, idempotency_key) + if path not in self.folders: + raise CatalogOperationError(f"{path!r} does not exist") + found = [p for p in self.folders if p.startswith(f"{path}.")] + return sorted(found, key=lambda p: (p.count("."), p)) + + def entryexists(self, path: str, *, idempotency_key: str) -> bool: + self._record("entryexists", path, idempotency_key) + return path in self.tables or path in self.views + + def getviewsql(self, view_path: str, *, idempotency_key: str) -> ViewDefinition: + self._record("getviewsql", view_path, idempotency_key) + if view_path not in self.views: + raise CatalogOperationError(f"{view_path!r} is not a view") + sql, context = self.views[view_path] + return ViewDefinition(sql=sql, context=context) + + def gettableitemsfrom(self, path: str, *, idempotency_key: str) -> TableInfo: + self._record("gettableitemsfrom", path, idempotency_key) + if path in self.tables: + return TableInfo(schema={"c": "INTEGER"}, row_count=self.tables[path]) + if path in self.views: + return TableInfo(schema={"c": "INTEGER"}, row_count=self._view_rows(path)) + raise CatalogOperationError(f"{path!r} does not exist") + + def _view_rows(self, view_path: str) -> int: + """A view counts whatever the first thing its SQL names counts. + + Quotes are stripped before matching, so a rewritten + ``"bwd"."archive"."raw"."t"`` really does resolve to ``bwd.archive.raw.t`` + — which is what makes the row-count check on a copied view mean + something. A view naming nothing that exists counts 0, the way a broken + rewrite would show up. + """ + seen: set[str] = set() + path, sql = view_path, self.views[view_path][0].replace('"', "") + while path not in seen: + seen.add(path) + for table, count in self.tables.items(): + if table.lower() in sql.lower(): + return count + nested = next((v for v in self.views if v != path and v.lower() in sql.lower()), None) + if nested is None: + return 0 + path, sql = nested, self.views[nested][0].replace('"', "") + return 0 + + # -- writes ---------------------------------------------------------- + def createfolder( + self, path: str, *, create_parents: bool = False, idempotency_key: str + ) -> None: + self._record("createfolder", path, idempotency_key) + self.folders.add(path) + + def datacopy( + self, + source_path: str, + target_path: str, + *, + overwrite: bool = False, + create_target_folder: bool = False, + idempotency_key: str, + ) -> None: + self._record("datacopy", target_path, idempotency_key) + if source_path in self._fail_copy_of: + raise CatalogOperationError(f"copy of {source_path!r} refused") + rows = self.tables[source_path] + self.tables[target_path] = rows - 1 if source_path in self._short_copy_of else rows + + def createviewas( + self, + view_path: str, + sql: str, + *, + overwrite: bool = False, + create_target_folder: bool = True, + idempotency_key: str, + ) -> None: + self._record("createviewas", view_path, idempotency_key) + self.views[view_path] = (sql, ()) + + def deletetable(self, table_path: str, *, idempotency_key: str) -> None: + self._record("deletetable", table_path, idempotency_key) + self.tables.pop(table_path, None) + + def deleteview(self, view_path: str, *, idempotency_key: str) -> None: + self._record("deleteview", view_path, idempotency_key) + self.views.pop(view_path, None) + + def deletefolder(self, path: str, *, cascade: bool = False, idempotency_key: str) -> None: + self._record("deletefolder", path, idempotency_key) + still_inside = [ + p + for p in (*self.tables, *self.views, *self.folders) + if p.startswith(f"{path}.") + ] + if still_inside: + raise CatalogOperationError(f"folder {path!r} is not empty: {sorted(still_inside)}") + self.folders.discard(path) + + # -- metadata -------------------------------------------------------- + def getwikifrom(self, path: str, *, idempotency_key: str) -> str: + if path not in self.wikis: + raise CatalogOperationError(f"{path!r} has no wiki") + return self.wikis[path] + + def setwikito(self, path: str, text: str, *, idempotency_key: str, **_: Any) -> None: + self.wikis[path] = text + + def gettagsfrom(self, path: str, *, idempotency_key: str) -> list[str]: + return self.tags.get(path, []) + + def settagsto(self, path: str, tags: list[str], *, idempotency_key: str) -> None: + self.tags[path] = list(tags) + + +def make_catalog(**overrides: Any) -> FakeCatalog: + """The tree every test below starts from, unless it says otherwise. + + bwd.draft (folder) + bwd.draft.raw (folder) + bwd.draft.raw.measurements 100 rows + bwd.draft.raw.stations 10 rows + bwd.draft.assessments (view over raw.measurements) + bwd.draft.summary (view over the assessments view) + """ + base: dict[str, Any] = { + "folders": {"bwd", "bwd.draft", "bwd.draft.raw", "bwd.archive"}, + "tables": {"bwd.draft.raw.measurements": 100, "bwd.draft.raw.stations": 10}, + "views": { + "bwd.draft.assessments": ( + 'SELECT * FROM "bwd"."draft"."raw"."measurements" WHERE ok = true', + (), + ), + "bwd.draft.summary": ('SELECT * FROM "bwd"."draft"."assessments"', ()), + }, + } + base.update(overrides) + return FakeCatalog(**base) + + +# --------------------------------------------------------------------------- +# remap_view_sql +# --------------------------------------------------------------------------- + + +ENTRIES = { + "bwd.draft.raw.measurements": "bwd.archive.raw.measurements", + "bwd.draft.assessments": "bwd.archive.assessments", +} + + +def remap(sql: str, **kwargs: Any): + return remap_view_sql( + sql, source_path="bwd.draft", target_path="bwd.archive", entries=ENTRIES, **kwargs + ) + + +def test_rewrites_a_fully_qualified_reference_into_the_target() -> None: + result = remap('SELECT * FROM "bwd"."draft"."raw"."measurements"') + + assert result.sql == 'SELECT * FROM "bwd"."archive"."raw"."measurements"' + assert result.references == ("bwd.draft.raw.measurements",) + + +def test_rewrites_an_unquoted_reference_and_keeps_the_column_it_qualifies() -> None: + result = remap("SELECT bwd.draft.assessments.value FROM bwd.draft.assessments") + + assert result.sql == ( + 'SELECT "bwd"."archive"."assessments"."value" FROM "bwd"."archive"."assessments"' + ) + + +def test_matches_paths_case_insensitively() -> None: + assert remap('SELECT * FROM "BWD"."Draft"."raw"."Measurements"').sql == ( + 'SELECT * FROM "bwd"."archive"."raw"."measurements"' + ) + + +def test_leaves_references_outside_the_moved_subtree_alone() -> None: + sql = 'SELECT * FROM "other"."space"."t" JOIN "bwd"."draft"."assessments" ON 1 = 1' + + assert remap(sql).sql == ( + 'SELECT * FROM "other"."space"."t" JOIN "bwd"."archive"."assessments" ON 1 = 1' + ) + + +def test_never_rewrites_inside_a_string_literal_or_a_comment() -> None: + sql = ( + "SELECT 'bwd.draft.raw.measurements' AS label, " + "'it''s bwd.draft here too' AS quoted " + "-- and bwd.draft.assessments in a comment\n" + "/* and bwd.draft.assessments in a block */ " + 'FROM "bwd"."draft"."assessments"' + ) + result = remap(sql) + + assert "'bwd.draft.raw.measurements'" in result.sql + assert "'it''s bwd.draft here too'" in result.sql + assert "-- and bwd.draft.assessments in a comment" in result.sql + assert "/* and bwd.draft.assessments in a block */" in result.sql + assert result.sql.endswith('FROM "bwd"."archive"."assessments"') + + +def test_does_not_read_a_decimal_number_as_a_path() -> None: + assert remap("SELECT 1.5 AS x FROM t").sql == "SELECT 1.5 AS x FROM t" + + +def test_spells_out_a_context_relative_reference_into_the_subtree() -> None: + # The view said `FROM assessments`, resolved against bwd.draft. After the + # move it has no such context, so the reference has to be written in full. + result = remap("SELECT * FROM assessments", context=("bwd", "draft")) + + assert result.sql == 'SELECT * FROM "bwd"."archive"."assessments"' + assert result.references == ("bwd.draft.assessments",) + + +def test_qualifies_a_context_relative_reference_that_is_not_moving() -> None: + probed: list[str] = [] + + def exists(path: str) -> bool: + probed.append(path) + return path == "bwd.reference.codelist" + + result = remap( + "SELECT * FROM reference.codelist", + context=("bwd",), + external_exists=exists, + ) + + assert result.sql == 'SELECT * FROM "bwd"."reference"."codelist"' + assert probed == ["bwd.reference.codelist"] + assert result.notes # it says what it did + + +def test_keeps_the_column_on_a_context_relative_reference() -> None: + result = remap("SELECT assessments.value FROM assessments", context=("bwd", "draft")) + + assert result.sql == ( + 'SELECT "bwd"."archive"."assessments"."value" FROM "bwd"."archive"."assessments"' + ) + + +def test_does_not_probe_when_no_external_exists_is_given() -> None: + assert remap("SELECT * FROM reference.codelist", context=("bwd",)).sql == ( + "SELECT * FROM reference.codelist" + ) + + +def test_says_so_when_the_context_is_unknown_and_a_bare_name_is_used() -> None: + result = remap("SELECT * FROM assessments") + + assert result.sql == "SELECT * FROM assessments" + assert any("sqlContext is unknown" in note for note in result.notes) + + +def test_falls_back_to_the_source_root_for_a_path_the_scan_did_not_list() -> None: + # A table created between the scan and the rewrite is still under the + # source root, so its prefix is swapped anyway. + assert remap('SELECT * FROM "bwd"."draft"."raw"."brand_new"').sql == ( + 'SELECT * FROM "bwd"."archive"."raw"."brand_new"' + ) + + +# --------------------------------------------------------------------------- +# folderscan +# --------------------------------------------------------------------------- + + +def test_scan_finds_folders_tables_and_views_and_maps_them_to_the_target() -> None: + tree = folderscan(make_catalog(), "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert [entry.source_path for entry in tree.folders] == ["bwd.draft.raw"] + assert [entry.target_path for entry in tree.folders] == ["bwd.archive.2025.raw"] + assert [entry.source_path for entry in tree.tables] == [ + "bwd.draft.raw.measurements", + "bwd.draft.raw.stations", + ] + assert [entry.target_path for entry in tree.tables] == [ + "bwd.archive.2025.raw.measurements", + "bwd.archive.2025.raw.stations", + ] + assert len(tree) == 5 + + +def test_scan_puts_a_view_after_the_view_it_selects_from() -> None: + tree = folderscan(make_catalog(), "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + ordered = [entry.source_path for entry in tree.views] + assert ordered.index("bwd.draft.assessments") < ordered.index("bwd.draft.summary") + summary = next(e for e in tree.views if e.source_path == "bwd.draft.summary") + assert summary.depends_on == ("bwd.draft.assessments",) + assert summary.remapped_sql == 'SELECT * FROM "bwd"."archive"."2025"."assessments"' + + +def test_scan_derives_folders_from_entry_paths_when_rest_cannot_list_them() -> None: + catalog = make_catalog(folders=set()) # REST answers "does not exist" + tree = folderscan(catalog, "bwd.draft", "bwd.archive", idempotency_key="k") + + assert [entry.source_path for entry in tree.folders] == ["bwd.draft.raw"] + assert any("rebuilt from the paths" in note for note in tree.notes) + + +def test_scan_refuses_a_target_inside_the_source() -> None: + with pytest.raises(CatalogOperationError, match="inside source"): + folderscan(make_catalog(), "bwd.draft", "bwd.draft.archive", idempotency_key="k") + + +def test_scan_refuses_a_source_inside_the_target() -> None: + with pytest.raises(CatalogOperationError, match="inside target"): + folderscan(make_catalog(), "bwd.draft.raw", "bwd.draft", idempotency_key="k") + + +def test_scan_refuses_the_same_path_twice() -> None: + with pytest.raises(CatalogOperationError, match="same folder"): + folderscan(make_catalog(), "bwd.draft", "BWD.Draft", idempotency_key="k") + + +def test_scan_raises_when_the_source_cannot_be_read_at_all() -> None: + with pytest.raises(CatalogOperationError, match="could not read"): + folderscan(FakeCatalog(), "bwd.nothing", "bwd.archive", idempotency_key="k") + + +# --------------------------------------------------------------------------- +# foldercopy +# --------------------------------------------------------------------------- + + +def test_dry_run_writes_nothing_and_counts_the_source() -> None: + catalog = make_catalog() + report = foldercopy( + catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k", dry_run=True + ) + + assert [row.status for row in report.results] == ["planned"] * 5 + assert [row.source_rows for row in report.tables] == [100, 10] + assert report.verified is False + assert not any(op in {"datacopy", "createviewas", "createfolder"} for op, _ in catalog.calls) + + +def test_copy_reproduces_the_tree_and_verifies_by_row_count() -> None: + catalog = make_catalog() + report = foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert report.verified is True + assert catalog.tables["bwd.archive.2025.raw.measurements"] == 100 + assert catalog.tables["bwd.archive.2025.raw.stations"] == 10 + assert [(row.source_rows, row.target_rows) for row in report.tables] == [(100, 100), (10, 10)] + # the copied view points at the copy, not back at the original + assert catalog.views["bwd.archive.2025.assessments"][0] == ( + 'SELECT * FROM "bwd"."archive"."2025"."raw"."measurements" WHERE ok = true' + ) + # and it resolves: both sides of every view counted the same real rows, + # rather than both quietly counting nothing + assert [(row.source_rows, row.target_rows) for row in report.views] == [(100, 100), (100, 100)] + # and the source is untouched + assert catalog.tables["bwd.draft.raw.measurements"] == 100 + assert "bwd.draft.assessments" in catalog.views + + +def test_copy_fails_the_entry_when_the_counts_disagree() -> None: + catalog = make_catalog(short_copy_of={"bwd.draft.raw.stations"}) + report = foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert report.verified is False + stations = next(r for r in report.tables if r.entry.name == "stations") + assert (stations.source_rows, stations.target_rows) == (10, 9) + assert stations.counts_match is False + + +def test_one_failed_table_does_not_stop_the_rest_of_the_sweep() -> None: + catalog = make_catalog(fail_copy_of={"bwd.draft.raw.measurements"}) + report = foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert [row.status for row in report.tables] == ["failed", "copied"] + assert catalog.tables["bwd.archive.2025.raw.stations"] == 10 + assert report.verified is False + + +def test_a_stale_view_already_at_the_target_is_caught_by_the_count() -> None: + # Something left over from an earlier, different job sits where this view + # should go. skip_existing leaves it alone — but counting both sides is + # what stops that from passing for a copy. + catalog = make_catalog() + stale_sql = 'SELECT * FROM "bwd"."draft"."raw"."stations"' + catalog.views["bwd.archive.2025.assessments"] = (stale_sql, ()) + + report = foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + stale = next(r for r in report.views if r.entry.name == "assessments") + assert stale.status == "failed" + assert (stale.source_rows, stale.target_rows) == (100, 10) + assert "not equivalent" in (stale.error or "") + assert report.verified is False + + +def test_views_are_not_counted_when_verify_views_is_off() -> None: + catalog = make_catalog() + report = foldercopy( + catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k", verify_views=False + ) + + assert [row.source_rows for row in report.views] == [None, None] + assert report.verified is True + + +def test_re_running_skips_what_already_landed() -> None: + catalog = make_catalog() + foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + catalog.calls.clear() + + report = foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert [row.status for row in report.tables] == ["skipped", "skipped"] + assert [row.status for row in report.views] == ["skipped", "skipped"] + assert report.verified is True + assert not any(op == "datacopy" for op, _ in catalog.calls) + + +def test_an_existing_target_with_a_different_count_fails_rather_than_being_accepted() -> None: + catalog = make_catalog() + catalog.tables["bwd.archive.2025.raw.measurements"] = 7 + + report = foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + failed = next(r for r in report.tables if r.entry.name == "measurements") + assert failed.status == "failed" + assert "overwrite=True" in (failed.error or "") + assert catalog.tables["bwd.archive.2025.raw.measurements"] == 7 # left alone + + +def test_overwrite_replaces_an_existing_target() -> None: + catalog = make_catalog() + catalog.tables["bwd.archive.2025.raw.measurements"] = 7 + + report = foldercopy( + catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k", overwrite=True + ) + + assert report.verified is True + assert catalog.tables["bwd.archive.2025.raw.measurements"] == 100 + + +def test_metadata_travels_with_the_entries() -> None: + catalog = make_catalog( + wikis={"bwd.draft.raw": "the raw landing zone"}, + tags={"bwd.draft.raw.measurements": ["bronze"]}, + ) + foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert catalog.wikis["bwd.archive.2025.raw"] == "the raw landing zone" + assert catalog.tags["bwd.archive.2025.raw.measurements"] == ["bronze"] + + +def test_every_entry_gets_its_own_idempotency_key() -> None: + catalog = make_catalog() + foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="job-1") + + copy_keys = [ + key for (op, _), key in zip(catalog.calls, catalog.keys, strict=True) if op == "datacopy" + ] + assert copy_keys == [ + "job-1:datacopy:bwd.archive.2025.raw.measurements", + "job-1:datacopy:bwd.archive.2025.raw.stations", + ] + + +def test_a_prepared_tree_is_applied_without_a_second_scan() -> None: + catalog = make_catalog() + tree = folderscan(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + catalog.calls.clear() + + foldercopy(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k", tree=tree) + + assert not any(op == "getentriesfrom" for op, _ in catalog.calls) + + +# --------------------------------------------------------------------------- +# foldermove +# --------------------------------------------------------------------------- + + +def test_move_removes_the_source_only_after_a_verified_copy() -> None: + catalog = make_catalog() + report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert report.verified is True + assert report.source_deleted is True + assert not [p for p in catalog.tables if p.startswith("bwd.draft")] + assert not [p for p in catalog.views if p.startswith("bwd.draft")] + assert "bwd.draft" not in catalog.folders + assert "bwd.draft.raw" not in catalog.folders + # everything is where it should be on the other side + assert catalog.tables["bwd.archive.2025.raw.measurements"] == 100 + assert "bwd.archive.2025.summary" in catalog.views + + +def test_move_takes_the_source_apart_in_a_deletable_order() -> None: + catalog = make_catalog() + foldermove(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + removals = [(op, path) for op, path in catalog.calls if op.startswith("delete")] + order = [path for _, path in removals] + # dependent view before the one it selects from; folders last, deepest first + assert order.index("bwd.draft.summary") < order.index("bwd.draft.assessments") + assert order.index("bwd.draft.raw.measurements") < order.index("bwd.draft.raw") + assert order.index("bwd.draft.raw") < order.index("bwd.draft") + assert order[-1] == "bwd.draft" + + +def test_move_leaves_the_source_alone_when_the_copy_did_not_verify() -> None: + catalog = make_catalog(short_copy_of={"bwd.draft.raw.stations"}) + report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert report.verified is False + assert report.source_deleted is False + assert report.deleted == [] + assert catalog.tables["bwd.draft.raw.stations"] == 10 + assert "bwd.draft" in catalog.folders + assert any("left exactly as it was" in note for note in report.notes) + + +def test_move_can_keep_the_source_root_folder() -> None: + catalog = make_catalog() + foldermove( + catalog, + "bwd.draft", + "bwd.archive.2025", + idempotency_key="k", + delete_source_root=False, + ) + + assert "bwd.draft" in catalog.folders + assert not [p for p in catalog.tables if p.startswith("bwd.draft.")] + + +def test_move_dry_run_removes_nothing() -> None: + catalog = make_catalog() + report = foldermove( + catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k", dry_run=True + ) + + assert report.source_deleted is False + assert catalog.tables["bwd.draft.raw.measurements"] == 100 + assert any("dry run" in note for note in report.notes) + + +def test_move_reports_a_source_folder_it_could_not_empty() -> None: + catalog = make_catalog() + # Something nobody told us about turns up inside the source mid-move. + original_deletetable = catalog.deletetable + + def deletetable(table_path: str, *, idempotency_key: str) -> None: + original_deletetable(table_path, idempotency_key=idempotency_key) + catalog.tables.setdefault("bwd.draft.raw.surprise", 1) + + catalog.deletetable = deletetable # type: ignore[method-assign] + report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + + assert report.source_deleted is False + assert [path for path, _ in report.delete_errors] == ["bwd.draft.raw", "bwd.draft"] + assert any("could not be removed" in note for note in report.notes) + + +def test_summary_reads_as_a_report() -> None: + catalog = make_catalog() + report = foldermove(catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k") + text = report.summary() + + assert "RESULT: bwd.draft -> bwd.archive.2025" in text + assert "[100 == 100 rows]" in text + assert "verified: True" in text + assert "removed from the source" in text + + +def test_progress_is_reported_as_it_goes() -> None: + messages: list[str] = [] + foldermove( + make_catalog(), + "bwd.draft", + "bwd.archive.2025", + idempotency_key="k", + on_progress=messages.append, + ) + + assert any(message.startswith("scanning") for message in messages) + assert any("copy verified" in message for message in messages) + assert any(message.startswith("removed bwd.draft") for message in messages) + + +def test_module_exports_the_three_verbs() -> None: + assert foldertree.folderscan is folderscan + assert foldertree.foldercopy is foldercopy + assert foldertree.foldermove is foldermove + + +def test_nothing_at_the_target_is_destroyed_without_overwrite() -> None: + # skip_existing=False says "don't accept what's there", not "replace it". + catalog = make_catalog() + catalog.tables["bwd.archive.2025.raw.measurements"] = 100 # same count, still not ours to drop + + report = foldercopy( + catalog, "bwd.draft", "bwd.archive.2025", idempotency_key="k", skip_existing=False + ) + + occupied = next(r for r in report.tables if r.entry.name == "measurements") + assert occupied.status == "failed" + assert "overwrite=True" in (occupied.error or "") + assert not any(op == "datacopy" and path.endswith("measurements") for op, path in catalog.calls) diff --git a/tests/catalog/test_operations.py b/tests/catalog/test_operations.py index 7236e3e..baa2fd0 100644 --- a/tests/catalog/test_operations.py +++ b/tests/catalog/test_operations.py @@ -1437,3 +1437,126 @@ def test_deletefolder_engine_starting_is_remembered_and_retryable(executor) -> N fake_rest._raise_on_delete_folder = None operations.retry_pending(executor, "k", catalog_rest=fake_rest) assert fake_rest.deleted == ["a.b"] + + +# --- getentriesfrom / entryexists / getviewsql / createviewas / deletetable --- +# --- getfoldersfrom ---------------------------------------------------------- + + +def test_getentriesfrom_returns_each_path_with_its_kind() -> None: + fake = FakeExecutor( + rows=[ + {"TABLE_SCHEMA": "bwd.draft", "TABLE_NAME": "v", "TABLE_TYPE": "VIEW"}, + {"TABLE_SCHEMA": "bwd.draft.raw", "TABLE_NAME": "t", "TABLE_TYPE": "TABLE"}, + ] + ) + + entries = operations.getentriesfrom(fake, "bwd.draft", idempotency_key="k") + + assert [(entry.path, entry.kind) for entry in entries] == [ + ("bwd.draft.raw.t", "TABLE"), + ("bwd.draft.v", "VIEW"), + ] + assert '"TABLE_TYPE"' in fake.statements[0] + assert "LIKE 'bwd.draft.%' ESCAPE" in fake.statements[0] + + +def test_getentriesfrom_reads_anything_that_is_not_a_view_as_a_table() -> None: + fake = FakeExecutor( + rows=[{"TABLE_SCHEMA": "a", "TABLE_NAME": "t", "TABLE_TYPE": "SYSTEM_TABLE"}] + ) + + assert operations.getentriesfrom(fake, "a", idempotency_key="k")[0].kind == "TABLE" + + +def test_entryexists_answers_from_information_schema() -> None: + assert operations.entryexists( + FakeExecutor(rows=[{"TABLE_NAME": "t"}]), "a.t", idempotency_key="k" + ) + assert not operations.entryexists(FakeExecutor(rows=[]), "a.t", idempotency_key="k") + + +def test_getviewsql_prefers_the_catalog_entity_and_keeps_its_context() -> None: + fake = FakeExecutor() + catalog_rest = FakeCatalogRest(existing={"a", "a.v"}) + catalog_rest.view_definitions = {"a.v": ("SELECT 1", ["a"])} # type: ignore[attr-defined] + + definition = operations.getviewsql(fake, "a.v", catalog_rest=catalog_rest, idempotency_key="k") + + assert definition.sql == "SELECT 1" + assert definition.context == ("a",) + assert fake.statements == [] # never had to ask INFORMATION_SCHEMA + + +def test_getviewsql_falls_back_to_information_schema_without_a_context() -> None: + fake = FakeExecutor(rows=[{"VIEW_DEFINITION": "SELECT * FROM t"}]) + + definition = operations.getviewsql(fake, "a.v", idempotency_key="k") + + assert definition.sql == "SELECT * FROM t" + assert definition.context == () + assert 'INFORMATION_SCHEMA."VIEWS"' in fake.statements[0] + + +def test_getviewsql_raises_rather_than_guessing_a_definition() -> None: + with pytest.raises(CatalogOperationError, match="could not read the view definition"): + operations.getviewsql(FakeExecutor(rows=[]), "a.v", idempotency_key="k") + + +def test_createviewas_sends_the_query_body_through_unchanged(catalog_rest) -> None: + fake = FakeExecutor(rows=[]) # nothing at the target yet + + operations.createviewas( + fake, "a.v", 'SELECT * FROM "b"."t"', catalog_rest=catalog_rest, idempotency_key="k" + ) + + assert fake.statements[-1] == 'CREATE VIEW "a"."v" AS SELECT * FROM "b"."t"' + + +def test_createviewas_refuses_an_occupied_target_unless_told_to_overwrite(catalog_rest) -> None: + fake = FakeExecutor(rows=[{"TABLE_NAME": "v"}]) + + with pytest.raises(CatalogOperationError, match="overwrite=True"): + operations.createviewas( + fake, "a.v", "SELECT 1", catalog_rest=catalog_rest, idempotency_key="k" + ) + + +def test_createviewas_drops_whatever_kind_is_actually_there_first(catalog_rest) -> None: + fake = FakeExecutor( + rows_sequence=[[{"TABLE_NAME": "v"}], [{"TABLE_TYPE": "BASE TABLE"}]] + ) + + operations.createviewas( + fake, + "a.v", + "SELECT 1", + overwrite=True, + catalog_rest=catalog_rest, + idempotency_key="k", + ) + + assert fake.statements[-2:] == [ + 'DROP TABLE IF EXISTS "a"."v"', + 'CREATE VIEW "a"."v" AS SELECT 1', + ] + + +def test_deletetable_is_the_counterpart_of_deleteview() -> None: + fake = FakeExecutor() + + operations.deletetable(fake, "a.t", idempotency_key="k") + + assert fake.statements == ['DROP TABLE IF EXISTS "a"."t"'] + + +def test_getfoldersfrom_delegates_to_the_rest_catalog() -> None: + catalog_rest = FakeCatalogRest(existing={"a"}) + catalog_rest.folder_listing = ["a.b", "a.b.c"] # type: ignore[attr-defined] + + assert operations.getfoldersfrom(catalog_rest, "a", idempotency_key="k") == ["a.b", "a.b.c"] + + +def test_the_new_operations_can_be_replayed_by_retry_pending() -> None: + for name in ("getentriesfrom", "entryexists", "getviewsql", "createviewas", "deletetable"): + assert name in operations._OPERATIONS diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py index a4f8d26..98d6ed1 100644 --- a/tests/catalog/test_rest.py +++ b/tests/catalog/test_rest.py @@ -865,3 +865,101 @@ def test_list_datasets_refused_token_raises_auth_error() -> None: with pytest.raises(CatalogAuthError): CatalogRestClient(BASE_URL, "expired").list_datasets(["s", "top"]) + + +# --- children / list_folders / get_view_definition -------------------------- + + +def _folder_child(*path: str) -> dict[str, object]: + return { + "id": "-".join(path), + "path": list(path), + "type": "CONTAINER", + "containerType": "FOLDER", + } + + +def _dataset_child(*path: str) -> dict[str, object]: + return {"id": "-".join(path), "path": list(path), "type": "DATASET"} + + +@respx.mock +def test_children_returns_the_entities_directly_inside_a_folder() -> None: + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a/b").mock( + return_value=httpx.Response( + 200, + json={ + "entityType": "folder", + "children": [_folder_child("a", "b", "sub"), _dataset_child("a", "b", "t")], + }, + ) + ) + client = CatalogRestClient(BASE_URL, "pat") + + assert [child["id"] for child in client.children("a.b")] == ["a-b-sub", "a-b-t"] + + +@respx.mock +def test_children_raises_rather_than_calling_a_missing_folder_empty() -> None: + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a/gone").mock(return_value=httpx.Response(404)) + client = CatalogRestClient(BASE_URL, "pat") + + with pytest.raises(CatalogOperationError, match="does not exist"): + client.children("a.gone") + + +@respx.mock +def test_list_folders_walks_the_subtree_shallowest_first() -> None: + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a").mock( + return_value=httpx.Response( + 200, + json={"children": [_folder_child("a", "b"), _dataset_child("a", "t")]}, + ) + ) + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a/b").mock( + return_value=httpx.Response(200, json={"children": [_folder_child("a", "b", "c")]}) + ) + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a/b/c").mock( + return_value=httpx.Response(200, json={"children": []}) + ) + client = CatalogRestClient(BASE_URL, "pat") + + assert client.list_folders("a") == ["a.b", "a.b.c"] + + +@respx.mock +def test_list_folders_can_stay_at_the_top_level() -> None: + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a").mock( + return_value=httpx.Response(200, json={"children": [_folder_child("a", "b")]}) + ) + client = CatalogRestClient(BASE_URL, "pat") + + assert client.list_folders("a", recursive=False) == ["a.b"] + + +@respx.mock +def test_get_view_definition_returns_the_sql_and_its_context() -> None: + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a/v").mock( + return_value=httpx.Response( + 200, + json={ + "entityType": "dataset", + "type": "VIRTUAL_DATASET", + "sql": "SELECT * FROM t", + "sqlContext": ["a", "b"], + }, + ) + ) + client = CatalogRestClient(BASE_URL, "pat") + + assert client.get_view_definition("a.v") == ("SELECT * FROM t", ["a", "b"]) + + +@respx.mock +def test_get_view_definition_is_none_for_something_that_is_not_a_view() -> None: + respx.get(f"{BASE_URL}/api/v3/catalog/by-path/a/t").mock( + return_value=httpx.Response(200, json={"entityType": "dataset", "type": "PHYSICAL_DATASET"}) + ) + client = CatalogRestClient(BASE_URL, "pat") + + assert client.get_view_definition("a.t") is None diff --git a/tests/dds_ingestion/test_definition_and_plan.py b/tests/dds_ingestion/test_definition_and_plan.py index 834b717..00428b1 100644 --- a/tests/dds_ingestion/test_definition_and_plan.py +++ b/tests/dds_ingestion/test_definition_and_plan.py @@ -1,4 +1,5 @@ -"""PY-3 (optional table definition) and PY-4 (show the plan, dry run). +"""PY-3 (optional table definition), PY-4 (show the plan, dry run) and PY-9 +(retry with a corrected definition). Companion to the DDS DI-12 plan; see docs/dds-ingest-robustness-client-plan.md. """ @@ -22,6 +23,7 @@ ColumnDef, DryRunResult, FileSpec, + StatusResult, TableDefinition, definition_payload, ) @@ -266,3 +268,91 @@ def test_folder_dry_run_uploads_nothing_and_shows_the_plan( assert client.begin_calls == [] and client.uploaded == [] and client.commit_calls == [] assert job.session_id is None assert "Importing as a new editable table" in capsys.readouterr().out + + +# --- PY-9: retry with a corrected definition ---------------------------------------- + +DEFINITION_JSON = {"columns": [{"name": "a", "type": "INT", "nullable": False}]} + + +@respx.mock +def test_retry_without_a_definition_sends_todays_request(creds: DremioCreds) -> None: + """Golden: no body, as before DI-12.6.""" + route = respx.post(f"{BASE_URL}/api/v1/ingest/sess-1/retry").mock( + return_value=httpx.Response(200, json={"session_id": "sess-1", "status": "committing"}) + ) + with IngestClient(BASE_URL, creds) as client: + client.retry("sess-1") + request = route.calls.last.request + assert request.read() == b"" + assert "content-type" not in request.headers + + +@respx.mock +def test_retry_sends_a_corrected_definition(creds: DremioCreds) -> None: + route = respx.post(f"{BASE_URL}/api/v1/ingest/sess-1/retry").mock( + return_value=httpx.Response(200, json={"session_id": "sess-1", "status": "committing"}) + ) + with IngestClient(BASE_URL, creds) as client: + client.retry("sess-1", definition=DEFINITION) + assert json.loads(route.calls.last.request.read()) == {"definition": DEFINITION_JSON} + + +class FailedSessionClient(PlanClient): + """A session that failed with ``schema_required``; ``retry`` records its arguments.""" + + def __init__(self, *, resumable: bool) -> None: + super().__init__(plan=PLAN_JSON) + self.status_payload = { + "status": "failed", + "progress": {}, + "failed_stage": "load" if resumable else "upload", + "resumable": resumable, + "error_code": "schema_required", + } + self.retry_calls: list[dict[str, object]] = [] + + def retry(self, session_id: str, **kwargs: object) -> StatusResult: + self.retry_calls.append({"session_id": session_id, **kwargs}) + return StatusResult.from_json({"session_id": session_id, "status": "done", "progress": {}}) + + +def _attached(folder: Path, client: FakeClient) -> FolderIngest: + return FolderIngest.attach( + "sess-1", + folder, + "bio/uploads", + data_format="csv", + intent="editable", + show_progress=False, + client=client, # type: ignore[arg-type] + ) + + +def test_retry_resumes_the_load_with_the_corrected_definition(tmp_path: Path) -> None: + client = FailedSessionClient(resumable=True) + job = _attached(_csv_folder(tmp_path), client) + + outcome = job.retry(table_definition=DEFINITION) + + assert outcome.resumed + assert client.retry_calls == [{"session_id": "sess-1", "definition": DEFINITION}] + assert client.begin_calls == [] # nothing uploaded again + + +def test_retry_without_a_definition_calls_retry_as_before(tmp_path: Path) -> None: + client = FailedSessionClient(resumable=True) + _attached(_csv_folder(tmp_path), client).retry() + assert client.retry_calls == [{"session_id": "sess-1"}] + + +def test_a_retry_that_uploads_again_sends_the_corrected_definition_on_begin( + tmp_path: Path, +) -> None: + client = FailedSessionClient(resumable=False) + job = _attached(_csv_folder(tmp_path), client) + + job.retry(table_definition=DEFINITION) + + assert client.retry_calls == [] + assert client.begin_calls[0]["definition"] == DEFINITION diff --git a/tests/dds_ingestion/test_diagnose.py b/tests/dds_ingestion/test_diagnose.py new file mode 100644 index 0000000..440aab0 --- /dev/null +++ b/tests/dds_ingestion/test_diagnose.py @@ -0,0 +1,200 @@ +"""PY-7: FolderIngest.diagnose(), through the real IngestClient over mocked HTTP.""" + +from __future__ import annotations + +from pathlib import Path + +import httpx +import respx +from eea_datalakehouse.dds_ingestion.client import IngestClient +from eea_datalakehouse.dds_ingestion.credentials import DremioCreds +from eea_datalakehouse.dds_ingestion.folder import Diagnosis, FolderIngest + +from .conftest import BASE_URL + +PLAN = { + "placement": "staged", + "table_path": "bio/uploads/t", + "storage_path": None, + "action": "create", + "definition_source": "parquet_footer", +} +DRY_RUN = { + "session_id": None, + "status": "dry_run", + "s3": {"bucket": "b", "key_prefix": "p/", "uploads": []}, + "plan": PLAN, +} + + +def _diagnose(folder: Path, creds: DremioCreds, **kwargs: object) -> Diagnosis: + with IngestClient(BASE_URL, creds) as client: + job = FolderIngest( + folder, + "bio/uploads", + data_format="parquet", + show_progress=False, + client=client, + **kwargs, # type: ignore[arg-type] + ) + return job.diagnose() + + +def _summary(result: Diagnosis) -> list[tuple[str, bool | None]]: + return [(step.name, step.ok) for step in result.steps] + + +@respx.mock +def test_everything_green(data_folder: Path, creds: DremioCreds) -> None: + respx.get(f"{BASE_URL}/health").mock(return_value=httpx.Response(200, json={"status": "ok"})) + respx.get(f"{BASE_URL}/api/v1/ingest/limits").mock( + return_value=httpx.Response(200, json={"max_bytes": 1000, "stage_max_bytes": 1000}) + ) + respx.post(f"{BASE_URL}/storage-selftest").mock( + return_value=httpx.Response(200, json={"storage_write_test": {"ok": True}}) + ) + respx.post(f"{BASE_URL}/api/v1/ingest/begin").mock( + return_value=httpx.Response(200, json=DRY_RUN) + ) + + result = _diagnose(data_folder, creds) + + assert result.ok + assert _summary(result) == [ + ("DDS reachable", True), + ("Credential accepted", True), + ("File sizes", True), + ("Storage write test", True), + ("Dry run of this folder", True), + ] + assert result.steps[4].detail == "Importing as a new editable table at bio/uploads/t." + + +@respx.mock +def test_an_unreachable_dds_skips_the_rest(data_folder: Path, creds: DremioCreds) -> None: + respx.get(f"{BASE_URL}/health").mock(side_effect=httpx.ConnectError("refused")) + result = _diagnose(data_folder, creds) + assert not result.ok + assert _summary(result) == [ + ("DDS reachable", False), + ("Credential accepted", None), + ("File sizes", None), + ("Storage write test", None), + ("Dry run of this folder", None), + ] + + +@respx.mock +def test_a_rejected_credential(data_folder: Path, creds: DremioCreds) -> None: + respx.get(f"{BASE_URL}/health").mock(return_value=httpx.Response(200)) + respx.get(f"{BASE_URL}/api/v1/ingest/limits").mock( + return_value=httpx.Response(401, json={"error": "unauthorized", "message": "bad PAT"}) + ) + respx.post(f"{BASE_URL}/storage-selftest").mock( + return_value=httpx.Response(200, json={"storage_write_test": {"ok": True}}) + ) + respx.post(f"{BASE_URL}/api/v1/ingest/begin").mock( + return_value=httpx.Response(401, json={"error": "unauthorized", "message": "bad PAT"}) + ) + result = _diagnose(data_folder, creds) + assert dict(_summary(result))["Credential accepted"] is False + assert result.steps[1].detail == "bad PAT" + + +@respx.mock +def test_an_older_server(data_folder: Path, creds: DremioCreds) -> None: + respx.get(f"{BASE_URL}/health").mock(return_value=httpx.Response(200)) + respx.get(f"{BASE_URL}/api/v1/ingest/limits").mock(return_value=httpx.Response(404)) + respx.get(f"{BASE_URL}/api/v1/ingest").mock(return_value=httpx.Response(200, json=[])) + respx.post(f"{BASE_URL}/storage-selftest").mock(return_value=httpx.Response(404)) + respx.post(f"{BASE_URL}/api/v1/ingest/begin").mock( + return_value=httpx.Response( + 200, + json={ + "session_id": "s1", + "status": "uploading", + "s3": {"bucket": "b", "key_prefix": "p/"}, + }, + ) + ) + cancel = respx.delete(f"{BASE_URL}/api/v1/ingest/s1").mock(return_value=httpx.Response(204)) + + result = _diagnose(data_folder, creds) + + assert result.ok # nothing failed; two checks could not run + assert _summary(result) == [ + ("DDS reachable", True), + ("Credential accepted", True), + ("File sizes", None), + ("Storage write test", None), + ("Dry run of this folder", None), + ] + assert cancel.called # the session the old server opened was not left behind + + +@respx.mock +def test_no_write_privilege_on_the_target(data_folder: Path, creds: DremioCreds) -> None: + respx.get(f"{BASE_URL}/health").mock(return_value=httpx.Response(200)) + respx.get(f"{BASE_URL}/api/v1/ingest/limits").mock(return_value=httpx.Response(200, json={})) + respx.post(f"{BASE_URL}/storage-selftest").mock( + return_value=httpx.Response( + 200, + json={"storage_write_test": {"ok": False, "failures": ["catalog/upload: HTTP 501"]}}, + ) + ) + respx.post(f"{BASE_URL}/api/v1/ingest/begin").mock( + return_value=httpx.Response( + 403, json={"error": "forbidden", "message": "no WRITE privilege on bio/uploads"} + ) + ) + result = _diagnose(data_folder, creds) + assert not result.ok + steps = {step.name: step for step in result.steps} + assert steps["Storage write test"].ok is False + assert "HTTP 501" in steps["Storage write test"].detail + assert steps["Dry run of this folder"].detail == "no WRITE privilege on bio/uploads" + + +# --- PY-10: the files against DDS's size limits ------------------------------------- + + +def _mock_a_working_dds(limits: dict[str, object]) -> None: + respx.get(f"{BASE_URL}/health").mock(return_value=httpx.Response(200)) + respx.get(f"{BASE_URL}/api/v1/ingest/limits").mock( + return_value=httpx.Response(200, json=limits) + ) + respx.post(f"{BASE_URL}/storage-selftest").mock( + return_value=httpx.Response(200, json={"storage_write_test": {"ok": True}}) + ) + respx.post(f"{BASE_URL}/api/v1/ingest/begin").mock( + return_value=httpx.Response(200, json=DRY_RUN) + ) + + +def _sizes(result: Diagnosis) -> tuple[bool | None, str]: + step = next(step for step in result.steps if step.name == "File sizes") + return step.ok, step.detail + + +@respx.mock +def test_a_file_larger_than_dds_accepts(data_folder: Path, creds: DremioCreds) -> None: + # data_folder: a.parquet is 6 bytes, sub/b.parquet 7. + _mock_a_working_dds({"max_bytes": 6, "stage_max_bytes": 6}) + ok, detail = _sizes(_diagnose(data_folder, creds)) + assert ok is False + assert detail == "larger than DDS accepts (6 bytes): sub/b.parquet (7 bytes)" + + +@respx.mock +def test_a_file_over_the_stage_cap_matters_only_through_dds( + data_folder: Path, creds: DremioCreds +) -> None: + _mock_a_working_dds({"max_bytes": 100, "stage_max_bytes": 6}) + + ok, detail = _sizes(_diagnose(data_folder, creds)) + assert ok is True + assert "cannot fall back to it: sub/b.parquet (7 bytes)" in detail + + ok, detail = _sizes(_diagnose(data_folder, creds, upload_mode="proxy")) + assert ok is False + assert "upload_mode='proxy' sends every file: sub/b.parquet (7 bytes)" in detail diff --git a/tests/dds_ingestion/test_failure_reporting.py b/tests/dds_ingestion/test_failure_reporting.py index 6ec2f9e..8bd82cf 100644 --- a/tests/dds_ingestion/test_failure_reporting.py +++ b/tests/dds_ingestion/test_failure_reporting.py @@ -112,6 +112,23 @@ def test_a_failed_commit_is_described_like_the_transfers_panel(data_folder: Path assert client.cancelled == [] # kept for retry by default +def test_a_failed_commit_carries_its_stage(data_folder: Path) -> None: + # PY-13: the stage is an attribute too, not only part of the note. + with pytest.raises(IngestApiError) as exc: + _job(data_folder, FailingCommitClient(retryable=True)).run() + assert exc.value.failed_stage == "load" + + +@respx.mock +def test_an_api_error_has_no_stage_of_its_own(creds: DremioCreds) -> None: + respx.post(f"{BASE_URL}/api/v1/ingest/commit").mock( + return_value=httpx.Response(400, json={"error": "bad_request", "message": "no"}) + ) + with IngestClient(BASE_URL, creds) as client, pytest.raises(IngestApiError) as exc: + client.commit(session_id="s") + assert exc.value.failed_stage is None + + @pytest.mark.parametrize( ("retryable", "cleanup", "cancelled"), [(True, True, True), (False, True, False), (True, False, False)], @@ -123,3 +140,57 @@ def test_cleanup_on_failure( with pytest.raises(IngestApiError): _job(data_folder, client, cleanup_on_failure=cleanup).run() assert (client.cancelled == ["sess-X"]) is cancelled + + +# --- a retry after cleanup_on_failure (PY-8) ----------------------------------------- + + +class FlakyCommitClient(FailingCommitClient): + """Fails its first commit (retryable), then commits.""" + + def __init__(self) -> None: + super().__init__(retryable=True) + self.commits = 0 + + def commit(self, *, session_id: str, **kwargs: object) -> CommitResult: + self.commits += 1 + if self.commits == 1: + return super().commit(session_id=session_id, **kwargs) + return FakeClient.commit(self, session_id=session_id, **kwargs) + + +def test_a_retry_after_a_cleanup_uploads_again(data_folder: Path) -> None: + client = FlakyCommitClient() + job = _job(data_folder, client, cleanup_on_failure=True) + with pytest.raises(IngestApiError): + job.run() + assert client.cancelled == ["sess-X"] + + outcome = job.retry() + + assert outcome.commit.status == "committed" + assert len(client.begin_calls) == 2 # the upload was gone with the session + + +def test_a_cleanup_whose_cancel_failed_is_resumed_on_the_server(data_folder: Path) -> None: + class CancelFailsClient(FailingCommitClient): + def __init__(self) -> None: + super().__init__(retryable=True) + self.retried: list[str] = [] + + def cancel(self, session_id: str) -> None: + raise RuntimeError("DDS unreachable") + + def retry(self, session_id: str) -> StatusResult: + self.retried.append(session_id) + return StatusResult.from_json({"session_id": session_id, "status": "done"}) + + client = CancelFailsClient() + job = _job(data_folder, client, cleanup_on_failure=True) + with pytest.raises(IngestApiError): + job.run() + + outcome = job.retry() + + assert outcome.resumed and client.retried == ["sess-X"] + assert len(client.begin_calls) == 1 diff --git a/tests/dds_ingestion/test_folder_formats.py b/tests/dds_ingestion/test_folder_formats.py index e2a654b..6941fb5 100644 --- a/tests/dds_ingestion/test_folder_formats.py +++ b/tests/dds_ingestion/test_folder_formats.py @@ -167,3 +167,41 @@ def test_csv_options_reach_begin_only_for_csv(creds: DremioCreds) -> None: sent = [json.loads(call.request.read()) for call in route.calls] assert sent[0]["csv_options"] == {"delimiter": ";"} assert "csv_options" not in sent[1] + + +# --- the CSV encoding: DDS passes none on, and Dremio reads CSV as UTF-8 ---------- + +LATIN_1_CSV = "navn;by\nJørgen;Århus\n".encode("latin-1") + + +def test_a_non_utf8_encoding_option_is_refused_before_anything_starts(tmp_path: Path) -> None: + (tmp_path / "a.csv").write_bytes(LATIN_1_CSV) + client = FakeClient() + job = _job( + tmp_path, "csv", client, intent="editable", + csv_options={"delimiter": ";", "encoding": "latin-1"}, + ) + with pytest.raises(IngestApiError, match="DDS ignores it") as exc: + job.run() + assert exc.value.retryable is False + assert client.begin_calls == [] + + +@pytest.mark.parametrize("spelling", ["utf8", "UTF-8", "utf_8"]) +def test_utf8_however_spelled_is_fine(tmp_path: Path, spelling: str) -> None: + (tmp_path / "a.csv").write_text("navn;by\nJørgen;Århus\n", encoding="utf-8") + client = FakeClient() + options = {"delimiter": ";", "encoding": spelling} + _job(tmp_path, "csv", client, intent="editable", csv_options=options).run() + assert _sent_columns(client) == [("navn", "VARCHAR"), ("by", "VARCHAR")] + assert client.begin_calls[0]["csv_options"] == options # still sent as given + + +def test_a_csv_that_is_not_utf8_is_named_before_anything_starts(tmp_path: Path) -> None: + (tmp_path / "a.csv").write_text("navn;by\nAnna;Oslo\n", encoding="utf-8") + (tmp_path / "b.csv").write_bytes(LATIN_1_CSV) + client = FakeClient() + job = _job(tmp_path, "csv", client, intent="editable", csv_options={"delimiter": ";"}) + with pytest.raises(IngestApiError, match="'b.csv' is not valid UTF-8"): + job.run() + assert client.begin_calls == [] diff --git a/tests/dds_ingestion/test_folder_upload_modes.py b/tests/dds_ingestion/test_folder_upload_modes.py index 077aeeb..a9baec0 100644 --- a/tests/dds_ingestion/test_folder_upload_modes.py +++ b/tests/dds_ingestion/test_folder_upload_modes.py @@ -1,4 +1,5 @@ -"""PY-1 (upload_mode + /stage fallback) and PY-2 (cancel when an upload fails). +"""PY-1 (upload_mode + /stage fallback), PY-2 (cancel when an upload fails), +PY-8 (retry after that cancel) and PY-10 (the /stage size cap). Companion to the DDS DI-12 plan; see docs/dds-ingest-robustness-client-plan.md. """ @@ -10,13 +11,15 @@ import httpx import pytest -from eea_datalakehouse.dds_ingestion.client import S3UploadError +from eea_datalakehouse.dds_ingestion.client import IngestApiError, S3UploadError +from eea_datalakehouse.dds_ingestion.folder import IngestStateError from eea_datalakehouse.dds_ingestion.models import ( BeginResult, FileSpec, StageResult, UploadTarget, ) +from eea_datalakehouse.dds_ingestion.session import IngestCommitError, IngestSession from .test_folder import FakeClient, _make_ingest @@ -88,6 +91,7 @@ def test_proxy_mode_sends_every_file_through_dds(data_folder: Path) -> None: assert sorted(client.staged) == ["a.parquet", "sub/b.parquet"] assert client.attempted == [] assert outcome.commit.status == "committed" + assert (outcome.upload_mode, outcome.files_staged) == ("proxy", 2) def test_auto_mode_falls_back_to_dds_on_a_network_error(data_folder: Path) -> None: @@ -98,6 +102,13 @@ def test_auto_mode_falls_back_to_dds_on_a_network_error(data_folder: Path) -> No assert "sub/b.parquet" in client.uploaded assert outcome.commit.status == "committed" assert client.cancelled == [] + # The fallback shows: one file of two went through DDS (PY-13). + assert (outcome.upload_mode, outcome.files_uploaded, outcome.files_staged) == ("direct", 2, 1) + + +def test_a_plain_direct_upload_stages_nothing(data_folder: Path) -> None: + outcome = _make_ingest(data_folder, ModeClient(upload_mode="auto")).run() + assert outcome.files_staged == 0 def test_auto_mode_does_not_retry_an_s3_http_error_through_dds(data_folder: Path) -> None: @@ -163,3 +174,148 @@ def test_a_successful_ingest_is_never_cancelled(data_folder: Path) -> None: client = ModeClient() _make_ingest(data_folder, client).run() assert client.cancelled == [] + + +# --- PY-8: retry after a cancelled upload ---------------------------------------- + + +class FlakyClient(ModeClient): + """ModeClient whose failures happen once: the next attempt at that file works.""" + + def upload_file(self, target: UploadTarget, data: bytes) -> str | None: + error = self.fail.pop(target.rel_path, None) + if error is not None: + self.attempted.append(target.rel_path) + raise error + return super().upload_file(target, data) + + +def test_retry_uploads_again_after_a_network_error(data_folder: Path) -> None: + client = FlakyClient(fail={"a.parquet": _network_error()}) + job = _make_ingest(data_folder, client) + with pytest.raises(httpx.ConnectError): + job.run() + + outcome = job.retry() + + assert outcome.commit.status == "committed" + assert len(client.begin_calls) == 2 # a new session + assert client.cancelled == ["sess-X"] # only the first one + + +def test_retry_does_not_upload_again_what_would_fail_the_same_way(data_folder: Path) -> None: + client = FlakyClient(fail={"a.parquet": _s3_error("a.parquet")}) + job = _make_ingest(data_folder, client) + with pytest.raises(S3UploadError): + job.run() + + with pytest.raises(IngestStateError, match="HTTP 403") as exc: + job.retry() + + assert isinstance(exc.value.__cause__, S3UploadError) + assert len(client.begin_calls) == 1 + + +def test_retry_does_not_upload_again_when_the_cancel_failed(data_folder: Path) -> None: + client = FlakyClient( + fail={"a.parquet": _network_error()}, cancel_error=RuntimeError("DDS unreachable") + ) + job = _make_ingest(data_folder, client) + with pytest.raises(httpx.ConnectError): + job.run() + + with pytest.raises(IngestStateError, match="could not be cancelled"): + job.retry() + + assert len(client.begin_calls) == 1 + + +def test_retry_before_any_run_still_says_there_is_no_session(data_folder: Path) -> None: + with pytest.raises(IngestStateError, match="no session to retry"): + _make_ingest(data_folder, ModeClient()).retry() + + +def test_ingest_session_retry_shows_the_upload_error(data_folder: Path) -> None: + client = FlakyClient(fail={"a.parquet": _s3_error("a.parquet")}) + session = IngestSession(client=client) # type: ignore[arg-type] + session.ingest(data_folder, "bio.uploads", data_format="parquet", show_progress=False) + + with pytest.raises(IngestCommitError) as exc: + session.commit(retry=True) + + assert "AccessDenied" in str(exc.value) + assert "no session to retry" not in str(exc.value) + + +def test_ingest_session_retry_recovers_from_a_network_error(data_folder: Path) -> None: + client = FlakyClient(fail={"a.parquet": _network_error()}) + session = IngestSession(client=client) # type: ignore[arg-type] + session.ingest(data_folder, "bio.uploads", data_format="parquet", show_progress=False) + + report = session.commit(retry=True) + + assert [o.commit.status for o in report.outcomes] == ["committed"] + + +# --- PY-10: the /stage size cap --------------------------------------------------- + + +class CappedClient(FlakyClient): + """FlakyClient that answers GET /ingest/limits; ``stage_max_bytes=None`` is a + server before DI-12.11 (404).""" + + def __init__(self, *, stage_max_bytes: int | None, **kwargs: object) -> None: + super().__init__(**kwargs) # type: ignore[arg-type] + self.stage_max_bytes = stage_max_bytes + self.limits_calls = 0 + + def limits(self, runner: str = "notebook") -> dict[str, object]: + self.limits_calls += 1 + if self.stage_max_bytes is None: + raise IngestApiError(404, "Not Found", where="GET /api/v1/ingest/limits") + return {"max_bytes": 1000, "stage_max_bytes": self.stage_max_bytes} + + +def test_proxy_refuses_a_file_over_the_stage_cap_before_uploading_anything( + data_folder: Path, +) -> None: + # data_folder: a.parquet is 6 bytes, sub/b.parquet 7. + client = CappedClient(upload_mode="proxy", stage_max_bytes=6) + with pytest.raises(IngestApiError, match=r"too large: 'sub/b.parquet' \(7 bytes\)") as exc: + _make_ingest(data_folder, client).run() + + assert client.staged == [] + assert client.cancelled == ["sess-X"] + assert exc.value.retryable is False + + +def test_proxy_against_a_server_without_limits_uploads_as_before(data_folder: Path) -> None: + client = CappedClient(upload_mode="proxy", stage_max_bytes=None) + _make_ingest(data_folder, client).run() + assert sorted(client.staged) == ["a.parquet", "sub/b.parquet"] + + +def test_no_fallback_for_a_file_over_the_stage_cap(data_folder: Path) -> None: + client = CappedClient( + upload_mode="auto", stage_max_bytes=5, fail={"a.parquet": _network_error()} + ) + with pytest.raises(httpx.ConnectError) as exc: + _make_ingest(data_folder, client).run() + + assert client.staged == [] + assert any("Not sent through DDS instead" in note for note in exc.value.__notes__) + + +def test_the_limits_are_asked_only_when_a_file_may_go_through_dds(data_folder: Path) -> None: + plain = CappedClient(upload_mode="auto", stage_max_bytes=100) + _make_ingest(data_folder, plain).run() + assert plain.limits_calls == 0 # a direct upload sends what it sent before + + two_fallbacks = CappedClient( + upload_mode="auto", + stage_max_bytes=100, + fail={"a.parquet": _network_error(), "sub/b.parquet": _network_error()}, + ) + outcome = _make_ingest(data_folder, two_fallbacks).run() + assert outcome.files_staged == 2 + assert two_fallbacks.limits_calls == 1 diff --git a/tests/dds_ingestion/test_ingest_session.py b/tests/dds_ingestion/test_ingest_session.py index 1995248..0f6dfb5 100644 --- a/tests/dds_ingestion/test_ingest_session.py +++ b/tests/dds_ingestion/test_ingest_session.py @@ -11,10 +11,13 @@ import pytest +from eea_datalakehouse.dds_ingestion.client import IngestApiError +from eea_datalakehouse.dds_ingestion.folder import Diagnosis, FolderIngest from eea_datalakehouse.dds_ingestion.models import ( BeginResult, CommitResult, S3Plan, + StageResult, StatusResult, UploadTarget, ) @@ -166,3 +169,122 @@ def retry(self, session_id: str) -> StatusResult: assert len(report.outcomes) == 1 assert report.outcomes[0].resumed is True assert client.retry_calls == ["sess-1"] + + +def test_the_one_line_options_reach_folder_ingest(tmp_path: Path) -> None: + # PY-11: upload_mode, csv_options, json_options and table_definition, each + # given on the one line that queues the ingest. + class RecordingClient(_StubClient): + def __init__(self) -> None: + super().__init__() + self.begin_kwargs: list[dict[str, Any]] = [] + self.staged: list[str] = [] + + def begin(self, *, files: list[Any], **kwargs: Any) -> BeginResult: + self.begin_kwargs.append(kwargs) + return super().begin(files=files, **kwargs) + + def stage(self, session_id: str, rel_path: str, data: bytes) -> StageResult: + self.staged.append(rel_path) + return StageResult(rel_path=rel_path, key=f"p/{rel_path}", bytes_written=len(data)) + + csv_folder = tmp_path / "csv" + csv_folder.mkdir() + (csv_folder / "a.csv").write_text("a;b\n1;2\n") + json_folder = tmp_path / "json" + json_folder.mkdir() + (json_folder / "a.json").write_text('[{"a": 1}]') + columns = {"columns": [{"name": "a", "type": "BIGINT"}]} + + client = RecordingClient() + session = IngestSession(client=client) # type: ignore[arg-type] + session.ingest( + csv_folder, "bio.uploads.c", data_format="csv", intent="editable", show_progress=False, + csv_options={"delimiter": ";"}, table_definition=columns, upload_mode="proxy", + ) + session.ingest( + json_folder, "bio.uploads.j", data_format="json", show_progress=False, + json_options={"format": "array"}, + ) + session.commit() + + csv_begin, json_begin = client.begin_kwargs + assert csv_begin["csv_options"] == {"delimiter": ";"} + assert csv_begin["definition"] == columns + assert client.staged == ["a.csv"] # upload_mode="proxy": through DDS, not S3 + assert json_begin["json_options"] == {"format": "array"} + + +def test_diagnose_checks_every_queued_ingest_and_keeps_the_queue( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + checked: list[tuple[Path, bool]] = [] + + def fake_diagnose(self: FolderIngest) -> Diagnosis: + checked.append((self.folder, self.show_progress)) + return Diagnosis() + + monkeypatch.setattr(FolderIngest, "diagnose", fake_diagnose) + client = _StubClient() + session = IngestSession(client=client) # type: ignore[arg-type] + a, b = _make_folder(tmp_path, "a"), _make_folder(tmp_path, "b") + session.ingest(a, "bio.uploads.a", data_format="parquet", show_progress=False) + session.ingest(b, "bio.uploads.b", data_format="parquet", show_progress=False) + + session.diagnose() + + assert checked == [(a, True), (b, True)] # the lines print even for a quiet ingest + assert client.begin_calls == 0 + assert repr(session) == "IngestSession(pending=2)" + assert f"ingest {a!r} -> 'bio.uploads.a':" in capsys.readouterr().out + + +def test_diagnose_reports_a_missing_folder_and_goes_on( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.setattr(FolderIngest, "diagnose", lambda self: Diagnosis()) + session = IngestSession(client=_StubClient()) # type: ignore[arg-type] + session.ingest(tmp_path / "gone", "bio.uploads.x", data_format="parquet") + session.ingest(_make_folder(tmp_path, "b"), "bio.uploads.b", data_format="parquet") + + session.diagnose() + + out = capsys.readouterr().out + assert "✗ cannot check it: not a directory" in out + assert "'bio.uploads.b':" in out + + +def test_diagnose_with_nothing_queued_says_so(capsys: pytest.CaptureFixture[str]) -> None: + IngestSession(client=_StubClient()).diagnose() # type: ignore[arg-type] + assert "nothing queued" in capsys.readouterr().out + + +def test_cleanup_on_failure_with_retry_uploads_again(tmp_path: Path) -> None: + class FailsOnceClient(_StubClient): + def __init__(self) -> None: + super().__init__() + self.cancelled: list[str] = [] + + def commit(self, *, session_id: str, **kwargs: Any) -> CommitResult: + if not self.commit_calls: + self.commit_calls.append(session_id) + err = IngestApiError(503, "Dremio is busy", where="POST commit") + err.retryable = True + raise err + return super().commit(session_id=session_id, **kwargs) + + def cancel(self, session_id: str) -> None: + self.cancelled.append(session_id) + + client = FailsOnceClient() + session = IngestSession(client=client) # type: ignore[arg-type] + session.ingest( + _make_folder(tmp_path, "a"), "bio.uploads", data_format="parquet", + show_progress=False, cleanup_on_failure=True, + ) + + report = session.commit(retry=True) + + assert [o.commit.status for o in report.outcomes] == ["done"] + assert client.cancelled == ["sess-1"] # the failed commit left nothing behind + assert client.begin_calls == 2 # and the retry uploaded again diff --git a/tests/notebook/test_magics.py b/tests/notebook/test_magics.py index 89d7fe5..45d6111 100644 --- a/tests/notebook/test_magics.py +++ b/tests/notebook/test_magics.py @@ -272,6 +272,11 @@ def test_ingest_magic_queues_onto_one_shared_session(ip: Any) -> None: assert repr(session) == "IngestSession(pending=1)" +def test_ingest_diagnose_with_nothing_queued(ip: Any, capsys: pytest.CaptureFixture[str]) -> None: + ip.run_line_magic("ingest", "diagnose") # bare name, like commit + assert "nothing queued" in capsys.readouterr().out + + def test_empty_line_prints_usage(ip: Any, capsys: pytest.CaptureFixture[str]) -> None: # No credentials needed to reach the usage message — it's printed before # a session is ever built. @@ -398,8 +403,12 @@ def test_ingest_help_lists_methods( table_html = displayed[0].data assert "Command" in table_html and "Parameters" in table_html and "Description" in table_html assert ">ingest<" in table_html and ">commit<" in table_html + assert ">diagnose<" in table_html assert "folder, target_catalog_path" in table_html assert "retry=False, max_retries=3" in table_html + # PY-11: the one-line options show, with their defaults. + assert "csv_options=None" in table_html and "upload_mode='auto'" in table_html + assert "cleanup_on_failure=False" in table_html assert "str | None" not in table_html # no Python type-hint syntax leaking into the table assert _magics_instance(ip)._ingest_session is None