From 1038f551a7d38f7f9158ddaa6ced56b66f714455 Mon Sep 17 00:00:00 2001 From: oskaresparza Date: Thu, 17 Sep 2026 14:22:05 +0200 Subject: [PATCH 1/2] Make %catalog execute immediately and round out its verb set `%catalog` calls now commit themselves the moment a cell runs instead of queueing behind a separate commit step; `CatalogSession` still does the committing underneath (retry/rollback unchanged), it's just no longer something a custodian has to remember to call. Adds `use(path)`/`%%catalog use(path)` to set relative-path context explicitly (validated against the catalog, `../` support for walking up parent paths anywhere a relative path is accepted), and rounds out the verb set with `get_wiki`, `get_tags`, `list`, `schema`, `delete_view`, `delete_table` (all erroring on a missing path). Renames `copy`/`move` to `data_copy`/`data_move`, `tag`/`untag` to `set_tags`/`delete_tags`, and drops `set_meta` in favor of `set_wiki`. `%catalog help`/`%ingest help` render as an HTML table grouped by verb category (data, table, view, wiki, tags) rather than a plain list. Co-Authored-By: Claude Sonnet 5 --- README.md | 65 +++- debugger/test_catalog_magic.ipynb | 308 +++++++++++++++ docs/notebook-facade-for-data-scientists.md | 94 +++-- docs/notebooks/catalog_session_example.ipynb | 147 ++++---- src/eea_datalakehouse/catalog/client.py | 23 ++ src/eea_datalakehouse/catalog/operations.py | 129 +++++++ src/eea_datalakehouse/catalog/session.py | 372 +++++++++++++++---- src/eea_datalakehouse/notebook/magics.py | 367 +++++++++++++++--- tests/catalog/conftest.py | 4 + tests/catalog/test_client.py | 62 ++++ tests/catalog/test_operations.py | 115 ++++++ tests/catalog/test_session.py | 68 +++- tests/catalog/test_session_context.py | 322 +++++++++++++++- tests/catalog/test_session_queries.py | 257 +++++++++++++ tests/notebook/test_magics.py | 172 ++++++++- 15 files changed, 2226 insertions(+), 279 deletions(-) create mode 100644 debugger/test_catalog_magic.ipynb create mode 100644 tests/catalog/test_session_queries.py diff --git a/README.md b/README.md index e7d178e..40daa3c 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,11 @@ exact same arguments. `CREATE VIEW` transition for a location that started as a physical table (straight off ingest) and is moving to being a view. Checks `source_path` exists first; by default also creates `view_path`'s containing folder if missing. +- `createview(source_path, target_path, overwrite=False, create_target_folder=False)` — + `CREATE VIEW ... AS SELECT * FROM source_path`, leaving `source_path` untouched (unlike + `datamove`). No data of its own — a view is a saved query — so this runs over REST like + every other metadata-only operation, not Arrow Flight. Same folder-append and `overwrite` + behavior as `datacopy`/`datamove`. - `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. @@ -171,9 +176,11 @@ exact same arguments. ## Notebook facade (`%catalog` / `%ingest`) -For interactive use in JupyterLab, `eea_datalakehouse.notebook` registers two line magics — -thin, queue-then-commit wrappers over `CatalogSession`/`IngestSession` aimed at data -custodians rather than application developers. See +For interactive use in JupyterLab, `eea_datalakehouse.notebook` registers two line magics +aimed at data custodians rather than application developers, thin wrappers over +`CatalogSession`/`IngestSession`. `%catalog` runs each call **immediately** — no commit +step to remember. `%ingest` still queues and needs an explicit `commit()`, since a catalog +operation can't run before its target has actually been ingested. See `docs/notebook-facade-for-data-scientists.md` for the full design, and `docs/notebooks/catalog_session_example.ipynb` / `ingest_session_example.ipynb` for worked examples. Install the extra this needs once: `pip install "EEADataLakehouse[notebook]"`. @@ -181,18 +188,58 @@ examples. Install the extra this needs once: `pip install "EEADataLakehouse[note ```python import eea_datalakehouse.notebook # registers %catalog/%ingest — no %load_ext needed -%catalog copy("draft.raw_2026", "bwd.reference.water_temperature") -%catalog tag(".water_temperature", ["reviewed"]) -%catalog commit(retry=True) +%catalog data_copy("draft.raw_2026", "bwd.reference.water_temperature") +%catalog set_tags(".water_temperature", ["reviewed"]) %ingest ingest(folder="./bw_2026", target_catalog_path="bwd.reference", data_format="parquet", table_name="water_temperature") %ingest commit(retry=True) ``` -`%catalog help` (or `%catalog help()`) lists every `CatalogSession` method with its -signature and a short description; `%ingest help` does the same for `IngestSession` — handy -when you don't remember an exact parameter name mid-notebook. +`%catalog help` (or `%catalog help()`) prints every command as a plain table — name, +parameters, description — rather than a raw Python signature, since its audience is a data +custodian, not necessarily a developer. `%ingest help` does the same for `IngestSession`, +listing its (fewer) methods with their real signatures — handy when you don't remember an +exact parameter name mid-notebook. + +`%%catalog` (the cell-magic form) sets the context once, with `use(path)` on its magic line, +then runs every other line of the cell in order under that context, without repeating the full +path on each line. A leading `.` resolves any `path`/`source_path`/`target_path` against the +context (so `data_copy`/`set_tags`/`create_folder`/... all understand it); `use` alone also accepts a bare +path with no dot, once a context exists — `"2027"` and `".2027"` narrow it the same way there. +One or more leading `../` (or a bare `..`) instead walks up that many levels of the context +first, everywhere a relative path is understood, not just in `use` — `"../water_temperature"` is +a sibling of the context, `"../../water_temperature"` a level further up. `use` also makes a live +check that the resolved path actually exists in the catalog, raising if it doesn't, rather than +silently pointing context somewhere later calls would fail against anyway; `use(None)` clears the +context. `get_context()` shows what it currently is — always the full resolved path: + +```python +%%catalog use("bwd.reference") +set_tags(".water_temperature", ["reviewed"]) +create_folder(".2027") # create_folder still needs the dot — only use() makes it optional + +%catalog use("bwd.reference") # back to a path that already exists +%catalog use("2027") # bare, no dot — same as use(".2027"); already created above +%catalog get_context() # -> 'bwd.reference.2027' +%catalog set_tags("../water_temperature", ["archived"]) # ../ works for any verb, not just use() +``` + +`get_wiki(path)`, `get_tags(path)`, `list(path)` (every table/view under `path`, at any depth) +and `schema(path)` (column types + row count) answer immediately, like `get_context()` — nothing +to commit or undo. `delete_view(path)`/`delete_table(path)` queue like every other write, but — +unlike the idempotent `DROP ... IF EXISTS` `Catalog.deleteview`/`deletetable` wrap — require +`path` to already exist, and (like `delete_folder`) can never be undone. Every one of these +raises `CatalogOperationError` if `path` doesn't exist; `schema` also requires it to be a table +or view, not a folder: + +```python +%catalog get_wiki("bwd.reference.water_temperature") +%catalog get_tags("bwd.reference.water_temperature") +%catalog list("bwd.reference") # -> ['bwd.reference.water_temperature', ...] +%catalog schema("bwd.reference.water_temperature") # -> TableInfo(schema={...}, row_count=...) +%catalog delete_view("bwd.reference.old_view") +``` ## Layout diff --git a/debugger/test_catalog_magic.ipynb b/debugger/test_catalog_magic.ipynb new file mode 100644 index 0000000..e7a0b98 --- /dev/null +++ b/debugger/test_catalog_magic.ipynb @@ -0,0 +1,308 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# `%catalog` smoke test — immediate execution, no commit\n", + "\n", + "A functional test notebook for the `%catalog` magic (`src/eea_datalakehouse/notebook/magics.py`),\n", + "meant to be run against this project's own debug/local Dremio stack (`debugger/.env` — see\n", + "`debugger/debug_run.py`, whose `run_createfolder`/`run_setmeta2wiki`/... functions exercise the\n", + "same underlying `Catalog` calls the hard way). Unlike `docs/notebooks/catalog_session_example.ipynb`\n", + "(a documentation walkthrough), this one exists to actually **prove** the behaviour: every\n", + "`%catalog` call below is followed by an independent check, through a fresh `Catalog` client the\n", + "magic never touched, that the change already landed — no separate `%catalog commit` anywhere in\n", + "this notebook.\n", + "\n", + "**This writes to the catalog for real** (a scratch folder, cleaned up at the end — see\n", + "\"Cleanup\"). Run from this project's own dev environment (`pip install -e \".[dev]\"`), which already\n", + "includes the `notebook` extra; `debugger/.env` must be filled in (copy from `.env.example`) and\n", + "the stack it points at reachable." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import os\n", + "from pathlib import Path\n", + "\n", + "\n", + "def _find_debugger_env(start: Path | None = None) -> Path:\n", + " \"\"\"`debugger/.env` — located by walking up from this notebook rather than\n", + " hardcoded, so it works wherever Jupyter's cwd happens to be (same\n", + " reasoning as `03_ingest.ipynb`'s `_repo_root()`). Identified by sitting\n", + " next to `debug_run.py`, not just any `.env`.\"\"\"\n", + " here = start or Path.cwd()\n", + " for candidate in (here, *here.parents):\n", + " if (candidate / \"debug_run.py\").exists() and (candidate / \".env\").exists():\n", + " return candidate / \".env\"\n", + " raise RuntimeError(\n", + " f\"could not find debugger/.env from {here} — copy debugger/.env.example to \"\n", + " \"debugger/.env, fill it in, and open this notebook from inside the repository\"\n", + " )\n", + "\n", + "\n", + "ENV_PATH = _find_debugger_env()\n", + "for _line in ENV_PATH.read_text().splitlines():\n", + " _line = _line.strip()\n", + " if not _line or _line.startswith(\"#\") or \"=\" not in _line:\n", + " continue\n", + " _key, _, _value = _line.partition(\"=\")\n", + " os.environ.setdefault(_key.strip(), _value.strip())\n", + "\n", + "# %catalog reads DREMIO_USERNAME (see notebook/magics.py's _build_catalog_session);\n", + "# debugger/.env uses the shorter DREMIO_USER — same bridging debug_run.py itself\n", + "# does for the DDS side (_DREMIO_USER/_DREMIO_PWD).\n", + "os.environ.setdefault(\"DREMIO_USERNAME\", os.environ.get(\"DREMIO_USER\", \"\"))\n", + "\n", + "print(f\"env file {ENV_PATH}\")\n", + "print(f\"DREMIO_BASE_URL {os.environ.get('DREMIO_BASE_URL') or ''}\")\n", + "print(f\"DREMIO_USERNAME {os.environ.get('DREMIO_USERNAME') or ''}\")\n", + "print(f\"DREMIO_TOKEN {'set' if os.environ.get('DREMIO_TOKEN') else ''}\")\n" + ] + }, + { + "cell_type": "code", + "execution_count": 1, + "id": "c5e8b5d6", + "metadata": {}, + "outputs": [], + "source": [ + "import eea_datalakehouse.notebook # registers %catalog/%ingest — no %load_ext needed" + ] + }, + { + "cell_type": "markdown", + "id": "415930da", + "metadata": {}, + "source": [ + "## Help\n", + "\n", + "Sanity check before anything else: `%catalog help()` should print the command table without\n", + "needing any of the environment above (it's answered before a session is built)." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "c311fdb0", + "metadata": {}, + "outputs": [], + "source": "%catalog help()" + }, + { + "cell_type": "markdown", + "id": "a6f909ab", + "metadata": {}, + "source": [ + "## Scratch space\n", + "\n", + "Everything below happens under a folder distinctly named for this notebook, alongside the\n", + "`altia_test` scratch folder `debug_run.py` already uses for the same kind of manual testing —\n", + "so it's obviously not real data, and `delete_folder(cascade=True)` in \"Cleanup\" removes all of\n", + "it at the end." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "25a4af1d", + "metadata": {}, + "outputs": [], + "source": [ + "TEST_ROOT = \"catalog.water_management_resources.bathing_water.bwd.draft\"\n", + "TEST_FOLDER = f\"{TEST_ROOT}.catalog_magic_smoke_test\"\n", + "print(TEST_FOLDER)" + ] + }, + { + "cell_type": "markdown", + "id": "1e79e191", + "metadata": {}, + "source": [ + "## 1. A call runs immediately\n", + "\n", + "`create_folder` below is the only `%catalog` call in this cell — nothing else queues it, and\n", + "there's no `%catalog commit` after it." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "e563984c", + "metadata": {}, + "outputs": [], + "source": [ + "%catalog create_folder(TEST_FOLDER, create_parents=True)" + ] + }, + { + "cell_type": "markdown", + "id": "97f7f415", + "metadata": {}, + "source": [ + "**Proof:** the next cell builds a brand new `Catalog` client — one the magic's\n", + "`CatalogSession` never touched — and asks Dremio directly whether the folder is there. If\n", + "`%catalog` only queued the step, this would say `False`." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "0fca8ccf", + "metadata": {}, + "outputs": [], + "source": [ + "from eea_datalakehouse.catalog import Catalog\n", + "\n", + "verify_catalog = Catalog(\n", + " os.environ[\"DREMIO_BASE_URL\"], os.environ[\"DREMIO_TOKEN\"],\n", + " username=os.environ.get(\"DREMIO_USERNAME\"),\n", + ")\n", + "# `_catalog_rest.exists` is the same private check `CatalogSession.create_folder`'s own\n", + "# `run()` uses internally (see session.py) — reached into directly here only because this\n", + "# is a debug/verification notebook, same spirit as debug_run.py's `catalog._flight_executor`\n", + "# access elsewhere in this folder.\n", + "print(\"folder exists right now:\", verify_catalog._catalog_rest.exists(TEST_FOLDER)) # noqa" + ] + }, + { + "cell_type": "markdown", + "id": "68efceb9", + "metadata": {}, + "source": [ + "## 2. Relative paths still work\n", + "\n", + "`create_folder` sets the session's context to the folder it just created (not its parent —\n", + "see `CatalogSession.create_folder`'s docstring), so a following `.name` call lands *inside*\n", + "it without spelling out `TEST_FOLDER` again." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "3c41cfca", + "metadata": {}, + "outputs": [], + "source": [ + "%catalog create_folder(\".nested\") # -> TEST_FOLDER + \".nested\"" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "1cd340ad", + "metadata": {}, + "outputs": [], + "source": [ + "print(\"nested folder exists right now:\", verify_catalog._catalog_rest.exists(f\"{TEST_FOLDER}.nested\")) # noqa" + ] + }, + { + "cell_type": "markdown", + "id": "24a3cbc3", + "metadata": {}, + "source": "## 3. Wiki text — also immediate\n\n`set_wiki` works on any catalog entity, folders included. Commits itself the moment its\ncell runs, exactly like `create_folder` above." + }, + { + "cell_type": "code", + "execution_count": null, + "id": "9bfe34e1", + "metadata": {}, + "outputs": [], + "source": [ + "%catalog set_wiki(TEST_FOLDER, \"# Catalog magic smoke test\\n\\nCreated by the %catalog debugger notebook.\")" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "a83f08c6", + "metadata": {}, + "outputs": [], + "source": [ + "print(verify_catalog.getwikifrom(TEST_FOLDER, idempotency_key=\"smoke-test-read-wiki\"))" + ] + }, + { + "cell_type": "markdown", + "id": "59854882", + "metadata": {}, + "source": [ + "## 4. Read-only queries answer immediately too\n", + "\n", + "`get_wiki`/`get_tags`/`list`/`schema` are read-only `CatalogSession` methods — no `%catalog\n", + "commit` involved, same as every other call in this notebook. `get_wiki` reads back the wiki\n", + "just set above; `list` finds no tables/views here since `TEST_FOLDER` only holds folders —\n", + "an empty list, not an error (it would only raise if `TEST_FOLDER` itself didn't exist)." + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": "## 5. What a failed call looks like\n\n`TEST_FOLDER` still has `.nested` inside it, so deleting it without `cascade=True` should\nfail — `%catalog` prints a short message instead of a traceback (see the design doc's\n\"Exceptions translated at the boundary\")." + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "%catalog delete_folder(TEST_FOLDER) # cascade=False (the default) — expected to fail, folder not empty" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "## Cleanup\n", + "\n", + "`cascade=True` removes `.nested` along with `TEST_FOLDER` itself — this, like `%catalog`'s\n", + "other calls, has already happened by the time the cell below finishes." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "%catalog delete_folder(TEST_FOLDER, cascade=True)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "print(\"folder exists after cleanup:\", verify_catalog._catalog_rest.exists(TEST_FOLDER)) # noqa" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": ".venv (3.12.3.final.0)", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.12.3" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} \ No newline at end of file diff --git a/docs/notebook-facade-for-data-scientists.md b/docs/notebook-facade-for-data-scientists.md index 5748fae..489a219 100644 --- a/docs/notebook-facade-for-data-scientists.md +++ b/docs/notebook-facade-for-data-scientists.md @@ -15,6 +15,38 @@ catalog context" — see `docs/notebooks/catalog_session_example.ipynb` and a prototype behind an opt-in `notebook` extra, not reviewed or adopted — the rest of this file is the design reasoning behind it, kept as written. +**Update:** `%catalog` no longer exposes queue-then-commit at the magic +level — each call now commits itself immediately (`CatalogSession` still +does the committing underneath, one call at a time, so the retry/rollback +behaviour described below is unchanged; there's just no longer a separate +`%catalog commit` a custodian has to remember). `%ingest` is unaffected and +still queues/commits as described throughout this doc. Context has also +grown past what's described below: `use(path)` (also via `%%catalog +use(path)`, the cell-magic form) sets it deliberately — making a live +check that the resolved path exists in the catalog first, unlike +everything else here, and always storing the full resolved path, never a +raw `.`/`..`-prefixed fragment — `get_context()` reads it back, and a +leading `../` (or a bare `..`) on any relative path, not just inside +`use`, walks up that many levels of the context first. `copy`/`move` were +later renamed `datacopy`/`datamove`, matching `Catalog`'s own names instead +of inventing friendlier ones, then renamed again to `data_copy`/`data_move` +for consistency with the rest of the facade's underscored verbs (`Catalog`'s +own `datacopy`/`datamove` are unchanged — only the `CatalogSession`/ +`%catalog` wrapper got the underscore); six more verbs were added — read-only +`get_wiki`, `get_tags`, `list`, `schema` (answered immediately, like +`get_context`), and queued `delete_view`/`delete_table` (never reversible, +like `delete_folder`, but unlike the idempotent `Catalog.deleteview`/ +`deletetable` they wrap, require `path` to already exist — originally +named `deleteview`/`deletetable` to match, then renamed with an underscore +for consistency with `delete_folder`/`delete_wiki`). `tag`/`untag` were +similarly renamed `set_tags`/`delete_tags` (matching `set_wiki`/ +`delete_wiki`'s pattern), and `set_meta` was removed — the raw +`Catalog.setmeta2wiki`/`getmetafromwiki` are still there for a folder's +wiki Meta Data section, just not wrapped by `CatalogSession` any more. See +`src/eea_datalakehouse/notebook/magics.py`'s module docstring and +`src/eea_datalakehouse/catalog/session.py`'s `use`/`get_context`/ +`_resolve_path` for the current, authoritative behaviour. + ## The facade's surface, end to end A consolidated view of what all the sections below amount to — every other @@ -35,13 +67,19 @@ result, in one place. **What it exposes** — a small, curated verb set, not the full developer API: -| catalog | ingestion | +| catalog (runs immediately) | ingestion (queue, then commit) | | --- | --- | -| `copy`, `move` | `ingest` | -| `tag`, `untag` | | -| `set_wiki`, `delete_wiki`, `set_meta` | | +| `data_copy`, `data_move` (data) | `ingest` | +| `list`, `schema`, `delete_table` (table) | | +| `create_view`, `delete_view` (view) | | +| `set_wiki`, `delete_wiki`, `get_wiki` (wiki) | | +| `set_tags`, `delete_tags`, `get_tags` (tags) | | | `create_folder`, `delete_folder` | | -| `commit(retry=True)` | `commit(retry=True)` | +| `use` (also via `%%catalog use(path)`), `get_context` | | +| | `commit(retry=True)` | + +(`%catalog help`'s own ordering follows the same five groups — data, table, +view, wiki, tags — then folders, with `use`/`get_context` always first.) Every call takes a path and the arguments a custodian actually thinks in (`overwrite=True`, a list of tag strings, a folder path) — never an @@ -58,16 +96,16 @@ import eea_datalakehouse.notebook %ingest ingest(folder="./bw_2026", target_catalog_path="bwd.reference", data_format="parquet") %ingest commit(retry=True) -%catalog tag(".water_temperature", ["reviewed"]) -%catalog set_meta(".", tags=[{"tag_name": "owner", "tag_value": "bw-team", "tag_title": "Owner"}]) -%catalog commit(retry=True) +%catalog set_tags(".water_temperature", ["reviewed"]) +%catalog set_wiki(".", "# Water temperature\n\nBathing water assessments.") ``` -Ingest first and commit it fully; only then touch the catalog side, in a -separate `%catalog` batch — that ordering is enforced by convention (two -sessions), not by code. Nothing reaches Dremio until a `commit`; a failure -prints a short message and (catalog side) undoes whatever it safely can, -rather than a traceback. +Ingest first and commit it fully; only then touch the catalog side — that +ordering is enforced by convention (a catalog operation can't run before its +target exists), not by code. `%ingest` doesn't reach Dremio until a +`commit`; `%catalog` reaches it the moment each call runs. Either way a +failure prints a short message and undoes whatever it safely can, rather +than a traceback. ## The problem @@ -135,7 +173,7 @@ facade; it has no business appearing in a signature a data scientist calls. ## Two sessions, not one — ingestion and catalog are sequential, not peers -Catalog operations can't run before their target exists — a `datacopy`, tag or +Catalog operations can't run before their target exists — a `data_copy`, tag or wiki update on a table that hasn't been ingested yet has nothing to act on. So ingestion and catalog aren't two halves of one atomic batch; they're two phases, strictly ordered: an ingest fully lands and commits, *then* — and only @@ -150,8 +188,8 @@ ingest.ingest(folder="./bw_2026", target_catalog_path="…/bwd/reference", ...) ingest.commit(retry=True) # must fully succeed before anything below runs catalog = lakehouse.catalog_session() -catalog.copy("draft.raw_2026", "bwd.reference.water_temperature", overwrite=True) -catalog.tag("bwd.reference.water_temperature", reviewed_by="jdoe") +catalog.data_copy("draft.raw_2026", "bwd.reference.water_temperature", overwrite=True) +catalog.set_tags("bwd.reference.water_temperature", reviewed_by="jdoe") catalog.commit(retry=True) # all-or-nothing, but only across catalog steps ``` @@ -279,15 +317,17 @@ This splits into two sides that live in two different repositories. **The receiving side, here — implemented and encapsulated on both routes in:** `CatalogSession.set_context(path)`/`_resolve` (`catalog/session.py`) resolve the open question below with an explicit marker (a leading `.`, e.g. -`session.tag(".water_temperature", ...)`), and — critically — `set_context` -itself is not something a data custodian is expected to call: +`session.set_tags(".water_temperature", ...)`), and — critically — `set_context` +itself is not something a data custodian is expected to call directly +(`use`, added later, is the custodian-facing entry point built on the same +mechanism — see "What it exposes" above): - **Ordinary use already keeps it current on its own.** Every queueing verb updates the context from whatever path it just touched (the target's - parent for `copy`/`move`; the touched path itself for a folder-scoped verb - like `create_folder`/`set_meta`), so a script that never once calls - `set_context` still gets working relative paths after its first absolute - one. + parent for `data_copy`/`data_move`; the touched path itself for a + folder-scoped verb like `create_folder`), so a script that never once + calls `set_context` still gets working relative paths after its first + absolute one. - **The Comm channel below is the *other* caller**, not the custodian either. **The emitting side, in `eeadata/EEALakeHouse` — still not built:** the @@ -309,10 +349,12 @@ repository, which this session doesn't have open. `rich`/notebook-display block) rather than surfaced as a raw `CatalogOperationError` traceback. - A handful of high-level verbs matching how a custodian actually thinks about - the task, e.g. `session.copy(...)`, `session.tag(...)`, rather than the full - `datacopy`/`gettagsfrom`/`settagsto` vocabulary — queued and committed as - above, possibly also reachable as one-shot `%magic` commands for a true - single-call use. + the task, e.g. `session.set_tags(...)`, rather than the full `gettagsfrom`/ + `settagsto` vocabulary — queued and committed as above, possibly also + reachable as one-shot `%magic` commands for a true single-call use. + (`datacopy`/`datamove` are the one deliberate exception — later renamed + back to match `Catalog`'s own names, rather than staying `copy`/`move`; + see `%catalog help`'s current vocabulary for what actually shipped.) - Ties into the same gaps `dev-notes.md` already flags: missing docstrings on `Catalog`'s delegate methods (so `Shift+Tab` shows nothing useful today), and no pinned notebook environment to develop/test this against diff --git a/docs/notebooks/catalog_session_example.ipynb b/docs/notebooks/catalog_session_example.ipynb index 8073ef8..27e5d54 100644 --- a/docs/notebooks/catalog_session_example.ipynb +++ b/docs/notebooks/catalog_session_example.ipynb @@ -3,20 +3,7 @@ { "cell_type": "markdown", "metadata": {}, - "source": [ - "# `%catalog` — queue, commit, rollback\n", - "\n", - "A worked example of the `%catalog` magic from `eea_datalakehouse.notebook.magics` — see\n", - "`docs/notebook-facade-for-data-scientists.md` for the design behind it.\n", - "\n", - "**Before running this for real:** set `DREMIO_BASE_URL`, `DREMIO_TOKEN` (and `DREMIO_USERNAME`\n", - "if you'll queue `copy`/`move`) in the kernel environment — the same variables\n", - "`debugger/debug_run.py` uses. Install the extra this needs once: `pip install \"EEADataLakehouse[notebook]\"`.\n", - "\n", - "Nothing below invents new vocabulary: every call after `%catalog` is a real\n", - "`CatalogSession` method (`src/eea_datalakehouse/catalog/session.py`) — this notebook is a tour of\n", - "that class, not a separate API." - ] + "source": "# `%catalog` — runs immediately, no commit needed\n\nA worked example of the `%catalog` magic from `eea_datalakehouse.notebook.magics` — see\n`docs/notebook-facade-for-data-scientists.md` for the design behind it.\n\n**Before running this for real:** set `DREMIO_BASE_URL`, `DREMIO_TOKEN` (and `DREMIO_USERNAME`\nif you'll call `data_copy`/`data_move`) in the kernel environment — the same variables\n`debugger/debug_run.py` uses. Install the extra this needs once: `pip install \"EEADataLakehouse[notebook]\"`.\n\nNothing below invents new vocabulary: every call after `%catalog` is a real\n`CatalogSession` method (`src/eea_datalakehouse/catalog/session.py`) — this notebook is a tour of\nthat class, not a separate API. Each call runs against Dremio the moment its cell executes;\nthere's no separate commit step to remember (see `%catalog help`)." }, { "cell_type": "code", @@ -34,7 +21,7 @@ { "cell_type": "markdown", "id": "0f5e1ae8", - "source": "## Quick reference\n\n`%catalog help` (or `%catalog help()`) lists every `CatalogSession` method with its\nsignature and a short description — handy when you don't remember an exact parameter\nname mid-notebook. It works even before `DREMIO_BASE_URL`/`DREMIO_TOKEN` are set, since\nit's answered before a session is built.", + "source": "## Quick reference\n\n`%catalog help` (or `%catalog help()`) prints every command as a plain table — name,\nparameters, description — rather than a raw Python signature, since a data custodian\nreading it may not be fluent in Python type-hint syntax. Handy when you don't remember\nan exact parameter name mid-notebook. It works even before `DREMIO_BASE_URL`/\n`DREMIO_TOKEN` are set, since it's answered before a session is built.", "metadata": {} }, { @@ -48,28 +35,19 @@ { "cell_type": "markdown", "metadata": {}, - "source": [ - "## Queue a few steps\n", - "\n", - "Each cell only **queues** an operation — nothing reaches Dremio yet. The same one\n", - "`CatalogSession` is reused across every `%catalog` cell in this kernel (see \"Session\n", - "context lives in the Python process\" in the design doc), so calls in different cells\n", - "accumulate on the same batch." - ] + "source": "## Each call runs right away\n\nThere's no queue and no `commit()` to call afterwards — the moment a cell below runs, the\noperation has already happened against Dremio (with the same retry-on-cold-engine and\nrollback-on-failure safety `commit()` always had, applied automatically per call). The\nsame `CatalogSession` is still reused across every `%catalog` cell in this kernel, purely\nto keep the \"current path\" context up to date (see \"Session context lives in the Python\nprocess\" in the design doc) — not to batch anything." }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], - "source": [ - "%catalog copy(\"bwd.draft.raw_2026\", \"bwd.reference.water_temperature\", overwrite=True)" - ] + "source": "%catalog data_copy(\"bwd.draft.raw_2026\", \"bwd.reference.water_temperature\", overwrite=True)" }, { "cell_type": "markdown", "id": "d1f58114", - "source": "%catalog tag(\".water_temperature\", [\"reviewed\", \"2026\"])", + "source": "%catalog set_tags(\".water_temperature\", [\"reviewed\", \"2026\"])", "metadata": {} }, { @@ -77,81 +55,98 @@ "execution_count": null, "metadata": {}, "outputs": [], - "source": [ - "%catalog tag(\"bwd.reference.water_temperature\", [\"reviewed\", \"2026\"])" - ] + "source": "%catalog set_tags(\"bwd.reference.water_temperature\", [\"reviewed\", \"2026\"])" + }, + { + "cell_type": "markdown", + "id": "ecc02a7a", + "source": "## No commit needed\n\nBy the time the `set_tags` cell above finished running, both calls had already\nhappened — `%catalog` commits each one right after it runs (`retry=True` under the hood,\nso a step that hits a Dremio engine still warming up is re-attempted automatically).\nThere's nothing left to flush.", + "metadata": {} }, { "cell_type": "code", - "execution_count": null, + "id": "e3533db0", + "source": "%catalog get_context() # 'bwd.reference' — left there by set_tags above", "metadata": {}, - "outputs": [], - "source": [ - "%catalog set_meta(\"bwd.reference\", tags=[\n", - " {\"tag_name\": \"owner\", \"tag_value\": \"bathing-water-team\", \"tag_title\": \"Owner\"},\n", - "], overwrite=False)" - ] + "execution_count": null, + "outputs": [] }, { "cell_type": "markdown", + "id": "bed1a961", + "source": "### `%%catalog` — set context once for the whole cell\n\nThe cell-magic form runs `use(...)` on its magic line, then every other line of the\ncell body in order, all under that context — no need to repeat `%catalog` or the path\non each line.", + "metadata": {} + }, + { + "cell_type": "code", + "id": "7e896ad6", + "source": "%%catalog use(\"bwd.reference\")\nset_tags(\".water_temperature\", [\"archived\"])\ncreate_folder(\".2027\")", "metadata": {}, - "source": [ - "## Commit — all-or-nothing\n", - "\n", - "`commit()` runs every queued step in order. `retry=True` re-attempts a step that hits a\n", - "Dremio engine still warming up (`EngineStartingError`) a few times before giving up — any\n", - "other error rolls back immediately regardless. Either way the queue is empty afterwards,\n", - "whether this cell succeeds or raises." - ] + "execution_count": null, + "outputs": [] + }, + { + "cell_type": "markdown", + "id": "4b6abba9", + "source": "### `use`'s two differences from every other verb\n\n`create_folder` above still needed the leading `.` on `.2027` — every verb's own\n`path`/`source_path`/`target_path` does, so an absolute path can always be passed even\nwith a context already set. `use` alone accepts a bare path too, once a context\nexists — `use(\"2027\")` and `use(\".2027\")` mean the same thing there. `use(None)` clears\nthe context entirely, the same as `set_context(None)`.\n\n`use` also makes a live check: the resolved path must already exist in the catalog, or\nit raises instead of quietly pointing context somewhere later calls would fail against\nanyway. That's why the cell below re-enters `\"bwd.reference.2027\"`, created above,\nrather than a path nothing has created yet.", + "metadata": {} }, { "cell_type": "code", + "id": "417250b0", + "source": "%catalog use(\"bwd.reference\") # back to a path that already exists\n%catalog use(\"2027\") # bare, no dot — same as use(\".2027\"); already created above\n%catalog get_context() # -> 'bwd.reference.2027'", + "metadata": {}, "execution_count": null, + "outputs": [] + }, + { + "cell_type": "markdown", + "id": "9be5a25f", + "source": "### Going up: `../`\n\nOne or more leading `../` (or a bare `..`) walks up that many levels of the context\nfirst, then resolves whatever's left against the result — and unlike the dot-optional\nshortcut above, this works for *every* relative path, not just `use`'s.", + "metadata": {} + }, + { + "cell_type": "code", + "id": "28305b98", + "source": "%catalog set_tags(\"../water_temperature\", [\"archived\"]) # up one level, then a sibling — any verb\n%catalog get_context() # -> 'bwd.reference' — set_tags' own tracking already moved it there", "metadata": {}, - "outputs": [], - "source": [ - "%catalog commit(retry=True)" - ] + "execution_count": null, + "outputs": [] }, { "cell_type": "markdown", + "id": "6cbe67f0", + "source": "## Read-only queries: `get_wiki`, `get_tags`, `list`, `schema`\n\nThese answer immediately too, like `get_context()` above — nothing to commit or undo.\nAll four raise `CatalogOperationError` if `path` doesn't exist; `schema` also requires a\ntable or view, not a folder.", + "metadata": {} + }, + { + "cell_type": "code", + "id": "3a9a4515", + "source": "%catalog set_wiki(\"bwd.reference.water_temperature\", \"# Water temperature\\n\\nBathing water assessments.\")\n%catalog get_wiki(\"bwd.reference.water_temperature\")", "metadata": {}, - "source": [ - "`commit` with no parentheses also works, as a shorthand:" - ] + "execution_count": null, + "outputs": [] }, { "cell_type": "code", + "id": "9f29c191", + "source": "%catalog get_tags(\"bwd.reference.water_temperature\") # -> ['archived'] — the last set_tags() call above", + "metadata": {}, "execution_count": null, + "outputs": [] + }, + { + "cell_type": "code", + "id": "91156d7b", + "source": "%catalog list(\"bwd.reference\") # every table/view under bwd.reference\n%catalog schema(\"bwd.reference.water_temperature\") # column types + row count, no rows fetched", "metadata": {}, - "outputs": [], - "source": [ - "%catalog commit" - ] + "execution_count": null, + "outputs": [] }, { "cell_type": "markdown", "metadata": {}, - "source": [ - "## What a failed commit looks like\n", - "\n", - "If a step in the batch fails, `%catalog` prints a short message instead of a full\n", - "traceback (see the design doc's \"Exceptions translated at the boundary\") — for example,\n", - "queuing a step against a target that already exists without `overwrite=True`:\n", - "\n", - "```\n", - "%catalog copy(\"bwd.draft.raw_2026\", \"bwd.reference.water_temperature\")\n", - "%catalog commit\n", - "# -> catalog error: commit failed at step 1/1 (copy 'bwd.draft.raw_2026' -> ...):\n", - "# target 'bwd.reference.water_temperature' already exists — pass overwrite=True ...\n", - "# Everything before it was rolled back.\n", - "```\n", - "\n", - "A batch mixing a reversible step with an `overwrite=True` step is explicit about what it\n", - "could *not* undo rather than pretending the rollback was clean — see `CatalogCommitError`'s\n", - "docstring, and \"Must be all-or-nothing\" in the design doc, for exactly which verbs that\n", - "applies to (`delete_folder`, and `copy`/`move` when `overwrite=True`)." - ] + "source": "## What a failed call looks like\n\nIf a call fails, `%catalog` prints a short message instead of a full traceback (see the\ndesign doc's \"Exceptions translated at the boundary\") — for example, calling `data_copy`\nagainst a target that already exists without `overwrite=True`:\n\n```\n%catalog data_copy(\"bwd.draft.raw_2026\", \"bwd.reference.water_temperature\")\n# -> catalog error: commit failed at step 1/1 (data_copy 'bwd.draft.raw_2026' -> ...):\n# target 'bwd.reference.water_temperature' already exists — pass overwrite=True ...\n# Everything before it was rolled back.\n```\n\nThat's still `CatalogCommitError` underneath (each call is committed as its own\none-step batch — see `CatalogSession.commit`'s docstring), so the same \"rolled back, or\nexplicit about what it couldn't undo\" guarantee applies as before — it just never needs a\nseparate `%catalog commit` to trigger it. `delete_folder`, `delete_view`/`delete_table`, and\n`data_copy`/`data_move` with `overwrite=True`, still can't be undone — see \"Must be\nall-or-nothing\" in the design doc." } ], "metadata": { diff --git a/src/eea_datalakehouse/catalog/client.py b/src/eea_datalakehouse/catalog/client.py index 92d8337..85f4dbe 100644 --- a/src/eea_datalakehouse/catalog/client.py +++ b/src/eea_datalakehouse/catalog/client.py @@ -244,9 +244,32 @@ def datamove( idempotency_key=idempotency_key, ) + def createview( + self, + source_path: str, + target_path: str, + *, + overwrite: bool = False, + create_target_folder: bool = False, + idempotency_key: str, + ) -> SqlResult: + # Metadata-only (no data moves) — REST, not Flight, like table2view. + return operations.createview( + self._executor, + source_path, + target_path, + overwrite=overwrite, + create_target_folder=create_target_folder, + catalog_rest=self._catalog_rest, + idempotency_key=idempotency_key, + ) + def deleteview(self, view_path: str, *, idempotency_key: str) -> SqlResult: return operations.deleteview(self._executor, view_path, idempotency_key=idempotency_key) + def deletetable(self, table_path: str, *, idempotency_key: str) -> SqlResult: + return operations.deletetable(self._executor, table_path, idempotency_key=idempotency_key) + def gettablesfrom(self, schema_path: str, *, idempotency_key: str) -> list[str]: return operations.gettablesfrom( self._executor, schema_path, idempotency_key=idempotency_key diff --git a/src/eea_datalakehouse/catalog/operations.py b/src/eea_datalakehouse/catalog/operations.py index 5442c83..0fc3c7e 100644 --- a/src/eea_datalakehouse/catalog/operations.py +++ b/src/eea_datalakehouse/catalog/operations.py @@ -579,6 +579,109 @@ def verify_target_exists() -> SqlResult: ) +def createview( + executor: SqlExecutor, + source_path: str, + target_path: str, + *, + overwrite: bool = False, + create_target_folder: bool = False, + catalog_rest: CatalogRestClient | None = None, + idempotency_key: str, +) -> SqlResult: + """Create a view at `target_path` over `source_path`: + ``CREATE VIEW ... AS SELECT * FROM source_path``. + + Unlike `datacopy`/`datamove`, this creates no new data of its own — a + view is a saved query, re-evaluated against `source_path` on every + read — so it runs over Dremio's REST Jobs API like every other + metadata-only operation (see the module docstring), not Arrow Flight. + `source_path` itself is never touched — non-destructive, unlike + `datamove`. + + Checks `source_path` actually exists (as a table or view) before doing + anything else, so a typo fails with a clear message instead of a + confusing CREATE VIEW error. If `target_path` is an existing folder + rather than a specific table/view path, the source's own name is + appended to it — `cp source dest/` semantics, landing the view inside + that folder under the same name (needs `catalog_rest`; skipped, using + `target_path` exactly as given, without one). + + `overwrite=False` (the default) checks the (possibly folder-adjusted) + target explicitly and raises `CatalogOperationError` if it already + exists, rather than letting a bare CREATE VIEW fail with Dremio's own + less specific error. `overwrite=True` drops whatever is actually there + first — detecting its real kind, since it might be a table, not a + view. When `create_target_folder` is true, also ensures its containing + folder exists first, creating any missing levels — this requires + `catalog_rest`. + """ + params: dict[str, Any] = { + "source_path": source_path, + "target_path": target_path, + "overwrite": overwrite, + "create_target_folder": create_target_folder, + } + + def check_source_exists() -> None: + if not _entry_exists(executor, source_path, idempotency_key=idempotency_key): + raise CatalogOperationError(f"source {source_path!r} does not exist") + + def resolve_target() -> None: + nonlocal target_path + target_path = _resolve_target_path(catalog_rest, target_path, source_path) + params["target_path"] = target_path + + def ensure_target_folder() -> None: + if not create_target_folder: + return + parent = _parent_path(target_path) + if parent is None: + return + if catalog_rest is None: + raise CatalogOperationError( + "createview(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, target_path, idempotency_key=idempotency_key): + return # nothing there — nothing to check or drop + if not overwrite: + raise CatalogOperationError( + f"target {target_path!r} already exists — pass overwrite=True to replace it" + ) + # Drop whatever it actually is (it might be a table, not a view) — + # DROP VIEW on a table (or vice versa) fails outright, same class of + # bug datamove's entry_type problem guards against. + existing_kind = _entry_kind(executor, target_path, idempotency_key=idempotency_key) + executor.execute( + f"DROP {existing_kind} IF EXISTS {_quote_path(target_path)}", + idempotency_key=idempotency_key, + ) + + def create_view() -> SqlResult: + return executor.execute( + f"CREATE VIEW {_quote_path(target_path)} AS SELECT * FROM {_quote_path(source_path)}", + idempotency_key=idempotency_key, + ) + + return _run_actions( + [ + check_source_exists, + resolve_target, + ensure_target_folder, + check_or_drop_target, + create_view, + ], + operation="createview", + target=target_path, + idempotency_key=idempotency_key, + params=params, + ) + + def deleteview( executor: SqlExecutor, view_path: str, @@ -603,6 +706,30 @@ def deleteview( ) +def deletetable( + executor: SqlExecutor, + table_path: str, + *, + idempotency_key: str, +) -> SqlResult: + """Delete the table at `table_path`. + + ``DROP TABLE IF EXISTS`` — idempotent, safe to retry or re-run even if + the table is already gone. If `table_path` is actually a view rather + than a table, this fails loudly rather than silently doing nothing or + dropping the wrong kind of entry — the mirror image of `deleteview`. + """ + statements = [f"DROP TABLE IF EXISTS {_quote_path(table_path)}"] + return _run_steps( + executor, + statements, + operation="deletetable", + target=table_path, + idempotency_key=idempotency_key, + params={"table_path": table_path}, + ) + + def _like_escape(value: str) -> str: """Escape a LIKE pattern's own wildcards so a literal `_`/`%` in a path segment (real folder/table names routinely contain underscores) isn't @@ -1148,7 +1275,9 @@ def deletefolder( "publishversion": publishversion, "datacopy": datacopy, "datamove": datamove, + "createview": createview, "deleteview": deleteview, + "deletetable": deletetable, "gettablesfrom": gettablesfrom, "gettableitemsfrom": gettableitemsfrom, "getwikifrom": getwikifrom, diff --git a/src/eea_datalakehouse/catalog/session.py b/src/eea_datalakehouse/catalog/session.py index b90802b..6a15da4 100644 --- a/src/eea_datalakehouse/catalog/session.py +++ b/src/eea_datalakehouse/catalog/session.py @@ -6,16 +6,16 @@ keys) and adds queue/commit/rollback on top:: session = CatalogSession(catalog) - session.copy("draft.raw_2026", "bwd.reference.water_temperature") - session.tag("bwd.reference.water_temperature", ["reviewed"]) + session.data_copy("draft.raw_2026", "bwd.reference.water_temperature") + session.set_tags("bwd.reference.water_temperature", ["reviewed"]) session.commit(retry=True) `commit()` is all-or-nothing: every queued step runs in order, and the moment one fails, everything already done in *this* commit is undone before the failure is reported — see each verb's own docstring below for exactly what its compensating action is, and where one isn't possible (`delete_folder`, -or `copy`/`move` with `overwrite=True`) that limitation is raised as part of -the failure, not hidden. +`delete_view`/`delete_table`, or `data_copy`/`data_move` with `overwrite=True`) +that limitation is raised as part of the failure, not hidden. This is NOT the same "session" `dds_ingestion.folder.FolderIngest` already has (a server-side transfer identified by `session_id`, resumed via @@ -34,11 +34,11 @@ import uuid from collections.abc import Callable from dataclasses import dataclass, field -from typing import Literal from . import operations from .client import Catalog from .errors import CatalogOperationError, EngineStartingError +from .operations import TableInfo _Undo = Callable[[], None] @@ -53,7 +53,8 @@ class CatalogCommitError(CatalogSessionError): `rolled_back` is `True` only if every already-succeeded step in this commit was cleanly undone. When it's `False`, `unresolved` names which steps are NOT undone — either because that verb has no compensating - action at all (`delete_folder`; `copy`/`move` with `overwrite=True`), or + action at all (`delete_folder`, `delete_view`/`delete_table`; `data_copy`/ + `data_move` with `overwrite=True`), or because undoing it was attempted and itself failed. Either way, the catalog is left in a state this object cannot fully explain away, and a human needs to look. @@ -105,11 +106,12 @@ def _read_wiki_or_none(catalog: Catalog, path: str, *, idempotency_key: str) -> def _drop_entry(catalog: Catalog, path: str, *, idempotency_key: str) -> None: """Drop whatever entity (table or view) now sits at `path`. - Used to undo a fresh `copy`. Reaches into `operations`' own private + Used to undo a fresh `data_copy`. Reaches into `operations`' own private helpers (`_entry_kind`, `_quote_path`) rather than a public op, because - there isn't one: `deleteview` only drops VIEWs, and a plain `datacopy` - target is always a TABLE (`CREATE TABLE ... AS SELECT`). Acceptable - here since this module lives in the same package as `operations.py`. + the public `deleteview`/`deletetable` (via `Catalog`) run over REST, not + Flight — this needs to stay on `catalog._flight_executor` like the + `data_copy`/`data_move` it's undoing. Acceptable here since this module + lives in the same package as `operations.py`. """ executor = catalog._flight_executor # noqa: SLF001 — same-package internal, see docstring kind = operations._entry_kind(executor, path, idempotency_key=idempotency_key) # noqa: SLF001 @@ -122,12 +124,13 @@ def _drop_entry(catalog: Catalog, path: str, *, idempotency_key: str) -> None: class CatalogSession: """Queue catalog verbs against `catalog`, then `commit()` them as one batch. - Every queueing method (`copy`, `move`, `tag`, `untag`, `set_wiki`, - `delete_wiki`, `set_meta`, `create_folder`, `delete_folder`) only records - the intent — nothing reaches Dremio until `commit()`. Each returns + Every queueing method (`data_copy`, `data_move`, `create_view`, `set_tags`, + `delete_tags`, `set_wiki`, `delete_wiki`, `create_folder`, + `delete_folder`, `delete_view`, `delete_table`) only records the intent — + nothing reaches Dremio until `commit()`. Each returns `self`, so calls chain:: - session.copy(...).tag(...).commit() + session.data_copy(...).set_tags(...).commit() `idempotency_key`s are generated internally (`session--`, one per step) — never pass or think about one; that's exactly the ceremony this @@ -151,12 +154,17 @@ def _key(self, suffix: str = "") -> str: def set_context(self, path: str | None) -> CatalogSession: """Set the "current" catalog path — a later relative path (a leading `.`, e.g. `.water_temperature`) resolves against this. `None` clears it. - - Not something a data custodian should normally call: ordinary use - already keeps this up to date on its own (see `_resolve` — every - queued step updates it from whatever path it just touched), and an - integration (the JupyterLab catalog-tree extension pushing a - selected leaf through a Comm — see + `path` here is always taken literally, even if it itself starts with + `.` — see `use` for a version that resolves a leading `.` against + whatever context already exists, which is what a data custodian + deliberately narrowing the context (e.g. `%%catalog use(".2027")`) + actually wants. + + Not something a data custodian should normally call directly: + ordinary use already keeps this up to date on its own (see + `_resolve` — every queued step updates it from whatever path it + just touched), and an integration (the JupyterLab catalog-tree + extension pushing a selected leaf through a Comm — see `docs/notebook-facade-for-data-scientists.md`, "Pre-filling catalog context") is the other caller, seeding it before any path has been typed yet. @@ -164,11 +172,78 @@ def set_context(self, path: str | None) -> CatalogSession: self._context = path return self + def use(self, path: str | None) -> CatalogSession: + """Set the current path for every call after this one — same as + `set_context`, but `path` may also be relative rather than only a + whole path, and `None` clears it exactly like `set_context(None)`. + A relative `path` may lead with a `.` (`".2027"`) or not + (`"2027"`) — both resolve against whatever context already exists + the same way; the dot is optional sugar here, unlike everywhere + else in this class (every other verb's `path`/`source_path`/ + `target_path` still requires it to mean relative, so an absolute + path can always be passed even with a context already set — see + `_resolve_path`). Once a context exists, `use` therefore has no way + to jump straight to an unrelated absolute path without a leading + dot; clear it first (`use(None)`) if that's what's needed. `path` + may also lead with one or more `../` (or be a bare `..`) to walk + up that many levels of the context first — `use("../stations")` + moves to a sibling of the context, `use("..")` to its parent. + + This is the one custodian-facing way to set context deliberately + (see `%%catalog use(path)` — the cell magic sets it once at the + top of a cell, for every call in that cell and every cell after + it, until changed again); ordinary use already keeps context + current on its own, so this is only needed to jump somewhere a + queued step hasn't already touched. + + Unlike everything else in this class, this makes a live call + against Dremio: the resolved path must already exist as some + catalog entity (folder, table, or view) — raises + `CatalogSessionError` otherwise, so context never silently points + somewhere real work would fail against later. Always stores the + fully resolved path — never a `.`/`..`-prefixed fragment — so + `get_context()` reads back exactly what `use` just checked. + """ + if path is None: + self._context = None + return self + if self._context is not None and not path.startswith("."): + path = f".{path}" + resolved = self._resolve_path(path) + try: + found = self._catalog._catalog_rest.exists(resolved) # noqa: SLF001 — see module docstring + except (CatalogOperationError, EngineStartingError) as exc: + raise CatalogSessionError( + f"could not check whether {resolved!r} exists: {exc}" + ) from exc + if not found: + raise CatalogSessionError(f"{resolved!r} does not exist in the catalog") + self._context = resolved + return self + + def get_context(self) -> str | None: + """The current path, always the full resolved path — never a + `.`/`..`-prefixed fragment, even right after a relative `use` — + see `use`/`set_context`. `None` if nothing has been set yet.""" + return self._context + def _resolve_path(self, path: str) -> str: - """Just the relative -> absolute resolution (a leading `.` resolves - against the current context) — does NOT update the context; see - `_resolve` for the verbs where the touched path also becomes the - new context.""" + """Just the relative -> absolute resolution — does NOT update the + context; see `_resolve` for the verbs where the touched path also + becomes the new context. Always returns a full, already-resolved + path (never a `.`/`..`-prefixed fragment), so a value stored from + here — including into `self._context` — is safe to read back as-is + (see `get_context`). + + A leading `.` resolves against the current context + (`".water_temperature"` -> `f"{context}.water_temperature"`). One + or more leading `../` segments (or a bare `..`) instead walk up + that many levels of the context *first* — `"../stations"` is a + sibling of the context, `"../../stations"` a level further up, and + so on; `".."` alone (no name after it) resolves to the context's + parent itself.""" + if path == ".." or path.startswith("../"): + return self._resolve_parent_path(path) if not path.startswith("."): return path if self._context is None: @@ -178,21 +253,47 @@ def _resolve_path(self, path: str) -> str: ) return f"{self._context}{path}" + def _resolve_parent_path(self, path: str) -> str: + """The `..`/`../...` half of `_resolve_path` — walk `self._context` + up one level per leading `../` (or the single `..`), then append + whatever's left, if anything.""" + if self._context is None: + raise CatalogSessionError( + f"{path!r} is relative (starts with '..') but no context is set yet — " + "use an absolute path first, or call set_context()" + ) + ancestor = self._context + remainder = path + while remainder == ".." or remainder.startswith("../"): + parent = operations._parent_path(ancestor) # noqa: SLF001 — same-package internal + if parent is None: + raise CatalogSessionError( + f"{path!r} goes above the top of the current context {self._context!r}" + ) + ancestor = parent + remainder = remainder[3:] if remainder.startswith("../") else "" + if not remainder: + return ancestor + if not remainder.startswith("."): + remainder = f".{remainder}" + return f"{ancestor}{remainder}" + def _resolve(self, path: str) -> str: """Resolve `path` (see `_resolve_path`), then update the context to its own parent — so the next short name keeps resolving against wherever this one just landed, without anyone calling - `set_context` again. Verbs with two paths (`copy`/`move`) resolve - both against the *same* starting context via `_resolve_path` - directly instead — only the target should become the new context, - not whatever `source_path`'s own folder happens to be.""" + `set_context` again. Verbs with two paths (`data_copy`/`data_move`/ + `create_view`) resolve both against the *same* starting context via + `_resolve_path` directly instead — only the target should become + the new context, not whatever `source_path`'s own folder happens + to be.""" resolved = self._resolve_path(path) self._context = operations._parent_path(resolved) or self._context # noqa: SLF001 return resolved # -- queueing verbs --------------------------------------------------- - def copy( + def data_copy( self, source_path: str, target_path: str, @@ -200,7 +301,7 @@ def copy( overwrite: bool = False, create_target_folder: bool = False, ) -> CatalogSession: - """Queue a `datacopy`. Reversible only when `overwrite=False` (there + """Queue a `data_copy`. Reversible only when `overwrite=False` (there was nothing at `target_path` to lose) — undo drops the table this step created. With `overwrite=True`, whatever used to be at `target_path` is gone the moment this step runs; there is nothing to @@ -229,22 +330,22 @@ def undo() -> None: return undo - self._steps.append(_Step(f"copy {source_path!r} -> {target_path!r}", run)) + self._steps.append(_Step(f"data_copy {source_path!r} -> {target_path!r}", run)) return self - def move( + def data_move( self, source_path: str, target_path: str, *, - entry_type: Literal["TABLE", "VIEW"] | None = None, overwrite: bool = False, create_target_folder: bool = False, ) -> CatalogSession: - """Queue a `datamove`. Reversible only when `overwrite=False` — undo - is a `datamove` back from `target_path` to `source_path` (the - target's kind is re-detected at undo time, not assumed). Same - `overwrite=True` limitation as `copy`. + """Queue a `data_move`, always as a TABLE — use `create_view` instead + to create a view over `source_path` without touching it. Reversible + only when `overwrite=False` — undo is a `data_move` back from + `target_path` to `source_path`. Same `overwrite=True` limitation as + `data_copy`. Either path may be relative (a leading `.`) to the session's current context — see `set_context`; `target_path` becomes the new context @@ -257,7 +358,7 @@ def run(catalog: Catalog, key: str) -> _Undo | None: catalog.datamove( source_path, target_path, - entry_type=entry_type, + entry_type="TABLE", overwrite=overwrite, create_target_folder=create_target_folder, idempotency_key=key, @@ -266,26 +367,62 @@ def run(catalog: Catalog, key: str) -> _Undo | None: raise _Irreversible("overwrote an existing target; its previous content is gone") def undo() -> None: - undo_key = f"{key}-undo" - kind = operations._entry_kind( # noqa: SLF001 — see _drop_entry - catalog._flight_executor, # noqa: SLF001 - target_path, - idempotency_key=f"{undo_key}-detect", - ) catalog.datamove( target_path, source_path, - entry_type=kind, + entry_type="TABLE", overwrite=False, - idempotency_key=undo_key, + idempotency_key=f"{key}-undo", ) return undo - self._steps.append(_Step(f"move {source_path!r} -> {target_path!r}", run)) + self._steps.append(_Step(f"data_move {source_path!r} -> {target_path!r}", run)) return self - def tag(self, path: str, tags: list[str]) -> CatalogSession: + def create_view( + self, + source_path: str, + target_path: str, + *, + overwrite: bool = False, + create_target_folder: bool = False, + ) -> CatalogSession: + """Queue a `createview` — creates a VIEW at `target_path` over + `source_path`, leaving `source_path` itself untouched (unlike + `data_move`). Reversible only when `overwrite=False` (there was nothing + at `target_path` to lose) — undo drops the view this step created. + With `overwrite=True`, whatever used to be at `target_path` is gone + the moment this step runs; there is nothing to restore, so this + step cannot be undone (see `CatalogCommitError`). + + Either path may be relative (a leading `.`) to the session's current + context — see `set_context`; `target_path` becomes the new context + afterwards.""" + source_path = self._resolve_path(source_path) + target_path = self._resolve_path(target_path) + self._context = operations._parent_path(target_path) or self._context # noqa: SLF001 + + def run(catalog: Catalog, key: str) -> _Undo | None: + catalog.createview( + source_path, + target_path, + overwrite=overwrite, + create_target_folder=create_target_folder, + idempotency_key=key, + ) + if overwrite: + raise _Irreversible("overwrote an existing target; its previous content is gone") + + def undo() -> None: + _drop_entry(catalog, target_path, idempotency_key=f"{key}-undo") + + return undo + + self._steps.append(_Step(f"create view {source_path!r} -> {target_path!r}", run)) + return self + + def set_tags(self, path: str, tags: list[str]) -> CatalogSession: """Queue `settagsto` (replaces the tag set). Undo restores whatever tags were on `path` immediately before this step ran. @@ -302,10 +439,10 @@ def undo() -> None: return undo - self._steps.append(_Step(f"tag {path!r} with {tags!r}", run)) + self._steps.append(_Step(f"set tags {tags!r} on {path!r}", run)) return self - def untag(self, path: str, tags: list[str]) -> CatalogSession: + def delete_tags(self, path: str, tags: list[str]) -> CatalogSession: """Queue `deletetags`. Undo restores the full tag set `path` had immediately before this step ran. @@ -322,7 +459,7 @@ def undo() -> None: return undo - self._steps.append(_Step(f"untag {tags!r} from {path!r}", run)) + self._steps.append(_Step(f"delete tags {tags!r} from {path!r}", run)) return self def set_wiki( @@ -373,37 +510,6 @@ def undo() -> None: self._steps.append(_Step(f"delete wiki on {path!r}", run)) return self - def set_meta( - self, path: str, tags: list[dict[str, str]] | None = None, *, overwrite: bool = True - ) -> CatalogSession: - """Queue `setmeta2wiki`. Undo restores the whole previous wiki text - (not just its Meta Data section) verbatim, or deletes it if `path` - had none. - - `path` may be relative (a leading `.`) to the session's current - context — see `set_context`. Unlike a leaf-level verb (`tag`, - `set_wiki`, ...), `path` here is a folder (see `_require_folder`), - so it becomes the new context *itself*, not its parent — the next - short name is expected to name something *inside* it.""" - path = self._resolve_path(path) - self._context = path - - def run(catalog: Catalog, key: str) -> _Undo: - existing = _read_wiki_or_none(catalog, path, idempotency_key=f"{key}-read") - catalog.setmeta2wiki(path, tags=tags, overwrite=overwrite, idempotency_key=key) - - def undo() -> None: - undo_key = f"{key}-undo" - if existing is None: - catalog.deletewiki(path, idempotency_key=undo_key) - else: - catalog.setwikito(path, existing, idempotency_key=undo_key) - - return undo - - self._steps.append(_Step(f"set metadata on {path!r}", run)) - return self - def create_folder(self, path: str, *, create_parents: bool = False) -> CatalogSession: """Queue `createfolder`. Undo deletes it — but only if this step actually created it; `createfolder` is idempotent, so a `path` @@ -447,6 +553,108 @@ def run(catalog: Catalog, key: str) -> _Undo | None: self._steps.append(_Step(f"delete folder {path!r}", run)) return self + def delete_view(self, path: str) -> CatalogSession: + """Queue `deleteview`. **Never reversible** — the dropped view's + definition isn't captured anywhere, so there's nothing to recreate + it from. Unlike `Catalog.deleteview`'s own `DROP VIEW IF EXISTS` + (idempotent — a no-op on a missing path), this raises + `CatalogOperationError` if `path` doesn't exist at all, so a typo + fails clearly instead of silently doing nothing. + + `path` may be relative (a leading `.`) to the session's current + context — see `set_context`. Does not change the context itself — + there's nothing meaningful to navigate into once it's deleted.""" + path = self._resolve_path(path) + + def run(catalog: Catalog, key: str) -> _Undo | None: + if not catalog._catalog_rest.exists(path): # noqa: SLF001 — see module docstring + raise CatalogOperationError(f"{path!r} does not exist") + catalog.deleteview(path, idempotency_key=key) + raise _Irreversible("dropped view's definition cannot be recreated") + + self._steps.append(_Step(f"delete view {path!r}", run)) + return self + + def delete_table(self, path: str) -> CatalogSession: + """Queue `deletetable`. **Never reversible** — the dropped table's + data isn't captured anywhere, so there's nothing to recreate it + from. Unlike `Catalog.deletetable`'s own `DROP TABLE IF EXISTS` + (idempotent — a no-op on a missing path), this raises + `CatalogOperationError` if `path` doesn't exist at all, so a typo + fails clearly instead of silently doing nothing. + + `path` may be relative (a leading `.`) to the session's current + context — see `set_context`. Does not change the context itself — + there's nothing meaningful to navigate into once it's deleted.""" + path = self._resolve_path(path) + + def run(catalog: Catalog, key: str) -> _Undo | None: + if not catalog._catalog_rest.exists(path): # noqa: SLF001 — see module docstring + raise CatalogOperationError(f"{path!r} does not exist") + catalog.deletetable(path, idempotency_key=key) + raise _Irreversible("dropped table's data cannot be recreated") + + self._steps.append(_Step(f"delete table {path!r}", run)) + return self + + # -- read-only queries — answered immediately, never queued ------------- + + def get_wiki(self, path: str) -> str: + """The wiki text at `path` (see `getwikifrom`). Answered immediately + — not queued, since there's nothing to commit or undo. Raises + `CatalogOperationError` if `path` doesn't exist, or exists but has + no wiki at all. + + `path` may be relative (a leading `.`) to the session's current + context — see `set_context`.""" + path = self._resolve_path(path) + return self._catalog.getwikifrom(path, idempotency_key=self._key()) + + def get_tags(self, path: str) -> list[str]: + """The tags on `path` (see `gettagsfrom`) — tables/views only. + Answered immediately — not queued, since there's nothing to commit + or undo. Raises `CatalogOperationError` if `path` doesn't exist or + isn't a table/view (folders have Dremio's own wiki Meta Data + section for this instead — see `Catalog.setmeta2wiki`). + + `path` may be relative (a leading `.`) to the session's current + context — see `set_context`.""" + path = self._resolve_path(path) + return self._catalog.gettagsfrom(path, idempotency_key=self._key()) + + def list(self, path: str) -> list[str]: + """Full paths of every table and view under `path`, at any depth + (see `gettablesfrom`). Answered immediately — not queued, since + there's nothing to commit or undo. Raises `CatalogOperationError` + if `path` doesn't exist — `gettablesfrom` itself doesn't check this + (an empty result and "nothing there" look the same to it), so this + checks first rather than returning `[]` for a typo'd path. + + `path` may be relative (a leading `.`) to the session's current + context — see `set_context`.""" + path = self._resolve_path(path) + if not self._catalog._catalog_rest.exists(path): # noqa: SLF001 — see module docstring + raise CatalogOperationError(f"{path!r} does not exist") + return self._catalog.gettablesfrom(path, idempotency_key=self._key()) + + def schema(self, path: str) -> TableInfo: + """The column schema and row count of `path` (see + `gettableitemsfrom`) — never fetches the actual rows. Answered + immediately — not queued, since there's nothing to commit or undo. + `path` must be a table or view — raises `CatalogOperationError` + otherwise (a folder has no schema of its own), or if `path` doesn't + exist at all. + + `path` may be relative (a leading `.`) to the session's current + context — see `set_context`.""" + path = self._resolve_path(path) + operations._require_table_or_view( # noqa: SLF001 — see module docstring + self._catalog._catalog_rest, # noqa: SLF001 + path, + "schema", + ) + return self._catalog.gettableitemsfrom(path, idempotency_key=self._key()) + # -- commit ------------------------------------------------------------- def commit( diff --git a/src/eea_datalakehouse/notebook/magics.py b/src/eea_datalakehouse/notebook/magics.py index 1598d02..06fdb9d 100644 --- a/src/eea_datalakehouse/notebook/magics.py +++ b/src/eea_datalakehouse/notebook/magics.py @@ -1,6 +1,6 @@ -"""`%catalog` and `%ingest` — thin syntactic sugar over `CatalogSession`/ -`IngestSession` (see `docs/notebook-facade-for-data-scientists.md`, "Two -magics, two sessions"). +"""`%catalog`/`%%catalog` and `%ingest` — thin syntactic sugar over +`CatalogSession`/`IngestSession` (see +`docs/notebook-facade-for-data-scientists.md`, "Two magics, two sessions"). Available the moment this package is imported inside IPython — `import eea_datalakehouse.notebook` (see that package's `__init__.py`) registers both @@ -8,29 +8,35 @@ eea_datalakehouse.notebook.magics` still works too (and is the only option outside IPython's auto-import path, e.g. a config that imports this module directly) — `load_ipython_extension` below is a no-op if the auto-import -already registered these, so using both never double-registers or drops a -session's queued-but-not-committed state. +already registered these, so using both never double-registers or drops an +`IngestSession`'s queued-but-not-committed state. In any cell, once loaded either way:: - %catalog copy("draft.raw_2026", "bwd.reference.water_temperature") - %catalog tag("bwd.reference.water_temperature", ["reviewed"]) - %catalog commit(retry=True) + %catalog data_copy("draft.raw_2026", "bwd.reference.water_temperature") + %catalog set_tags("bwd.reference.water_temperature", ["reviewed"]) %ingest ingest(folder="./bw_2026", target_catalog_path="bwd.reference", data_format="parquet", table_name="water_temperature") %ingest commit(retry=True) -Each magic evaluates the rest of the line as a method call on ONE session -object, created lazily the first time that magic is used in this kernel and -kept for the kernel's lifetime (see the design doc's "Session context lives -in the Python process") — so calls in different cells accumulate on the same -queue, and `commit()` flushes it. `%catalog`/`%ingest` invent no vocabulary -of their own: every name after them is a real `CatalogSession`/`IngestSession` -method (see those modules for the full list) — this file only builds the -session and turns exceptions into a short printed message instead of a +`%catalog` executes each call immediately — there's no queue and no separate +commit step to remember. A `CatalogSession` still sits underneath it (to keep +the "current path" context, and the same retry-on-`EngineStartingError`/ +rollback-on-failure safety `commit()` always had — see `CatalogSession.commit`), +but that's an implementation detail this magic hides by committing right +after every call. `%ingest`, on the other hand, still queues (`IngestSession`) +and needs an explicit `commit()` — a catalog operation can't run before its +target has actually been ingested, so `%ingest`'s batch and `%catalog`'s +immediate calls were never meant to share one queue anyway (see the design +doc's "Two sessions, not one"). `%catalog`/`%ingest` invent no vocabulary of +their own: every name after them is a real `CatalogSession`/`IngestSession` +method (`%catalog help`/`%ingest help` lists them) — this file only builds +the session(s) and turns exceptions into a short printed message instead of a traceback, per the design doc's facade plan. `commit` with no parentheses is -accepted as a convenience for `commit()`. +accepted as a convenience for `commit()` — still meaningful for `%ingest`; a +no-op for `%catalog`, whose queue is always empty by the time a cell +finishes. `eval()` below runs exactly the Python the user typed after the magic name, in their own notebook namespace — no new capability over what the same user @@ -45,22 +51,35 @@ `CatalogSession`'s "current path" context (a leading `.` on a path resolves against it — see that class' `set_context`/`_resolve`) is already kept up to date automatically just from ordinary `%catalog` use, so no data -custodian ever needs to call `set_context` themselves. On top of that, this -module also registers a Jupyter Comm target (see `_register_context_comm`) -so an integrated frontend — the JupyterLab catalog-tree extension, in the -separate `eeadata/EEALakeHouse` repo — can push a selected leaf's path into -this kernel invisibly: no cell runs, nothing a custodian could see or type, -either. See `docs/notebook-facade-for-data-scientists.md`, "Pre-filling -catalog context from a JupyterLab tree click". +custodian ever needs to set it themselves for that. `use(path)` sets it +deliberately instead — unlike `set_context`, `path` itself may be relative +too, resolved against whatever context already exists. Most often reached +through `%%catalog`, the cell-magic form: it runs `use(...)` on its magic +line, then every other line of the cell in order, so the whole cell shares +one context without repeating a path on each line:: + + %%catalog use("bwd.reference") + tag(".water_temperature", ["reviewed"]) + create_folder(".2027") + +This module also registers a Jupyter Comm target (see +`_register_context_comm`) so an integrated frontend — the JupyterLab +catalog-tree extension, in the separate `eeadata/EEALakeHouse` repo — can +push a selected leaf's path into this kernel invisibly: no cell runs, +nothing a custodian could see or type, either. See +`docs/notebook-facade-for-data-scientists.md`, "Pre-filling catalog context +from a JupyterLab tree click". """ from __future__ import annotations import inspect import os +from html import escape as _escape from typing import Any -from IPython.core.magic import Magics, line_magic, magics_class +from IPython.core.magic import Magics, cell_magic, line_magic, magics_class +from IPython.display import HTML, display from ..catalog import Catalog from ..catalog.session import CatalogSession, CatalogSessionError @@ -68,7 +87,8 @@ _USAGE = { "catalog": ( - '%catalog copy("a.b", "c.d") | %catalog tag("a.b", ["reviewed"]) | %catalog commit' + '%catalog data_copy("a.b", "c.d") | %catalog set_tags("a.b", ["reviewed"])' + " (each call runs immediately)" ), "ingest": ( '%ingest ingest(folder="./data", target_catalog_path="a.b", data_format="parquet")' @@ -76,25 +96,130 @@ ), } +# Shared between set_wiki's _CATALOG_HELP entry below and set_tags'/ +# delete_tags' spirit (tags: a list of {tag_name, ...} dicts) — kept as one +# constant so the wiki-tags example can't quietly drift. +_META_TAGS_EXAMPLE = '[{"tag_name": "owner", "tag_value": "bw-team", "tag_title": "Owner"}]' + +# Shared between copy/move/create_view's _CATALOG_HELP entries — all three +# resolve create_target_folder the same way (see _resolve_target_path's +# ensure_target_folder step in operations.py). +_CREATE_TARGET_FOLDER_NOTE = ( + "create_target_folder=True creates every missing folder level of target_path — " + "except the space/source itself, which must already exist (raises " + "CatalogOperationError otherwise)." +) + # One short line per public method — shown by `%catalog help`/`%ingest help`. # Kept separate from each method's own (much longer) docstring on purpose: # this is a quick-reference table, not a replacement for reading the real -# docstring in `catalog/session.py`/`dds_ingestion/session.py`. +# docstring in `catalog/session.py`/`dds_ingestion/session.py`. `commit` is +# deliberately left off `_CATALOG_HELP` — it still exists on `CatalogSession` +# (auto_commit in `_dispatch` calls it), but isn't something a custodian needs +# to call themselves any more. `set_context` is left off too — `use` does the +# same job and also resolves a relative path, so it's the one to list. +# +# Ordered deliberately, not alphabetically: `use`/`get_context` first (context +# is always the first thing to reach for), then five grouped sections — data, +# table, view, wiki, tags — each set/delete/get (create/delete for view, since +# there's no "get" for one), and `create_folder`/`delete_folder` last, since +# folders aren't one of those five groups. _CATALOG_HELP = [ ( - "set_context", - "Set the current path; a later relative path (leading '.') resolves against it.", + "use", + "Set the current path for every call after this one; path may be a whole path, " + "or relative to the current context (with or without a leading '.') once one " + "exists — '../name' or a bare '..' walks up one level first, chainable " + "('../../name'). None clears it. Raises if the resolved path doesn't exist in " + "the catalog. See %%catalog to set it once at the top of a cell.", + ), + ( + "get_context", + "Show the current path (None if nothing has been set yet).", + ), + # -- data --------------------------------------------------------------- + ( + "data_copy", + "Copy source_path to target_path. overwrite=False (default) raises " + "CatalogOperationError if target_path already exists; overwrite=True replaces it " + "and is not undoable. " + _CREATE_TARGET_FOLDER_NOTE, + ), + ( + "data_move", + "Move source_path to target_path, always as a TABLE (use create_view for a view). " + "overwrite=False (default) raises CatalogOperationError if target_path already " + "exists; overwrite=True replaces it and is not undoable. " + _CREATE_TARGET_FOLDER_NOTE, + ), + # -- table ---------------------------------------------------------------- + ( + "list", + "Show every table/view under path, at any depth, as full dot-separated paths. " + "Raises CatalogOperationError if path doesn't exist.", + ), + ( + "schema", + "Show path's column schema and row count (never fetches the actual rows). path " + "must be a table or view — raises CatalogOperationError otherwise, or if path " + "doesn't exist at all.", + ), + ( + "delete_table", + "Delete the table at path. Not undoable. Raises CatalogOperationError if path " + "doesn't exist (unlike a bare DROP TABLE, this doesn't quietly no-op on a typo).", + ), + # -- view ----------------------------------------------------------------- + ( + "create_view", + "Create a VIEW at target_path over source_path, leaving source_path untouched. " + "overwrite=False (default) raises CatalogOperationError if target_path already " + "exists; overwrite=True replaces it and is not undoable. " + _CREATE_TARGET_FOLDER_NOTE, + ), + ( + "delete_view", + "Delete the view at path. Not undoable. Raises CatalogOperationError if path " + "doesn't exist (unlike a bare DROP VIEW, this doesn't quietly no-op on a typo).", + ), + # -- wiki ----------------------------------------------------------------- + ( + "set_wiki", + "Set a path's wiki text. tags: a list of {tag_name, tag_value, tag_title} dicts, " + "e.g. " + _META_TAGS_EXAMPLE + ".", + ), + ("delete_wiki", "Delete a path's wiki text."), + ( + "get_wiki", + "Show a path's wiki text. Raises CatalogOperationError if the path doesn't " + "exist, or exists but has no wiki at all.", + ), + # -- tags ----------------------------------------------------------------- + ( + "set_tags", + 'Replace a path\'s tag set. tags: a list of strings, e.g. ["reviewed"]. Tables/views ' + "only — raises CatalogOperationError otherwise.", + ), + ( + "delete_tags", + 'Remove tags from a path\'s tag set. tags: a list of strings, e.g. ["reviewed"]. ' + "Tables/views only — raises CatalogOperationError otherwise.", + ), + ( + "get_tags", + "Show the tags on a path — tables/views only. Raises CatalogOperationError if " + "the path doesn't exist or isn't a table/view.", + ), + # -- folders (not one of the five groups above) ---------------------------- + ( + "create_folder", + "Create a folder. create_parents=True creates every missing level of the full " + "path; create_parents=False (default) raises CatalogOperationError if the parent " + "folder doesn't already exist.", + ), + ( + "delete_folder", + "Delete a folder. Not undoable. cascade=True also deletes everything at path " + "and below; cascade=False (default) raises CatalogOperationError if the folder " + "isn't empty. A path that's already gone is not an error.", ), - ("copy", "Queue a copy. Reversible unless overwrite=True."), - ("move", "Queue a move. Reversible unless overwrite=True."), - ("tag", "Queue replacing a path's tag set."), - ("untag", "Queue removing tags from a path's tag set."), - ("set_wiki", "Queue setting a path's wiki text."), - ("delete_wiki", "Queue deleting a path's wiki text."), - ("set_meta", "Queue setting a folder's Meta Data wiki section."), - ("create_folder", "Queue creating a folder."), - ("delete_folder", "Queue deleting a folder. Never reversible."), - ("commit", "Run every queued step as one all-or-nothing batch."), ] _INGEST_HELP = [ ("ingest", "Queue one folder ingest (see FolderIngest for what each argument means)."), @@ -158,8 +283,84 @@ def _print_help(cls: type, methods: list[tuple[str, str]], label: str) -> None: print(f"%{label} help — show this message") +def _plain_params(func: Any) -> str: + """Comma-separated parameter names (minus `self`), for a non-developer + reading `%catalog help`'s table — no Python type-hint syntax (a bare + `str | None` union means nothing to a data custodian, and would collide + visually with the table's own `|` column separators anyway) and no `*` + keyword-only marker. A parameter with a default is shown as `name=default` + so it still reads as optional; everything else is required, in order.""" + sig = inspect.signature(func) + parts = [ + name if param.default is inspect.Parameter.empty else f"{name}={param.default!r}" + for name, param in sig.parameters.items() + if name != "self" + ] + return ", ".join(parts) + + +_CATALOG_HELP_HEADER_STYLE = ( + "text-align:left; padding:4px 12px; border-bottom:2px solid currentColor;" +) +_CATALOG_HELP_CELL_STYLE = ( + "text-align:left; padding:4px 12px; border-bottom:1px solid currentColor; vertical-align:top;" +) +_CATALOG_HELP_CODE_STYLE = _CATALOG_HELP_CELL_STYLE + " font-family:monospace; white-space:pre;" + + +def _catalog_help_html(rows: list[tuple[str, str, str]]) -> str: + """Build the `` markup for `%catalog help` — inline styles only + (no external stylesheet, no hardcoded background/text color — just + `currentColor` borders) so it reads correctly in both a light and a dark + notebook theme without knowing which one is active.""" + + def th(text: str) -> str: + return f'' + + def td(text: str, *, code: bool = False) -> str: + style = _CATALOG_HELP_CODE_STYLE if code else _CATALOG_HELP_CELL_STYLE + return f'' + + head = f"{th('Command')}{th('Parameters')}{th('Description')}" + body = "".join( + f"{td(name, code=True)}{td(params, code=True)}{td(description)}" + for name, params, description in rows + ) + return f'
{_escape(text)}{_escape(text)}
{head}{body}
' + + +def _print_catalog_help_table() -> None: + """`%catalog help` — a real HTML `` (Command / Parameters / + Description), rendered via `IPython.display` rather than `_print_help`'s + ASCII Python-signature listing: this magic's audience is a data + custodian reading it in JupyterLab, not necessarily someone comfortable + with a Python type signature or a monospace grid. Only this one magic's + help gets the rich-display treatment — everything else in this module + stays plain `print()` (see the module docstring's "eval() below runs + exactly the Python the user typed" — errors and usage lines are meant to + read like ordinary interpreter output, not a UI).""" + print(f"%catalog methods — usage: {_USAGE['catalog']}") + print( + "A path/source_path/target_path starting with '.' resolves against the current " + "context (see use); one or more leading '../' (or a bare '..') walks up that many " + "levels first. Ordinary use already keeps context current on its own." + ) + + rows = [ + (name, _plain_params(getattr(CatalogSession, name)), description) + for name, description in _CATALOG_HELP + ] + display(HTML(_catalog_help_html(rows))) + + def _dispatch( - session: Any, line: str, user_ns: dict[str, Any], label: str, error_type: type[Exception] + session: Any, + line: str, + user_ns: dict[str, Any], + label: str, + error_type: type[Exception], + *, + auto_commit: bool = False, ) -> Any: line = line.strip() if not line: @@ -169,10 +370,16 @@ def _dispatch( line = f"{line}()" namespace = {**user_ns, "__session__": session} try: - return eval(f"__session__.{line}", namespace) # see module docstring re: eval + result = eval(f"__session__.{line}", namespace) # see module docstring re: eval + if auto_commit and result is session: + # A queueing verb just chained back to `self` — commit it right + # away instead of waiting for a separate commit() call (see the + # module docstring: %catalog executes immediately). + result = session.commit(retry=True) except error_type as exc: print(f"{label} error: {exc}") return None + return result _CONTEXT_COMM_TARGET = "eea_datalakehouse.catalog_context" @@ -233,24 +440,75 @@ def _apply_context(self, path: str) -> None: return self._catalog_session.set_context(path) + def _ensure_catalog_session(self) -> bool: + """Build `self._catalog_session` if it doesn't exist yet, applying + any Comm-pushed context that arrived first. Returns `False` (after + printing a friendly error) if credentials aren't available — + `%catalog`/`%%catalog` both bail out at that point rather than + dispatching against no session.""" + if self._catalog_session is not None: + return True + try: + self._catalog_session = _build_catalog_session() + except RuntimeError as exc: + print(f"catalog error: {exc}") + return False + if self._pending_context is not None: + self._catalog_session.set_context(self._pending_context) + self._pending_context = None + return True + @line_magic def catalog(self, line: str) -> Any: if _is_help(line): - _print_help(CatalogSession, _CATALOG_HELP, "catalog") + _print_catalog_help_table() + return None + if not self._ensure_catalog_session(): return None - if self._catalog_session is None: - try: - self._catalog_session = _build_catalog_session() - except RuntimeError as exc: - print(f"catalog error: {exc}") - return None - if self._pending_context is not None: - self._catalog_session.set_context(self._pending_context) - self._pending_context = None return _dispatch( - self._catalog_session, line, self.shell.user_ns, "catalog", CatalogSessionError + self._catalog_session, + line, + self.shell.user_ns, + "catalog", + CatalogSessionError, + auto_commit=True, ) + @cell_magic("catalog") + def catalog_cell(self, line: str, cell: str) -> Any: + """`%%catalog` — one call per line, in order, typically opened with + `use(path)` to set the context once for the whole cell (see + `CatalogSession.use`) instead of repeating a path on every line:: + + %%catalog use("bwd.reference") + tag(".water_temperature", ["reviewed"]) + create_folder(".2027") + + Each line dispatches exactly like a `%catalog` line-magic call + (same immediate-execution, same friendly error printing) — the + magic line (`use("bwd.reference")` above) runs first, then every + non-blank line of the cell body, in order. Stops at the first line + that fails (its error has already been printed) rather than + running the rest against a context that isn't what the custodian + expected.""" + if not self._ensure_catalog_session(): + return None + result: Any = None + for statement in (line, *cell.splitlines()): + if not statement.strip(): + continue + result = _dispatch( + self._catalog_session, + statement, + self.shell.user_ns, + "catalog", + CatalogSessionError, + auto_commit=True, + ) + if result is None: + break # a friendly error was already printed by _dispatch + return result + @line_magic def ingest(self, line: str) -> Any: if _is_help(line): @@ -266,8 +524,9 @@ def ingest(self, line: str) -> Any: def load_ipython_extension(ipython: Any) -> None: # A no-op if `eea_datalakehouse.notebook`'s own auto-import already # registered this (see the module docstring) — registering again would - # replace the live instance with a fresh one, silently dropping any - # already-queued-but-not-committed CatalogSession/IngestSession state. + # replace the live instance with a fresh one, silently dropping the + # CatalogSession's "current path" context and any already-queued-but- + # not-committed IngestSession state. if "EEALakehouseMagics" in ipython.magics_manager.registry: return ipython.register_magics(EEALakehouseMagics) diff --git a/tests/catalog/conftest.py b/tests/catalog/conftest.py index 93f5db0..bbf1f59 100644 --- a/tests/catalog/conftest.py +++ b/tests/catalog/conftest.py @@ -87,6 +87,7 @@ def __init__( raise_on_set_tags: Exception | None = None, raise_on_create_folder: Exception | None = None, raise_on_delete_folder: Exception | None = None, + raise_on_exists: Exception | None = None, ) -> None: self.existing = set(existing or set()) self.folders = set(folders or set()) @@ -100,8 +101,11 @@ def __init__( self._raise_on_set_tags = raise_on_set_tags 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 def exists(self, path: str) -> bool: + if self._raise_on_exists is not None: + raise self._raise_on_exists return path in self.existing def is_folder(self, path: str) -> bool: diff --git a/tests/catalog/test_client.py b/tests/catalog/test_client.py index f2f6401..a71c14f 100644 --- a/tests/catalog/test_client.py +++ b/tests/catalog/test_client.py @@ -27,6 +27,16 @@ def test_injected_executor_is_used_directly() -> None: assert fake.statements == ['DROP VIEW IF EXISTS "a"."view"'] +def test_deletetable_delegates_correctly() -> None: + # deletetable, like deleteview, goes over the REST executor. + fake = FakeExecutor() + catalog = Catalog(BASE_URL, "pat", executor=fake) + + catalog.deletetable("a.table", idempotency_key="k") + + assert fake.statements == ['DROP TABLE IF EXISTS "a"."table"'] + + def test_without_injected_executor_builds_a_real_rest_executor( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -143,6 +153,23 @@ def test_datamove_delegates_correctly() -> None: ] +def test_createview_delegates_correctly() -> None: + # createview goes over the REST executor, not Flight — it creates no + # new data, just a saved query (same reasoning as table2view). + fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], []]) + catalog = Catalog(BASE_URL, "pat", executor=fake, catalog_rest=FakeCatalogRest()) + + catalog.createview("a.src", "a.dst", idempotency_key="k") + + assert fake.statements == [ + 'SELECT "TABLE_NAME" FROM INFORMATION_SCHEMA."TABLES" ' + "WHERE \"TABLE_SCHEMA\" = 'a' AND \"TABLE_NAME\" = 'src'", + 'SELECT "TABLE_NAME" FROM INFORMATION_SCHEMA."TABLES" ' + "WHERE \"TABLE_SCHEMA\" = 'a' AND \"TABLE_NAME\" = 'dst'", + 'CREATE VIEW "a"."dst" AS SELECT * FROM "a"."src"', + ] + + def test_gettablesfrom_delegates_and_returns_full_paths() -> None: fake = FakeExecutor(rows=[{"TABLE_SCHEMA": "bwd.versions", "TABLE_NAME": "assessments"}]) catalog = Catalog(BASE_URL, "pat", executor=fake) @@ -223,6 +250,41 @@ def test_retry_pending_routes_datamove_back_through_the_flight_executor() -> Non assert rest_fake.statements == [] +def test_retry_pending_routes_createview_back_through_the_rest_executor() -> None: + rest_fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], []]) + flight_fake = FakeExecutor() # must stay untouched — createview never uses Flight + catalog = Catalog( + BASE_URL, + "pat", + executor=rest_fake, + flight_executor=flight_fake, + catalog_rest=FakeCatalogRest(), + ) + retry_state.record( + "k", + "createview", + "a.dst", + "earlier failure", + params={ + "source_path": "a.src", + "target_path": "a.dst", + "overwrite": False, + "create_target_folder": False, + }, + ) + + catalog.retry_pending("k") + + assert rest_fake.statements == [ + 'SELECT "TABLE_NAME" FROM INFORMATION_SCHEMA."TABLES" ' + "WHERE \"TABLE_SCHEMA\" = 'a' AND \"TABLE_NAME\" = 'src'", + 'SELECT "TABLE_NAME" FROM INFORMATION_SCHEMA."TABLES" ' + "WHERE \"TABLE_SCHEMA\" = 'a' AND \"TABLE_NAME\" = 'dst'", + 'CREATE VIEW "a"."dst" AS SELECT * FROM "a"."src"', + ] + assert flight_fake.statements == [] + + def test_retry_pending_raises_when_nothing_is_pending() -> None: catalog = Catalog(BASE_URL, "pat") diff --git a/tests/catalog/test_operations.py b/tests/catalog/test_operations.py index 2274121..c16701f 100644 --- a/tests/catalog/test_operations.py +++ b/tests/catalog/test_operations.py @@ -468,6 +468,105 @@ def test_datamove_appends_source_name_when_target_is_an_existing_folder() -> Non ] +def test_createview_creates_when_target_does_not_exist() -> None: + fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], []]) + + operations.createview(fake, "a.src", "a.dst", idempotency_key="k") + + assert fake.statements == [ + _SOURCE_LOOKUP, + _TARGET_LOOKUP, + 'CREATE VIEW "a"."dst" AS SELECT * FROM "a"."src"', + ] + + +def test_createview_raises_when_target_already_exists_and_not_overwriting() -> None: + fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], [{"TABLE_NAME": "dst"}]]) + + with pytest.raises(CatalogOperationError, match="already exists"): + operations.createview(fake, "a.src", "a.dst", idempotency_key="k") + + # Fails fast — never gets to CREATE VIEW. + assert fake.statements == [_SOURCE_LOOKUP, _TARGET_LOOKUP] + + +def test_createview_overwrite_drops_existing_target_first() -> None: + # Target's own kind is detected (it might be a table, not a view, from + # an earlier different operation) rather than assumed. + fake = FakeExecutor( + rows_sequence=[ + [{"TABLE_NAME": "src"}], + [{"TABLE_NAME": "dst"}], + [{"TABLE_TYPE": "TABLE"}], + ] + ) + + operations.createview(fake, "a.src", "a.dst", overwrite=True, idempotency_key="k") + + assert fake.statements == [ + _SOURCE_LOOKUP, + _TARGET_LOOKUP, + _TARGET_KIND_LOOKUP, + 'DROP TABLE IF EXISTS "a"."dst"', + 'CREATE VIEW "a"."dst" AS SELECT * FROM "a"."src"', + ] + + +def test_createview_raises_when_source_does_not_exist() -> None: + fake = FakeExecutor(rows=[]) + + with pytest.raises(CatalogOperationError, match="does not exist"): + operations.createview(fake, "a.src", "a.dst", idempotency_key="k") + + # Fails fast — never gets to the target check or CREATE VIEW. + assert fake.statements == [_SOURCE_LOOKUP] + + +def test_createview_creates_missing_target_folder() -> None: + fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], []]) + catalog_rest = FakeCatalogRest(existing={"bwd"}) # "bwd.consumer" not yet there + + operations.createview( + fake, + "bwd.src", + "bwd.consumer.dst", + create_target_folder=True, + catalog_rest=catalog_rest, + idempotency_key="k", + ) + + assert catalog_rest.created == ["bwd.consumer"] + + +def test_createview_raises_when_folder_check_needed_but_no_catalog_rest_given() -> None: + fake = FakeExecutor(rows=[{"TABLE_NAME": "src"}]) + + with pytest.raises(CatalogOperationError, match="catalog_rest"): + operations.createview( + fake, "bwd.src", "bwd.consumer.dst", create_target_folder=True, idempotency_key="k" + ) + + +def test_createview_appends_source_name_when_target_is_an_existing_folder() -> None: + fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], []]) + catalog_rest = FakeCatalogRest(existing={"bwd", "bwd.folder"}, folders={"bwd.folder"}) + + operations.createview( + fake, "bwd.src", "bwd.folder", catalog_rest=catalog_rest, idempotency_key="k" + ) + + assert fake.statements[-1] == 'CREATE VIEW "bwd"."folder"."src" AS SELECT * FROM "bwd"."src"' + + +def test_createview_does_not_touch_the_source() -> None: + # The distinguishing feature vs datamove: no DROP statement anywhere. + fake = FakeExecutor(rows_sequence=[[{"TABLE_NAME": "src"}], []]) + + operations.createview(fake, "a.src", "a.dst", idempotency_key="k") + + assert not any(stmt.startswith("DROP") for stmt in fake.statements) + + def test_deleteview_drops_if_exists(executor) -> None: operations.deleteview(executor, "a.view", idempotency_key="k") @@ -482,6 +581,22 @@ def test_deleteview_is_registered_for_retry(executor) -> None: assert executor.statements == ['DROP VIEW IF EXISTS "a"."view"'] +def test_deletetable_drops_if_exists(executor) -> None: + operations.deletetable(executor, "a.table", idempotency_key="k") + + assert executor.statements == ['DROP TABLE IF EXISTS "a"."table"'] + + +def test_deletetable_is_registered_for_retry(executor) -> None: + retry_state.record( + "k", "deletetable", "a.table", "earlier failure", params={"table_path": "a.table"} + ) + + operations.retry_pending(executor, "k") + + assert executor.statements == ['DROP TABLE IF EXISTS "a"."table"'] + + def test_engine_starting_error_propagates_and_is_remembered(stalling_executor) -> None: with pytest.raises(EngineStartingError): operations.deleteview(stalling_executor, "a.view", idempotency_key="stall-key") diff --git a/tests/catalog/test_session.py b/tests/catalog/test_session.py index 56aeb7b..91c0649 100644 --- a/tests/catalog/test_session.py +++ b/tests/catalog/test_session.py @@ -27,7 +27,7 @@ def _isolated_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: def _catalog(rest: FakeCatalogRest, executor: FakeExecutor | None = None) -> Catalog: executor = executor or FakeExecutor() - # datacopy/datamove always go over flight_executor — share one FakeExecutor + # data_copy/data_move always go over flight_executor — share one FakeExecutor # so a test can reason about one single call log regardless of which verb # made the call. return Catalog(BASE_URL, "pat", executor=executor, flight_executor=executor, catalog_rest=rest) @@ -47,12 +47,12 @@ def test_commit_runs_queued_steps_in_order_and_clears_the_queue() -> None: rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) session = CatalogSession(_catalog(rest)) - session.create_folder("bwd.newfolder").tag("bwd.table1", ["reviewed"]) + session.create_folder("bwd.newfolder").set_tags("bwd.table1", ["reviewed"]) report = session.commit() assert report.succeeded == [ "create folder 'bwd.newfolder'", - "tag 'bwd.table1' with ['reviewed']", + "set tags ['reviewed'] on 'bwd.table1'", ] assert rest.created == ["bwd.newfolder"] assert rest.get_tags("bwd.table1") == ["reviewed"] @@ -67,7 +67,7 @@ def test_commit_rolls_back_a_cleanly_reversible_batch_on_failure() -> None: # step 1 succeeds and is reversible; step 2 fails because the target # already exists and overwrite defaults to False. session.create_folder("bwd.newfolder") - session.copy("bwd.table1", "bwd.table1") + session.data_copy("bwd.table1", "bwd.table1") with pytest.raises(CatalogCommitError) as exc_info: session.commit() @@ -75,20 +75,74 @@ def test_commit_rolls_back_a_cleanly_reversible_batch_on_failure() -> None: error = exc_info.value assert error.rolled_back is True assert error.unresolved == [] - assert error.failed_step == "copy 'bwd.table1' -> 'bwd.table1'" + assert error.failed_step == "data_copy 'bwd.table1' -> 'bwd.table1'" assert rest.created == ["bwd.newfolder"] assert rest.deleted == ["bwd.newfolder"] # step 1's undo ran assert repr(session) == "CatalogSession(pending=0)" # a failed commit still clears the queue +def test_data_move_rollback_moves_the_table_back() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) + # Six fetch_all calls in order: the forward move's source-exists, + # target-empty, and verify-target-exists checks, then the same three + # again for the undo's data_move back from table2 to table1. + executor = FakeExecutor( + rows_sequence=[ + [{"TABLE_NAME": "table1"}], + [], + [{"TABLE_NAME": "table2"}], + [{"TABLE_NAME": "table2"}], + [], + [{"TABLE_NAME": "table1"}], + ] + ) + session = CatalogSession(_catalog(rest, executor)) + + session.data_move("bwd.table1", "bwd.table2") # reversible + session.set_tags("bwd.missing", ["x"]) # fails: path does not exist + + with pytest.raises(CatalogCommitError) as exc_info: + session.commit() + + error = exc_info.value + assert error.rolled_back is True + assert error.failed_step == "set tags ['x'] on 'bwd.missing'" # step 2 is what actually failed + # step 1 (the move) succeeded, then got undone: a data_move back from + # table2 to table1, always as a TABLE — no re-detection needed, since + # move can no longer create a VIEW either. + assert 'CREATE TABLE "bwd"."table1" AS SELECT * FROM "bwd"."table2"' in executor.statements + assert 'DROP TABLE "bwd"."table2"' in executor.statements + + +def test_create_view_rollback_drops_the_view() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) + executor = FakeExecutor( + rows_sequence=[[{"TABLE_NAME": "table1"}], []], # source-exists, target-empty + rows=[{"TABLE_TYPE": "VIEW"}], # answers the undo's kind-detection lookup + ) + session = CatalogSession(_catalog(rest, executor)) + + session.create_view("bwd.table1", "bwd.view1") # reversible — source is untouched either way + session.set_tags("bwd.missing", ["x"]) # fails: path does not exist + + with pytest.raises(CatalogCommitError) as exc_info: + session.commit() + + error = exc_info.value + assert error.rolled_back is True + assert error.failed_step == "set tags ['x'] on 'bwd.missing'" # step 2 is what actually failed + assert 'DROP VIEW IF EXISTS "bwd"."view1"' in executor.statements # undo: drop what it created + assert "bwd.table1" in rest.existing # source was never touched + + def test_commit_reports_what_it_could_not_undo() -> None: rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) executor = FakeExecutor(rows=[{"TABLE_NAME": "table1", "TABLE_TYPE": "TABLE"}]) session = CatalogSession(_catalog(rest, executor)) session.create_folder("bwd.newfolder") # reversible - session.copy("bwd.table1", "bwd.table1", overwrite=True) # succeeds, but NOT reversible - session.tag("bwd.missing", ["x"]) # fails: path does not exist + session.data_copy("bwd.table1", "bwd.table1", overwrite=True) # succeeds, but NOT reversible + session.set_tags("bwd.missing", ["x"]) # fails: path does not exist with pytest.raises(CatalogCommitError) as exc_info: session.commit() diff --git a/tests/catalog/test_session_context.py b/tests/catalog/test_session_context.py index 154d652..146ed5a 100644 --- a/tests/catalog/test_session_context.py +++ b/tests/catalog/test_session_context.py @@ -36,7 +36,7 @@ def test_relative_path_without_any_context_raises() -> None: session = CatalogSession(_catalog(rest)) with pytest.raises(CatalogSessionError, match="no context is set"): - session.tag(".table1", ["reviewed"]) + session.set_tags(".table1", ["reviewed"]) def test_touching_a_leaf_infers_context_for_a_later_relative_call() -> None: @@ -45,14 +45,14 @@ def test_touching_a_leaf_infers_context_for_a_later_relative_call() -> None: ) session = CatalogSession(_catalog(rest)) - session.tag("bwd.reference.water_temperature", ["reviewed"]) # sets context to bwd.reference - session.tag(".stations", ["reviewed"]) # resolves to bwd.reference.stations + session.set_tags("bwd.reference.water_temperature", ["reviewed"]) # -> bwd.reference + session.set_tags(".stations", ["reviewed"]) # resolves to bwd.reference.stations report = session.commit() assert report.succeeded == [ - "tag 'bwd.reference.water_temperature' with ['reviewed']", - "tag 'bwd.reference.stations' with ['reviewed']", + "set tags ['reviewed'] on 'bwd.reference.water_temperature'", + "set tags ['reviewed'] on 'bwd.reference.stations'", ] assert rest.get_tags("bwd.reference.stations") == ["reviewed"] @@ -62,11 +62,114 @@ def test_set_context_seeds_it_explicitly_before_any_path_is_used() -> None: session = CatalogSession(_catalog(rest)) session.set_context("bwd.reference") - session.tag(".water_temperature", ["reviewed"]) + session.set_tags(".water_temperature", ["reviewed"]) report = session.commit() - assert report.succeeded == ["tag 'bwd.reference.water_temperature' with ['reviewed']"] + assert report.succeeded == ["set tags ['reviewed'] on 'bwd.reference.water_temperature'"] + + +def test_use_with_a_whole_path_sets_context_like_set_context() -> None: + rest = FakeCatalogRest( + existing={"a", "bwd", "bwd.reference", "bwd.reference.water_temperature"} + ) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference") + session.set_tags(".water_temperature", ["reviewed"]) + + report = session.commit() + + assert report.succeeded == ["set tags ['reviewed'] on 'bwd.reference.water_temperature'"] + + +def test_use_with_a_relative_path_resolves_against_the_existing_context() -> None: + rest = FakeCatalogRest( + existing={ + "a", + "bwd", + "bwd.reference", + "bwd.reference.2027", + "bwd.reference.2027.water_temperature", + } + ) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference") + session.use(".2027") # narrows bwd.reference -> bwd.reference.2027 + session.set_tags(".water_temperature", ["reviewed"]) + + report = session.commit() + + assert report.succeeded == ["set tags ['reviewed'] on 'bwd.reference.2027.water_temperature'"] + + +def test_use_with_a_relative_path_and_no_context_yet_raises() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogSessionError, match="no context is set"): + session.use(".reference") + + +def test_use_bare_path_resolves_relative_once_context_exists() -> None: + # No leading '.' needed once inside a context — "2027" and ".2027" mean + # the same thing here (unlike every other verb's path arguments, which + # always require the dot to mean relative). + rest = FakeCatalogRest( + existing={ + "a", + "bwd", + "bwd.reference", + "bwd.reference.2027", + "bwd.reference.2027.water_temperature", + } + ) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference") + session.use("2027") # bare, no dot — still narrows to bwd.reference.2027 + session.set_tags(".water_temperature", ["reviewed"]) + + report = session.commit() + + assert report.succeeded == ["set tags ['reviewed'] on 'bwd.reference.2027.water_temperature'"] + + +def test_use_bare_path_with_no_context_yet_is_absolute() -> None: + # The very first use() has nothing to be relative to, so a bare path is + # unambiguous: it must be the whole path. + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd") + + assert session.get_context() == "bwd" + + +def test_use_none_clears_context_like_set_context() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference", "bwd.other"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference") + session.use(None) + + assert session.get_context() is None + # Cleared, so a bare path is absolute again rather than relative to + # whatever used to be there. + session.use("bwd.other") + assert session.get_context() == "bwd.other" + + +def test_get_context_reflects_state_set_by_other_verbs() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference.water_temperature"}) + session = CatalogSession(_catalog(rest)) + + assert session.get_context() is None + + session.set_tags("bwd.reference.water_temperature", ["reviewed"]) # auto-updates context + + assert session.get_context() == "bwd.reference" def test_create_folder_context_becomes_the_folder_itself_not_its_parent() -> None: @@ -75,17 +178,41 @@ def test_create_folder_context_becomes_the_folder_itself_not_its_parent() -> Non session.create_folder("bwd.reference.2027", create_parents=True) # context -> that folder session.set_context("bwd.reference") # re-seed for the rest of this test - session.tag(".water_temperature", ["reviewed"]) + session.set_tags(".water_temperature", ["reviewed"]) report = session.commit() assert report.succeeded == [ "create folder 'bwd.reference.2027'", - "tag 'bwd.reference.water_temperature' with ['reviewed']", + "set tags ['reviewed'] on 'bwd.reference.water_temperature'", + ] + + +def test_data_copy_resolves_both_paths_against_the_same_starting_context() -> None: + rest = FakeCatalogRest( + existing={"a", "bwd", "bwd.draft.raw_2026", "bwd.reference", "bwd.reference.stations"} + ) + executor = FakeExecutor( + rows_sequence=[ + [{"TABLE_NAME": "raw_2026"}], # source exists + [], # target does not exist yet + ] + ) + session = CatalogSession(_catalog(rest, executor)) + + session.set_context("bwd.draft") + session.data_copy(".raw_2026", "bwd.reference.water_temperature") # target NOT under bwd.draft + session.set_tags(".stations", ["reviewed"]) # resolves against the target's parent + + report = session.commit() + + assert report.succeeded == [ + "data_copy 'bwd.draft.raw_2026' -> 'bwd.reference.water_temperature'", + "set tags ['reviewed'] on 'bwd.reference.stations'", ] -def test_copy_resolves_both_paths_against_the_same_starting_context() -> None: +def test_data_move_resolves_both_paths_against_the_same_starting_context() -> None: rest = FakeCatalogRest( existing={"a", "bwd", "bwd.draft.raw_2026", "bwd.reference", "bwd.reference.stations"} ) @@ -93,17 +220,184 @@ def test_copy_resolves_both_paths_against_the_same_starting_context() -> None: rows_sequence=[ [{"TABLE_NAME": "raw_2026"}], # source exists [], # target does not exist yet + [{"TABLE_NAME": "water_temperature"}], # verify target exists after the move ] ) session = CatalogSession(_catalog(rest, executor)) session.set_context("bwd.draft") - session.copy(".raw_2026", "bwd.reference.water_temperature") # target does NOT use bwd.draft - session.tag(".stations", ["reviewed"]) # resolves against bwd.reference (the target's parent) + session.data_move(".raw_2026", "bwd.reference.water_temperature") # target NOT under bwd.draft + session.set_tags(".stations", ["reviewed"]) # resolves against the target's parent report = session.commit() assert report.succeeded == [ - "copy 'bwd.draft.raw_2026' -> 'bwd.reference.water_temperature'", - "tag 'bwd.reference.stations' with ['reviewed']", + "data_move 'bwd.draft.raw_2026' -> 'bwd.reference.water_temperature'", + "set tags ['reviewed'] on 'bwd.reference.stations'", ] + + +def test_create_view_resolves_both_paths_against_the_same_starting_context() -> None: + rest = FakeCatalogRest( + existing={"a", "bwd", "bwd.draft.raw_2026", "bwd.reference", "bwd.reference.stations"} + ) + executor = FakeExecutor( + rows_sequence=[ + [{"TABLE_NAME": "raw_2026"}], # source exists + [], # target does not exist yet + ] + ) + session = CatalogSession(_catalog(rest, executor)) + + session.set_context("bwd.draft") + session.create_view(".raw_2026", "bwd.reference.water_temperature") # target NOT bwd.draft + session.set_tags(".stations", ["reviewed"]) # resolves against the target's parent + + report = session.commit() + + assert report.succeeded == [ + "create view 'bwd.draft.raw_2026' -> 'bwd.reference.water_temperature'", + "set tags ['reviewed'] on 'bwd.reference.stations'", + ] + + +def test_bare_dotdot_resolves_to_the_parent_of_the_context() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference", "bwd.reference.2027"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference.2027") + session.use("..") + + assert session.get_context() == "bwd.reference" + + +def test_dotdot_slash_name_resolves_to_a_sibling_of_the_context() -> None: + rest = FakeCatalogRest( + existing={"a", "bwd", "bwd.reference.2027.stations", "bwd.reference.2027.water_temp"} + ) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference.2027.stations") + session.use("../water_temp") # sibling of the current context + + assert session.get_context() == "bwd.reference.2027.water_temp" + + +def test_chained_dotdot_walks_up_multiple_levels() -> None: + rest = FakeCatalogRest( + existing={"a", "bwd", "bwd.reference", "bwd.reference.2027.stations", "bwd.archive"} + ) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference.2027.stations") + session.use("../../../archive") # up three levels, then into "archive" + + assert session.get_context() == "bwd.archive" + + # A bare absolute path is only absolute again once the context is + # cleared — see use()'s own docstring on this tradeoff. + session.use(None) + session.use("bwd.reference.2027.stations") + session.use("../..") # up two levels, no name after it + + assert session.get_context() == "bwd.reference" + + +def test_dotdot_past_the_top_of_the_context_raises() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd") + + with pytest.raises(CatalogSessionError, match="goes above the top"): + session.use("..") + + +def test_dotdot_with_no_context_yet_raises() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogSessionError, match="no context is set"): + session.use("../reference") + + +def test_dotdot_works_for_verbs_other_than_use_too() -> None: + # ../ is a general path-resolution feature (_resolve_path), not + # something special-cased inside use() alone. + rest = FakeCatalogRest( + existing={ + "a", + "bwd", + "bwd.reference", + "bwd.reference.2027", + "bwd.reference.water_temperature", + } + ) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference.2027") + session.set_tags("../water_temperature", ["reviewed"]) # ../ from a plain verb's path arg + + report = session.commit() + + assert report.succeeded == ["set tags ['reviewed'] on 'bwd.reference.water_temperature'"] + + +def test_get_context_after_dotdot_is_the_full_resolved_path() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference", "bwd.reference.2027"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference.2027") + session.use("..") + + context = session.get_context() + assert context == "bwd.reference" + assert not context.startswith(".") # never a raw relative fragment + + +def test_use_raises_when_the_resolved_path_does_not_exist() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogSessionError, match="does not exist in the catalog"): + session.use("bwd.nonexistent") + + +def test_use_leaves_context_unchanged_after_a_failed_check() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference") + with pytest.raises(CatalogSessionError, match="does not exist"): + session.use(".nonexistent") + + assert session.get_context() == "bwd.reference" # still the last good value + + +def test_use_accepts_a_folder_not_just_a_table_or_view() -> None: + # exists() (CatalogRestClient) sees any entity type — unlike the + # INFORMATION_SCHEMA-based checks other verbs use, which only see + # tables/views. use() should work against a bare folder. + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference"}, folders={"bwd.reference"}) + session = CatalogSession(_catalog(rest)) + + session.use("bwd.reference") + + assert session.get_context() == "bwd.reference" + + +def test_use_wraps_a_stalled_existence_check_as_a_catalog_session_error() -> None: + from eea_datalakehouse.catalog.errors import EngineStartingError + + rest = FakeCatalogRest( + existing={"a", "bwd"}, + raise_on_exists=EngineStartingError("engine starting", idempotency_key="k"), + ) + session = CatalogSession(_catalog(rest)) + + # Not a raw EngineStartingError escaping — %catalog only catches + # CatalogSessionError (see notebook/magics.py's _dispatch), so this must + # come back wrapped or it would surface as an ugly traceback instead of + # a friendly printed message. + with pytest.raises(CatalogSessionError, match="could not check whether"): + session.use("bwd.reference") diff --git a/tests/catalog/test_session_queries.py b/tests/catalog/test_session_queries.py new file mode 100644 index 0000000..f062898 --- /dev/null +++ b/tests/catalog/test_session_queries.py @@ -0,0 +1,257 @@ +"""CatalogSession's read-only queries (get_wiki, get_tags, list, schema) and +delete verbs (delete_view, delete_table). + +See src/eea_datalakehouse/catalog/session.py — the read-only queries are +answered immediately (never queued); delete_view/delete_table are queued +like every other mutating verb, but — unlike the idempotent +`Catalog.deleteview`/`deletetable` they wrap — require `path` to already +exist. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from eea_datalakehouse.catalog import retry_state +from eea_datalakehouse.catalog.client import Catalog +from eea_datalakehouse.catalog.errors import CatalogOperationError +from eea_datalakehouse.catalog.operations import TableInfo +from eea_datalakehouse.catalog.session import CatalogCommitError, CatalogSession + +from .conftest import FakeCatalogRest, FakeExecutor + +BASE_URL = "https://dremio.example.test" + + +@pytest.fixture(autouse=True) +def _isolated_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(retry_state, "DEFAULT_STATE_PATH", tmp_path / "state.json") + + +def _catalog(rest: FakeCatalogRest, executor: FakeExecutor | None = None) -> Catalog: + executor = executor or FakeExecutor() + return Catalog(BASE_URL, "pat", executor=executor, flight_executor=executor, catalog_rest=rest) + + +# -- get_wiki ----------------------------------------------------------------- + + +def test_get_wiki_returns_the_wiki_text() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}, wikis={"bwd.table1": "hello"}) + session = CatalogSession(_catalog(rest)) + + assert session.get_wiki("bwd.table1") == "hello" + + +def test_get_wiki_resolves_a_relative_path() -> None: + rest = FakeCatalogRest( + existing={"a", "bwd", "bwd.reference", "bwd.reference.table1"}, + wikis={"bwd.reference.table1": "hi"}, + ) + session = CatalogSession(_catalog(rest)) + session.use("bwd.reference") + + assert session.get_wiki(".table1") == "hi" + + +def test_get_wiki_raises_when_path_does_not_exist() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogOperationError, match="does not exist"): + session.get_wiki("bwd.missing") + + +def test_get_wiki_does_not_queue_anything() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}, wikis={"bwd.table1": "hi"}) + session = CatalogSession(_catalog(rest)) + + session.get_wiki("bwd.table1") + + assert repr(session) == "CatalogSession(pending=0)" + + +# -- get_tags ------------------------------------------------------------------- + + +def test_get_tags_returns_the_tags() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}, tags={"bwd.table1": ["reviewed"]}) + session = CatalogSession(_catalog(rest)) + + assert session.get_tags("bwd.table1") == ["reviewed"] + + +def test_get_tags_raises_when_path_does_not_exist() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogOperationError, match="does not exist"): + session.get_tags("bwd.missing") + + +def test_get_tags_raises_on_a_folder() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.folder1"}, folders={"bwd.folder1"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogOperationError, match="only works on tables/views"): + session.get_tags("bwd.folder1") + + +# -- list ----------------------------------------------------------------------- + + +def test_list_returns_full_paths() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference"}) + executor = FakeExecutor( + rows=[{"TABLE_SCHEMA": "bwd.reference", "TABLE_NAME": "water_temperature"}] + ) + session = CatalogSession(_catalog(rest, executor)) + + assert session.list("bwd.reference") == ["bwd.reference.water_temperature"] + + +def test_list_resolves_a_relative_path() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference"}) + executor = FakeExecutor(rows=[]) + session = CatalogSession(_catalog(rest, executor)) + session.use("bwd") + + session.list(".reference") # must not raise — "bwd.reference" exists + + assert executor.statements # the gettablesfrom query actually ran + + +def test_list_raises_when_path_does_not_exist() -> None: + # Unlike the raw gettablesfrom (which would just return []), list() + # checks first rather than treating a typo as "nothing found". + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogOperationError, match="does not exist"): + session.list("bwd.missing") + + +# -- schema --------------------------------------------------------------------- + + +def test_schema_returns_table_info() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) + executor = FakeExecutor( + rows_sequence=[[{"COLUMN_NAME": "id", "DATA_TYPE": "INTEGER"}], [{"row_count": 5}]] + ) + session = CatalogSession(_catalog(rest, executor)) + + assert session.schema("bwd.table1") == TableInfo(schema={"id": "INTEGER"}, row_count=5) + + +def test_schema_raises_on_a_folder() -> None: + # The explicit requirement: schema() only works on a table or view. + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.folder1"}, folders={"bwd.folder1"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogOperationError, match="only works on tables/views"): + session.schema("bwd.folder1") + + +def test_schema_raises_when_path_does_not_exist() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + + with pytest.raises(CatalogOperationError, match="does not exist"): + session.schema("bwd.missing") + + +# -- delete_view ------------------------------------------------------------------- + + +def test_delete_view_drops_the_view() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.view1"}) + executor = FakeExecutor() + session = CatalogSession(_catalog(rest, executor)) + + session.delete_view("bwd.view1") + report = session.commit() + + assert report.succeeded == ["delete view 'bwd.view1'"] + assert executor.statements == ['DROP VIEW IF EXISTS "bwd"."view1"'] + + +def test_delete_view_resolves_a_relative_path() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.reference", "bwd.reference.view1"}) + executor = FakeExecutor() + session = CatalogSession(_catalog(rest, executor)) + session.use("bwd.reference") + + session.delete_view(".view1") + report = session.commit() + + assert report.succeeded == ["delete view 'bwd.reference.view1'"] + + +def test_delete_view_raises_when_path_does_not_exist() -> None: + # Unlike the raw Catalog.deleteview (DROP VIEW IF EXISTS — a no-op on a + # missing path), CatalogSession.delete_view requires it to be real. + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + session.delete_view("bwd.missing") + + with pytest.raises(CatalogCommitError, match="does not exist"): + session.commit() + + +def test_delete_view_is_never_reversible() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.view1"}) + session = CatalogSession(_catalog(rest)) + + session.delete_view("bwd.view1") # succeeds, but not reversible + session.set_tags("bwd.missing", ["x"]) # fails: path does not exist + + with pytest.raises(CatalogCommitError) as exc_info: + session.commit() + + error = exc_info.value + assert error.rolled_back is False + assert len(error.unresolved) == 1 + assert "cannot be recreated" in error.unresolved[0] + + +# -- delete_table ------------------------------------------------------------------ + + +def test_delete_table_drops_the_table() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) + executor = FakeExecutor() + session = CatalogSession(_catalog(rest, executor)) + + session.delete_table("bwd.table1") + report = session.commit() + + assert report.succeeded == ["delete table 'bwd.table1'"] + assert executor.statements == ['DROP TABLE IF EXISTS "bwd"."table1"'] + + +def test_delete_table_raises_when_path_does_not_exist() -> None: + rest = FakeCatalogRest(existing={"a", "bwd"}) + session = CatalogSession(_catalog(rest)) + session.delete_table("bwd.missing") + + with pytest.raises(CatalogCommitError, match="does not exist"): + session.commit() + + +def test_delete_table_is_never_reversible() -> None: + rest = FakeCatalogRest(existing={"a", "bwd", "bwd.table1"}) + session = CatalogSession(_catalog(rest)) + + session.delete_table("bwd.table1") # succeeds, but not reversible + session.set_tags("bwd.missing", ["x"]) # fails: path does not exist + + with pytest.raises(CatalogCommitError) as exc_info: + session.commit() + + error = exc_info.value + assert error.rolled_back is False + assert len(error.unresolved) == 1 + assert "cannot be recreated" in error.unresolved[0] diff --git a/tests/notebook/test_magics.py b/tests/notebook/test_magics.py index c8577b0..70a77f3 100644 --- a/tests/notebook/test_magics.py +++ b/tests/notebook/test_magics.py @@ -24,19 +24,29 @@ class _FakeCatalogSession: def __init__(self) -> None: self.calls: list[str] = [] self.committed = False + self.commit_kwargs: dict[str, Any] | None = None - def copy(self, *args: Any, **kwargs: Any) -> _FakeCatalogSession: - self.calls.append(f"copy{args!r}{kwargs!r}") + def data_copy(self, *args: Any, **kwargs: Any) -> _FakeCatalogSession: + self.calls.append(f"data_copy{args!r}{kwargs!r}") + return self + + def use(self, path: str) -> _FakeCatalogSession: + self.calls.append(f"use({path!r})") + return self + + def set_tags(self, *args: Any, **kwargs: Any) -> _FakeCatalogSession: + self.calls.append(f"set_tags{args!r}{kwargs!r}") return self def commit(self, **kwargs: Any) -> str: self.committed = True + self.commit_kwargs = kwargs return "committed" def raise_commit_error(self) -> None: raise CatalogCommitError( "boom", - failed_step="copy", + failed_step="data_copy", original_error=RuntimeError("x"), rolled_back=True, unresolved=[], @@ -60,6 +70,23 @@ def _magics_instance(ip: Any) -> EEALakehouseMagics: return ip.magics_manager.registry["EEALakehouseMagics"] # type: ignore[no-any-return] +def test_catalog_magic_executes_and_commits_immediately( + ip: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + fake = _FakeCatalogSession() + build_calls = [] + monkeypatch.setattr( + magics_module, "_build_catalog_session", lambda: (build_calls.append(1), fake)[1] + ) + + ip.run_line_magic("catalog", 'data_copy("a.b", "c.d", overwrite=True)') # no separate commit + + assert build_calls == [1] + assert fake.calls == ["data_copy('a.b', 'c.d'){'overwrite': True}"] + assert fake.committed is True + assert fake.commit_kwargs == {"retry": True} + + def test_catalog_magic_builds_the_session_once_and_reuses_it( ip: Any, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -69,15 +96,19 @@ def test_catalog_magic_builds_the_session_once_and_reuses_it( magics_module, "_build_catalog_session", lambda: (build_calls.append(1), fake)[1] ) - ip.run_line_magic("catalog", 'copy("a.b", "c.d", overwrite=True)') - ip.run_line_magic("catalog", "commit") + ip.run_line_magic("catalog", 'data_copy("a.b", "c.d")') + ip.run_line_magic("catalog", 'data_copy("e.f", "g.h")') assert build_calls == [1] # only built once, reused across calls - assert fake.calls == ["copy('a.b', 'c.d'){'overwrite': True}"] - assert fake.committed is True + assert fake.calls == ["data_copy('a.b', 'c.d'){}", "data_copy('e.f', 'g.h'){}"] -def test_bare_commit_without_parens_is_accepted(ip: Any, monkeypatch: pytest.MonkeyPatch) -> None: +def test_bare_commit_is_still_accepted_as_a_harmless_noop( + ip: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + # `commit` is no longer part of %catalog's primary vocabulary (see + # _CATALOG_HELP), but old notebooks/muscle memory calling it explicitly + # should still work rather than break. fake = _FakeCatalogSession() monkeypatch.setattr(magics_module, "_build_catalog_session", lambda: fake) @@ -108,7 +139,54 @@ def test_missing_credentials_prints_a_friendly_message( # a session cached on a *different* Magics instance — this test's `ip` # fixture registers its own. - ip.run_line_magic("catalog", 'copy("a.b", "c.d")') + ip.run_line_magic("catalog", 'data_copy("a.b", "c.d")') + + out = capsys.readouterr().out + assert "catalog error:" in out + assert "DREMIO_BASE_URL" in out + + +def test_catalog_cell_magic_runs_the_magic_line_then_each_cell_line_in_order( + ip: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + fake = _FakeCatalogSession() + monkeypatch.setattr(magics_module, "_build_catalog_session", lambda: fake) + + ip.run_cell_magic( + "catalog", + 'use("bwd.reference")', + 'set_tags(".water_temperature", ["reviewed"])\ndata_copy(".water_temperature", ".archive")', + ) + + assert fake.calls == [ + "use('bwd.reference')", + "set_tags('.water_temperature', ['reviewed']){}", + "data_copy('.water_temperature', '.archive'){}", + ] + assert fake.committed is True # auto-committed after each call, same as the line magic + + +def test_catalog_cell_magic_stops_at_the_first_failing_line( + ip: Any, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + fake = _FakeCatalogSession() + monkeypatch.setattr(magics_module, "_build_catalog_session", lambda: fake) + + ip.run_cell_magic("catalog", "", 'raise_commit_error()\nset_tags(".x", ["y"])') + + out = capsys.readouterr().out + assert "catalog error:" in out + assert "boom" in out + assert fake.calls == [] # the second line never ran + + +def test_catalog_cell_magic_missing_credentials_prints_a_friendly_message( + ip: Any, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.delenv("DREMIO_BASE_URL", raising=False) + monkeypatch.delenv("DREMIO_TOKEN", raising=False) + + ip.run_cell_magic("catalog", 'use("bwd.reference")', 'set_tags(".x", ["y"])') out = capsys.readouterr().out assert "catalog error:" in out @@ -142,14 +220,86 @@ def test_catalog_help_lists_methods_without_needing_credentials( ) -> None: monkeypatch.delenv("DREMIO_BASE_URL", raising=False) monkeypatch.delenv("DREMIO_TOKEN", raising=False) + # The table itself is rendered via IPython.display, not print() — capture + # what gets displayed rather than relying on capsys for it. + displayed = [] + monkeypatch.setattr(magics_module, "display", displayed.append) ip.run_line_magic("catalog", line) out = capsys.readouterr().out assert "%catalog methods" in out - assert "copy(source_path: str, target_path: str" in out - assert "commit(*, retry: bool = False" in out assert "DREMIO_BASE_URL" not in out # never tried to build a session + # General note about leading-'.'/'../' relative paths — printed once, + # not per-row, since it applies across every path/source_path/target_path. + assert "resolves against the current context" in out + assert "see use" in out + assert "walks up that many" in out # ../ support, mentioned generally + + assert len(displayed) == 1 + table_html = displayed[0].data + # A plain table — command / parameters / description — not raw Python + # signatures, so it reads for a non-developer. + assert "Command" in table_html and "Parameters" in table_html and "Description" in table_html + assert "data_copy" in table_html and "source_path, target_path, overwrite=False" in table_html + assert "str | None" not in table_html # no Python type-hint syntax leaking into the table + assert "list[str]" not in table_html + assert ">commit<" not in table_html # no "commit" row — not primary vocabulary anymore + assert ">tag<" not in table_html and ">untag<" not in table_html # renamed + assert ">set_meta<" not in table_html # removed entirely + # set_tags/delete_tags/set_wiki spell out their `tags` parameter's format + # (quotes come back html-escaped, e.g. ", hence the split assertions). + assert "tags: a list of strings, e.g." in table_html + # set_wiki is the only remaining method using the {tag_name, tag_value, + # tag_title} dict example, now that set_meta is gone. + assert table_html.count("tags: a list of {tag_name, tag_value, tag_title} dicts") == 1 + assert table_html.count("bw-team") == 1 + # data_copy/data_move/create_view call out the CatalogOperationError overwrite=False can raise. + assert table_html.count("overwrite=False (default) raises CatalogOperationError") == 3 + # create_view: a real command now, alongside data_move (which lost + # entry_type — it always moves as a TABLE; create_view covers the VIEW case). + assert "create_view" in table_html + assert "leaving source_path untouched" in table_html + assert "always as a TABLE" in table_html + assert "entry_type" not in table_html # no longer part of data_move's vocabulary + # data_copy/data_move/create_view share the same create_target_folder explanation. + assert table_html.count("creates every missing folder level of target_path") == 3 + assert table_html.count("the space/source itself, which must already exist") == 3 + # set_tags/delete_tags call out the specific error they can raise. + assert "Tables/views only — raises CatalogOperationError otherwise." in table_html + # use/get_context are listed; set_context is deliberately not (use covers it). + assert "with or without a leading" in table_html # use's dot-optional relative paths + assert "walks up one level" in table_html # use's ../ support + # use's live existence check (apostrophe in "doesn't" comes back escaped). + assert "Raises if the resolved path" in table_html + assert "exist in the catalog" in table_html + assert "get_context" in table_html + assert "Show the current path" in table_html + assert ">set_context<" not in table_html + # create_folder spells out create_parents' two behaviors (apostrophe in + # "doesn't" comes back html-escaped, hence stopping the check before it). + assert "create_parents=True creates every missing level" in table_html + assert ( + "create_parents=False (default) raises CatalogOperationError if the parent " + "folder" in table_html + ) + # delete_folder: cascade behavior, the not-empty error, and that a + # missing path is idempotent rather than an error (apostrophe in + # "isn't"/"that's" comes back html-escaped, hence the split checks). + assert "cascade=True also deletes everything at path and below" in table_html + assert "cascade=False (default) raises CatalogOperationError if the folder" in table_html + assert "already gone is not an error" in table_html + # New read-only queries and their delete counterparts. + assert "get_wiki" in table_html and "get_tags" in table_html + assert "delete_view" in table_html and "delete_table" in table_html + assert "deleteview" not in table_html and "deletetable" not in table_html # renamed + assert ">list<" in table_html and ">schema<" in table_html + assert "must be a table or view" in table_html # schema's type requirement + # list/delete_view/delete_table each spell out the existence error the + # same way (apostrophe in "doesn't" comes back html-escaped, hence + # stopping before it). + assert table_html.count("Raises CatalogOperationError if path") == 3 + assert "exists but has no wiki at all" in table_html # get_wiki's own wording assert _magics_instance(ip)._catalog_session is None From 1e5ec974629547cb76ce1f425dd0c41a33c05059 Mon Sep 17 00:00:00 2001 From: oskaresparza Date: Thu, 17 Sep 2026 14:28:56 +0200 Subject: [PATCH 2/2] Render %ingest help as the same HTML table %catalog help uses Both magics now share one _print_help_table/_help_table_html implementation instead of %ingest help printing a plain ASCII Python-signature listing. Also fixes a couple of stale leftovers from earlier renames: a tag(...) example in %%catalog's docstring and the ingest example notebook's description of help's old output format. Co-Authored-By: Claude Sonnet 5 --- README.md | 7 +- docs/notebooks/ingest_session_example.ipynb | 2 +- src/eea_datalakehouse/notebook/magics.py | 116 +++++++------------- tests/notebook/test_magics.py | 19 +++- 4 files changed, 60 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index 40daa3c..184c514 100644 --- a/README.md +++ b/README.md @@ -196,11 +196,10 @@ import eea_datalakehouse.notebook # registers %catalog/%ingest — no %load_ext %ingest commit(retry=True) ``` -`%catalog help` (or `%catalog help()`) prints every command as a plain table — name, +`%catalog help` (or `%catalog help()`) renders every command as an HTML table — name, parameters, description — rather than a raw Python signature, since its audience is a data -custodian, not necessarily a developer. `%ingest help` does the same for `IngestSession`, -listing its (fewer) methods with their real signatures — handy when you don't remember an -exact parameter name mid-notebook. +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. `%%catalog` (the cell-magic form) sets the context once, with `use(path)` on its magic line, then runs every other line of the cell in order under that context, without repeating the full diff --git a/docs/notebooks/ingest_session_example.ipynb b/docs/notebooks/ingest_session_example.ipynb index 1a98729..dc98794 100644 --- a/docs/notebooks/ingest_session_example.ipynb +++ b/docs/notebooks/ingest_session_example.ipynb @@ -37,7 +37,7 @@ { "cell_type": "markdown", "id": "53bc1b4a", - "source": "## Quick reference\n\n`%ingest help` (or `%ingest help()`) lists every `IngestSession` method with its signature\nand a short description — handy when you don't remember an exact parameter name\nmid-notebook.", + "source": "## Quick reference\n\n`%ingest help` (or `%ingest help()`) renders every `IngestSession` method as an HTML table —\nname, parameters, description — the same format `%catalog help` uses. Handy when you don't\nremember an exact parameter name mid-notebook.", "metadata": {} }, { diff --git a/src/eea_datalakehouse/notebook/magics.py b/src/eea_datalakehouse/notebook/magics.py index 06fdb9d..5f24015 100644 --- a/src/eea_datalakehouse/notebook/magics.py +++ b/src/eea_datalakehouse/notebook/magics.py @@ -242,50 +242,9 @@ def _is_help(line: str) -> bool: return line.strip() in ("help", "help()") -def _format_signature(func: Any) -> str: - """Render `func`'s signature (minus `self`) the way it reads in source — - `inspect.Signature`'s own `str()` wraps string annotations in quotes - (they're plain `str`s at runtime because of this module's, and the - session modules', `from __future__ import annotations`), which is - accurate but noisy for a notebook help message.""" - sig = inspect.signature(func) - parts = [] - seen_star = False - for name, param in sig.parameters.items(): - if name == "self": - continue - if param.kind is inspect.Parameter.KEYWORD_ONLY and not seen_star: - parts.append("*") - seen_star = True - piece = name - if param.annotation is not inspect.Parameter.empty: - piece += f": {param.annotation}" - if param.default is not inspect.Parameter.empty: - piece += f" = {param.default!r}" - parts.append(piece) - rendered = f"({', '.join(parts)})" - if sig.return_annotation is not inspect.Signature.empty: - rendered += f" -> {sig.return_annotation}" - return rendered - - -def _print_help(cls: type, methods: list[tuple[str, str]], label: str) -> None: - """Print every method in `methods` with its real signature (introspected - from `cls`, so it can't drift from the source) and a one-line - description. Signatures drop `self`; everything else — parameter names, - defaults, `*`-only markers, return types — comes straight from `cls`.""" - print(f"%{label} methods — usage: {_USAGE[label]}") - print() - for name, description in methods: - print(f" {name}{_format_signature(getattr(cls, name))}") - print(f" {description}") - print() - print(f"%{label} help — show this message") - - def _plain_params(func: Any) -> str: """Comma-separated parameter names (minus `self`), for a non-developer - reading `%catalog help`'s table — no Python type-hint syntax (a bare + reading `%catalog help`'s/`%ingest help`'s table — no Python type-hint syntax (a bare `str | None` union means nothing to a data custodian, and would collide visually with the table's own `|` column separators anyway) and no `*` keyword-only marker. A parameter with a default is shown as `name=default` @@ -299,26 +258,35 @@ def _plain_params(func: Any) -> str: return ", ".join(parts) -_CATALOG_HELP_HEADER_STYLE = ( +_HELP_TABLE_HEADER_STYLE = ( "text-align:left; padding:4px 12px; border-bottom:2px solid currentColor;" ) -_CATALOG_HELP_CELL_STYLE = ( +_HELP_TABLE_CELL_STYLE = ( "text-align:left; padding:4px 12px; border-bottom:1px solid currentColor; vertical-align:top;" ) -_CATALOG_HELP_CODE_STYLE = _CATALOG_HELP_CELL_STYLE + " font-family:monospace; white-space:pre;" +_HELP_TABLE_CODE_STYLE = _HELP_TABLE_CELL_STYLE + " font-family:monospace; white-space:pre;" + +# Catalog-specific: printed once above %catalog help's table, not per-row, +# since it applies across every path/source_path/target_path. %ingest help +# has no equivalent note. +_CATALOG_HELP_NOTE = ( + "A path/source_path/target_path starting with '.' resolves against the current " + "context (see use); one or more leading '../' (or a bare '..') walks up that many " + "levels first. Ordinary use already keeps context current on its own." +) -def _catalog_help_html(rows: list[tuple[str, str, str]]) -> str: - """Build the `
` markup for `%catalog help` — inline styles only - (no external stylesheet, no hardcoded background/text color — just - `currentColor` borders) so it reads correctly in both a light and a dark - notebook theme without knowing which one is active.""" +def _help_table_html(rows: list[tuple[str, str, str]]) -> str: + """Build the `
` markup shared by `%catalog help`/`%ingest help` — + inline styles only (no external stylesheet, no hardcoded background/text + color — just `currentColor` borders) so it reads correctly in both a + light and a dark notebook theme without knowing which one is active.""" def th(text: str) -> str: - return f'' + return f'' def td(text: str, *, code: bool = False) -> str: - style = _CATALOG_HELP_CODE_STYLE if code else _CATALOG_HELP_CELL_STYLE + style = _HELP_TABLE_CODE_STYLE if code else _HELP_TABLE_CELL_STYLE return f'' head = f"{th('Command')}{th('Parameters')}{th('Description')}" @@ -329,28 +297,24 @@ def td(text: str, *, code: bool = False) -> str: return f'
{_escape(text)}{_escape(text)}{_escape(text)}
{head}{body}
' -def _print_catalog_help_table() -> None: - """`%catalog help` — a real HTML `` (Command / Parameters / - Description), rendered via `IPython.display` rather than `_print_help`'s - ASCII Python-signature listing: this magic's audience is a data - custodian reading it in JupyterLab, not necessarily someone comfortable - with a Python type signature or a monospace grid. Only this one magic's - help gets the rich-display treatment — everything else in this module - stays plain `print()` (see the module docstring's "eval() below runs - exactly the Python the user typed" — errors and usage lines are meant to - read like ordinary interpreter output, not a UI).""" - print(f"%catalog methods — usage: {_USAGE['catalog']}") - print( - "A path/source_path/target_path starting with '.' resolves against the current " - "context (see use); one or more leading '../' (or a bare '..') walks up that many " - "levels first. Ordinary use already keeps context current on its own." - ) +def _print_help_table( + cls: type, methods: list[tuple[str, str]], label: str, *, note: str | None = None +) -> None: + """`%catalog help`/`%ingest help` — a real HTML `
` (Command / + Parameters / Description), rendered via `IPython.display` rather than a + raw Python-signature listing: both magics' audience is a data custodian + reading them in JupyterLab, not necessarily someone comfortable with a + Python type signature or a monospace grid. Only `help` output gets the + rich-display treatment — everything else in this module stays plain + `print()` (see the module docstring's "eval() below runs exactly the + Python the user typed" — errors and usage lines are meant to read like + ordinary interpreter output, not a UI).""" + print(f"%{label} methods — usage: {_USAGE[label]}") + if note: + print(note) - rows = [ - (name, _plain_params(getattr(CatalogSession, name)), description) - for name, description in _CATALOG_HELP - ] - display(HTML(_catalog_help_html(rows))) + rows = [(name, _plain_params(getattr(cls, name)), description) for name, description in methods] + display(HTML(_help_table_html(rows))) def _dispatch( @@ -461,7 +425,7 @@ def _ensure_catalog_session(self) -> bool: @line_magic def catalog(self, line: str) -> Any: if _is_help(line): - _print_catalog_help_table() + _print_help_table(CatalogSession, _CATALOG_HELP, "catalog", note=_CATALOG_HELP_NOTE) return None if not self._ensure_catalog_session(): return None @@ -481,7 +445,7 @@ def catalog_cell(self, line: str, cell: str) -> Any: `CatalogSession.use`) instead of repeating a path on every line:: %%catalog use("bwd.reference") - tag(".water_temperature", ["reviewed"]) + set_tags(".water_temperature", ["reviewed"]) create_folder(".2027") Each line dispatches exactly like a `%catalog` line-magic call @@ -512,7 +476,7 @@ def catalog_cell(self, line: str, cell: str) -> Any: @line_magic def ingest(self, line: str) -> Any: if _is_help(line): - _print_help(IngestSession, _INGEST_HELP, "ingest") + _print_help_table(IngestSession, _INGEST_HELP, "ingest") return None if self._ingest_session is None: self._ingest_session = IngestSession() diff --git a/tests/notebook/test_magics.py b/tests/notebook/test_magics.py index 70a77f3..ccb4fee 100644 --- a/tests/notebook/test_magics.py +++ b/tests/notebook/test_magics.py @@ -304,11 +304,24 @@ def test_catalog_help_lists_methods_without_needing_credentials( @pytest.mark.parametrize("line", ["help", "help()"]) -def test_ingest_help_lists_methods(ip: Any, capsys: pytest.CaptureFixture[str], line: str) -> None: +def test_ingest_help_lists_methods( + ip: Any, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], line: str +) -> None: + # Same HTML-table rendering as %catalog help — see that test for why + # display() is captured directly rather than via capsys. + displayed = [] + monkeypatch.setattr(magics_module, "display", displayed.append) + ip.run_line_magic("ingest", line) out = capsys.readouterr().out assert "%ingest methods" in out - assert "ingest(folder: str | Path, target_catalog_path: str" in out - assert "commit(*, retry: bool = False, max_retries: int = 3)" in out + + assert len(displayed) == 1 + 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 "folder, target_catalog_path" in table_html + assert "retry=False, max_retries=3" 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