From 3e5ff2d3c4c06f29c095e51da03400a81d4868b4 Mon Sep 17 00:00:00 2001 From: Joel Natividad <1980690+jqnatividad@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:48:54 -0400 Subject: [PATCH] build(deps): require qsv-client>=0.4.0 qsv-client 0.4.0 requires qsv 25.0.0 (DP+'s MINIMUM_QSV_VERSION) and checks it via `qsv --capabilities` before a client's first command, raising QsvVersionError for older binaries. - QSVCommand.version() maps QsvVersionError to DP+'s "At least qsv version ... required" JobError, so a too-old qsv_bin (including one without --capabilities) is reported as before. - test_ai_suggestions: the qsv_command fixture patched out the version check, so the capabilities probe ran inside each test's run_sync mock and failed; prime it at construction instead. - New test for a qsv that rejects --capabilities. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- ckanext/datapusher_plus/qsv_utils.py | 20 +++++++++++--------- requirements.txt | 5 +++-- tests/test_ai_suggestions.py | 14 ++++++++------ tests/test_qsv_command_client.py | 19 ++++++++++++++++++- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87254c6..c9734e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * New `ckanext.datapusher_plus.prefect_enabled` setting (default `true`). Setting it to `false` turns Prefect off entirely: `datapusher_submit` enqueues the job on CKAN's own background-job queue and the new `ckanext/datapusher_plus/jobs/local_runner.py` executes the same nine ingestion stages in-process, so no Prefect server, worker, work pool — or `import prefect` — is involved. Operators run `ckan jobs worker` instead. This is the fix for deployments where importing Prefect *itself* fails, e.g. a CKAN process running with `HOME=/root` but no write access there, which surfaces as `Error submitting job to DataPusher: [Errno 13] Permission denied: '/root/.prefect/profiles.toml'` (the alternative fix, if you want to keep Prefect, is to point `PREFECT_HOME` at a writable directory). The local runner keeps the `Jobs`/`Logs` tables, the `datapusher_hook` callbacks, the complete-with-skip contract, the `flow_timeout` deadline, and the post-database-failure datastore cleanup; it does *not* provide per-stage retries, result caching, the Prefect run graph/artifacts/events, or human-in-the-loop PII review — with Prefect off, a job crossing `pii_review_threshold` aborts before any datastore write rather than suspending for approval. The task_status row records the RQ job id as `rq_job_id` (not `flow_run_id`), so nothing builds a Prefect-UI deep link out of it. See [Running without Prefect](README.md#running-without-prefect). ### Changed -* Require `qsv-client>=0.2.0` (was `>=0.1.0`), so existing environments pick up its fixes on the next `pip install -r requirements.txt`. The one that matters for DP+: a timeout is bounded again even when a process qsv started — e.g. one spawned by `describegpt` in the AI suggestions stage — leaves the process group and keeps the output pipes open; previously the job waited for that process to exit. No DP+ code changes are needed. ([qsv-client 0.2.0 release notes](https://github.com/dathere/qsv-client/releases/tag/0.2.0)) +* Require `qsv-client>=0.4.0` (was `>=0.1.0`), so existing environments pick up its fixes on the next `pip install -r requirements.txt`. The one that matters most for DP+ (from 0.2.0): a timeout is bounded again even when a process qsv started — e.g. one spawned by `describegpt` in the AI suggestions stage — leaves the process group and keeps the output pipes open; previously the job waited for that process to exit. qsv-client 0.4.0 requires qsv 25.0.0 — the same floor as DP+'s `MINIMUM_QSV_VERSION` — and drops its `--version` fallback for older qsv. A too-old `qsv_bin` (including one without `--capabilities`) is still rejected at startup with `JobError: … At least qsv version 25.0.0 required. …`. ([qsv-client 0.2.0](https://github.com/dathere/qsv-client/releases/tag/0.2.0), [0.3.0](https://github.com/dathere/qsv-client/releases/tag/0.3.0), [0.4.0](https://github.com/dathere/qsv-client/releases/tag/0.4.0) release notes) * The Prefect-free half of the pipeline moved out of `jobs/prefect_flow.py` into the new `jobs/pipeline_core.py` (the CKAN status callback, input validation, `RuntimeContext` construction, the stage invoker and its `StageAbort` signal, and the datastore rollback body), shared verbatim by the Prefect flow and the local runner. `prefect_flow` keeps its historical private names as aliases, so custom flows composed from its `@task` primitives are unaffected. Tests that patched `prefect_flow.dsu` / `.QSVCommand` / `.Path` now patch `pipeline_core`. * Auto-indexing now uses a closed cardinality range `[auto_index_min_threshold, auto_index_threshold]` instead of a single upper bound (issue #142). Defaults bumped: `auto_index_threshold` `3` → `10` and the new `auto_index_min_threshold` defaults to `3`. The new lower bound skips the useless-single-value-column case @EricSoroos flagged in #142 (a 1-value text column previously produced a 10–40MB B-tree the Postgres planner would never choose). The upper bump from 3 → 10 widens the DataTables-SearchBuilder filtering sweet spot to cover typical enum-shaped columns (status = 3–10 values, common enums under ~10). **Operator-facing impact at default settings:** columns with cardinality 1–2 lose their auto-index (intentional — the indexes were dead weight); columns with cardinality 4–10 gain an auto-index. No migration is required; the change re-applies on the next resubmit. To restore pre-#142 behavior (no lower floor) set `ckanext.datapusher_plus.auto_index_min_threshold = 0`. The setting `auto_index_threshold = -1` ("index every column") still works but now hits the `min_threshold = 3` floor unless paired with `auto_index_min_threshold = 0`. * **BREAKING** Bumped `MINIMUM_QSV_VERSION` from `4.0.0` to `20.0.0`. Operators must upgrade their `qsv` binary at the path configured by `ckanext.datapusher_plus.qsv_bin` before deploying this version — DP+ will refuse to start otherwise. See [qsv 20.0.0 release notes](https://github.com/dathere/qsv/releases/tag/20.0.0) and the migration notes below. ([README install snippet](README.md#option-2-install-prebuilt-qsv-binaries) updated accordingly.) diff --git a/ckanext/datapusher_plus/qsv_utils.py b/ckanext/datapusher_plus/qsv_utils.py index b760f6d..d65f23d 100644 --- a/ckanext/datapusher_plus/qsv_utils.py +++ b/ckanext/datapusher_plus/qsv_utils.py @@ -14,7 +14,7 @@ import ckanext.datapusher_plus.utils as utils from typing import Optional, Dict, Any, List, Union -from qsv_client import Qsv, QsvError, QsvNotFound, QsvTimeout +from qsv_client import Qsv, QsvError, QsvNotFound, QsvTimeout, QsvVersionError from qsv_client.errors import error_from_run from ckanext.datapusher_plus.logging_utils import TRACE @@ -137,23 +137,25 @@ def _run_command( def version(self) -> str: """ - Get the qsv version. - - Read from ``qsv --capabilities`` on qsv versions that have it, and - from ``qsv --version`` otherwise. + Get the qsv version, read from ``qsv --capabilities``. Returns: - The qsv version string, e.g. ``"24.0.0"`` + The qsv version string, e.g. ``"25.0.0"`` Raises: - utils.JobError: If the version command fails - ValueError: If the version output cannot be parsed + utils.JobError: If the binary is older than qsv-client's minimum + (qsv 25.0.0), or ``--capabilities`` fails """ try: return self._qsv.version + except QsvVersionError as e: + raise utils.JobError( + f"At least qsv version {conf.MINIMUM_QSV_VERSION} required. {e}. " + f"You can get the latest release at https://github.com/dathere/qsv/releases/latest" + ) from e except QsvError as e: raise utils.JobError( - f"Cannot get qsv version info. Command: {self.qsv_bin} --version. {e}" + f"Cannot get qsv version info. Command: {self.qsv_bin} --capabilities. {e}" ) from e def check_version(self) -> bool: diff --git a/requirements.txt b/requirements.txt index 9fd4f2c..994212c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -31,5 +31,6 @@ blake3>=1.0.7 babel>=2.9 # qsv-client: runs qsv for QSVCommand. Kills the whole process group on # timeout (qsv and anything it started) and reads qsv's structured errors -# (QSV_ERROR_FORMAT=json) where the binary supports them. No dependencies. -qsv-client>=0.2.0 +# (QSV_ERROR_FORMAT=json). 0.4.0+ requires qsv 25.0.0+ (DP+'s own +# MINIMUM_QSV_VERSION) and refuses older binaries. No dependencies. +qsv-client>=0.4.0 diff --git a/tests/test_ai_suggestions.py b/tests/test_ai_suggestions.py index 225f681..018fbec 100644 --- a/tests/test_ai_suggestions.py +++ b/tests/test_ai_suggestions.py @@ -37,14 +37,16 @@ @pytest.fixture def qsv_command(): pytest.importorskip("ckan") - # ``QSVCommand.__init__`` verifies the qsv binary exists and runs a - # version check — neither is interesting for these tests. Patch - # both before constructing. + # ``QSVCommand.__init__`` verifies the qsv binary exists and runs its + # version check through ``qsv --capabilities``. Fake both, so the + # capabilities probe is done here and not inside a test's RUN_SYNC mock. from ckanext.datapusher_plus import qsv_utils - with mock.patch.object(qsv_utils.Path, "is_file", return_value=True), \ - mock.patch.object(qsv_utils.QSVCommand, "check_version", return_value=None): - yield qsv_utils.QSVCommand(logger=mock.Mock()) + capabilities = json.dumps({"binary": "qsvdp", "version": "99.0.0", "features": [], "commands": []}) + with mock.patch.object(qsv_utils.Path, "is_file", return_value=True): + with mock.patch(RUN_SYNC, return_value=_raw(capabilities)): + qsv_command = qsv_utils.QSVCommand(logger=mock.Mock()) + yield qsv_command def _completed(stdout: str = "", returncode: int = 0) -> subprocess.CompletedProcess: diff --git a/tests/test_qsv_command_client.py b/tests/test_qsv_command_client.py index e675055..d25993e 100644 --- a/tests/test_qsv_command_client.py +++ b/tests/test_qsv_command_client.py @@ -26,7 +26,11 @@ args = sys.argv[1:] cmd = args[0] if args else "" -if cmd == "--capabilities": +if cmd == "--capabilities" and os.environ.get("FAKE_QSV_NO_CAPABILITIES"): + # qsv before 25.0.0 + print("Unknown flag: '--capabilities'", file=sys.stderr) + sys.exit(1) +elif cmd == "--capabilities": print(json.dumps({"binary": "qsvdp", "version": os.environ.get("FAKE_QSV_VERSION", "99.0.0"), "features": [], "commands": ["count"], "error_formats": ["text", "json"]})) elif cmd == "ok": @@ -94,6 +98,19 @@ def test_constructor_rejects_a_too_old_qsv(fake_qsv, monkeypatch): qsv_utils.QSVCommand(logger=mock.Mock()) +def test_constructor_rejects_a_qsv_without_capabilities(fake_qsv, monkeypatch): + # qsv before 25.0.0 has no --capabilities; qsv-client 0.4.0+ raises + # QsvVersionError, which must surface as DP+'s minimum-version error + pytest.importorskip("ckan") + from ckanext.datapusher_plus import qsv_utils + + monkeypatch.setenv("FAKE_QSV_NO_CAPABILITIES", "1") + with mock.patch.object(qsv_utils.conf, "QSV_BIN", fake_qsv), pytest.raises( + _job_error(), match="At least qsv version.*does not support --capabilities" + ): + qsv_utils.QSVCommand(logger=mock.Mock()) + + def test_success_returns_a_completed_process(qsv, fake_qsv): result = qsv._run_command(["ok"]) assert isinstance(result, subprocess.CompletedProcess)