From 379a4dc1c74801f6dbecdbc1531db5257bafe9e0 Mon Sep 17 00:00:00 2001 From: Joel Natividad <1980690+jqnatividad@users.noreply.github.com> Date: Sat, 10 Oct 2026 13:26:03 -0400 Subject: [PATCH] fix: resolve stdout_path against cwd=, and raise QsvError from the probe - A relative stdout_path is now resolved against the client's cwd=, like a relative --output given to qsv, instead of Python's working directory. Behavior change: output files of runs that combine cwd= with a relative stdout_path move. - capabilities (and run with min_version) raise QsvError instead of a bare ValueError when --version output isn't recognized, and fall back to --version when --capabilities returns JSON of an unexpected shape. Qsv and AsyncQsv now share this probe logic. - The QsvError docstring named the argv attribute args; it is args_run. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 +++++++ README.md | 5 ++- src/qsv_client/client.py | 51 +++++++++++++++++++-------- src/qsv_client/errors.py | 3 +- tests/test_process.py | 74 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 128 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f7df3..ec02fd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,23 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Changed + +- **Behavior change:** a relative `stdout_path` is now resolved against the client's `cwd=`, + like a relative `--output` path given to qsv. Previously it was resolved against Python's + working directory, so code combining `cwd=` with a relative `stdout_path` will now write its + output file somewhere else, without any error. Pass an absolute path to keep the old location. + ([#7](https://github.com/dathere/qsv-client/pull/7)) + ### Fixed - `AsyncQsv`: `QsvTimeout.stderr` now includes what qsv wrote before the timeout, as it already did for `Qsv`. Previously, stderr already read when the timeout fired was lost. + ([#6](https://github.com/dathere/qsv-client/pull/6)) +- `capabilities` (and `run` with `min_version`) raise `QsvError` instead of a bare + `ValueError` when the binary's `--version` output isn't recognized, and fall back to `--version` + when `--capabilities` returns JSON of an unexpected shape. ([#7](https://github.com/dathere/qsv-client/pull/7)) +- The `QsvError` docstring named the argv attribute `args`; it is `args_run`. ([#7](https://github.com/dathere/qsv-client/pull/7)) ## [0.2.0] - 2026-10-09 diff --git a/README.md b/README.md index bd6446a..6a720e0 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,8 @@ qsv = Qsv( ``` A relative `binary` path is resolved against the current directory when the client is -created, so it still works with `cwd=`. +created, so it still works with `cwd=`. A relative `stdout_path` is resolved against `cwd=`, +like a relative `--output` path given to qsv. ### describegpt without exposing your key @@ -139,6 +140,8 @@ message is taken from stderr. `QsvNotFound` (no binary) and `QsvVersionError` (binary too old) are raised before anything runs. +If the binary's `--version` output isn't recognized (and `--capabilities` gave nothing +usable), `capabilities` (and `run` with `min_version`) raise a plain `QsvError`. ## Platform notes diff --git a/src/qsv_client/client.py b/src/qsv_client/client.py index dd5515d..7eed1a1 100644 --- a/src/qsv_client/client.py +++ b/src/qsv_client/client.py @@ -2,7 +2,6 @@ from __future__ import annotations -import contextlib import csv import io import json @@ -19,6 +18,7 @@ from .errors import ( EXIT_TIMEOUT, SUCCESS_EXIT_CODES, + QsvError, QsvNotFound, QsvTimeout, QsvVersionError, @@ -217,15 +217,41 @@ def _finish( def _open_stdout( - stdout_path: str | os.PathLike[str] | None, + stdout_path: str | os.PathLike[str] | None, cwd: str | None ) -> tuple[Path | None, IO[bytes] | None]: if stdout_path is None: return None, None path = Path(stdout_path) + if cwd is not None and not path.is_absolute(): + # like qsv's own --output: relative to the run's working directory + path = Path(cwd) / path path.parent.mkdir(parents=True, exist_ok=True) return path, path.open("wb") +def _caps_from_json(res: QsvResult) -> Capabilities | None: + """Capabilities from a ``--capabilities`` run, or None if it gave nothing usable.""" + if res.exit_code != 0 or not res.stdout: + return None + try: + return from_capabilities_json(res.stdout) + except (ValueError, TypeError, AttributeError): + # not JSON, or JSON of an unexpected shape: fall back to --version + return None + + +def _caps_from_version(ver: QsvResult, lst: QsvResult) -> Capabilities: + try: + return from_version_output(ver.stdout or "", lst.stdout or "") + except ValueError as exc: + raise QsvError( + f"could not determine the qsv version: {exc}", + exit_code=ver.exit_code, + args=ver.args, + stderr=ver.stderr, + ) from exc + + def _ensure_json_format(args: Sequence[Arg]) -> list[Arg]: out = list(args) if not any(str(a) == "--format" or str(a).startswith("--format=") for a in out): @@ -258,7 +284,8 @@ def run( Args: stdin: data to feed on stdin. Default: stdin is closed (``/dev/null``). stdout_path: stream stdout to this file instead of holding it in memory (use for - large outputs). The file is created/truncated, parent dirs included. + large outputs). The file is created/truncated, parent dirs included. A + relative path is resolved against the client's ``cwd``, if set. timeout: overrides the client default for this run. env: extra environment variables for this run. check: raise a :class:`QsvError` subclass if qsv fails (exit codes 0 and 255, @@ -297,7 +324,7 @@ def _run_unchecked( ) -> QsvResult: argv = self._argv(command, args) effective_timeout = timeout if timeout is not None else self.timeout - path, fh = _open_stdout(stdout_path) + path, fh = _open_stdout(stdout_path, self.cwd) started = time.monotonic() try: raw = _process.run_sync( @@ -337,15 +364,12 @@ def version(self) -> str: return self.capabilities.version def _probe(self) -> Capabilities: - caps: Capabilities | None = None res = self._run_unchecked("", ["--capabilities"], check=False, timeout=60) - if res.exit_code == 0 and res.stdout: - with contextlib.suppress(ValueError): - caps = from_capabilities_json(res.stdout) + caps = _caps_from_json(res) if caps is None: ver = self._run_unchecked("", ["--version"], timeout=60) lst = self._run_unchecked("", ["--list"], check=False, timeout=60) - caps = from_version_output(ver.stdout or "", lst.stdout or "") + caps = _caps_from_version(ver, lst) # checked here, so it holds however the cache gets filled self._check_min_version(caps) return caps @@ -430,7 +454,7 @@ async def _run_unchecked( ) -> QsvResult: argv = self._argv(command, args) effective_timeout = timeout if timeout is not None else self.timeout - path, fh = _open_stdout(stdout_path) + path, fh = _open_stdout(stdout_path, self.cwd) started = time.monotonic() try: raw = await _process.run_async( @@ -466,15 +490,12 @@ async def version(self) -> str: return (await self.capabilities()).version async def _probe(self) -> Capabilities: - caps: Capabilities | None = None res = await self._run_unchecked("", ["--capabilities"], check=False, timeout=60) - if res.exit_code == 0 and res.stdout: - with contextlib.suppress(ValueError): - caps = from_capabilities_json(res.stdout) + caps = _caps_from_json(res) if caps is None: ver = await self._run_unchecked("", ["--version"], timeout=60) lst = await self._run_unchecked("", ["--list"], check=False, timeout=60) - caps = from_version_output(ver.stdout or "", lst.stdout or "") + caps = _caps_from_version(ver, lst) self._check_min_version(caps) return caps diff --git a/src/qsv_client/errors.py b/src/qsv_client/errors.py index 7e7ddca..1816325 100644 --- a/src/qsv_client/errors.py +++ b/src/qsv_client/errors.py @@ -42,7 +42,8 @@ class QsvError(Exception): message: the error message, without qsv's text-mode prefix (e.g. ``csv error: ``). exit_code: the process exit code. command: the qsv subcommand, when known. - args: the full argv that was run (binary first). + args_run: the full argv that was run (binary first). (``args`` is the standard + ``Exception.args``.) stderr: everything the process wrote to stderr. structured: True when the error came from qsv's JSON error line, False when it was inferred from the exit code and free-text stderr. diff --git a/tests/test_process.py b/tests/test_process.py index bd1014d..1bca4a0 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -15,7 +15,7 @@ import pytest -from qsv_client import AsyncQsv, Qsv, QsvTimeout, QsvUsageError, _process +from qsv_client import AsyncQsv, Qsv, QsvError, QsvTimeout, QsvUsageError, _process # The fake starts a grandchild that inherits stdout and records its pid, then hangs. If only # the fake were killed, the grandchild would keep the stdout pipe open and the run would hang @@ -565,3 +565,75 @@ def no_thread(self: threading.Thread) -> None: assert _wait_reaped(_read_pid(pidfile)), "leader survived a failed feeder start" finally: _kill(_read_pid(pidfile)) + + +PRINT_CSV = """ +print("a,b") +print("1,2") +""" + + +def test_relative_stdout_path_follows_cwd( + fake_qsv: Callable[[str], str], tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + work, elsewhere = tmp_path / "work", tmp_path / "elsewhere" + work.mkdir() + elsewhere.mkdir() + monkeypatch.chdir(elsewhere) + res = Qsv(fake_qsv(PRINT_CSV), cwd=work).run("select", stdout_path="out/res.csv") + assert res.stdout_path == work / "out" / "res.csv" + assert res.csv_rows() == [{"a": "1", "b": "2"}] + assert not (elsewhere / "out").exists() + + +async def test_async_relative_stdout_path_follows_cwd( + fake_qsv: Callable[[str], str], tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + work, elsewhere = tmp_path / "work", tmp_path / "elsewhere" + work.mkdir() + elsewhere.mkdir() + monkeypatch.chdir(elsewhere) + res = await AsyncQsv(fake_qsv(PRINT_CSV), cwd=work).run("select", stdout_path="res.csv") + assert res.stdout_path == work / "res.csv" + assert res.csv_rows() == [{"a": "1", "b": "2"}] + + +# --capabilities answers with JSON of the wrong shape; --version is a valid qsvlite line +MALFORMED_CAPABILITIES = """ +import sys +if sys.argv[1] == "--capabilities": + print('{"version": "24.0.0", "feature_versions": ["oops"]}') +elif sys.argv[1] == "--version": + print("qsvlite 24.0.0-standard--4-4;1 GiB-0 B-2 GiB-4 GiB " + "(x86_64-unknown-linux-gnu compiled with Rust 1.99;x-y;z-2) compiled") +""" + +UNRECOGNIZED_VERSION = """ +print("not qsv at all") +""" + + +def test_malformed_capabilities_falls_back_to_version(fake_qsv: Callable[[str], str]) -> None: + caps = Qsv(fake_qsv(MALFORMED_CAPABILITIES)).capabilities + assert (caps.binary, caps.version) == ("qsvlite", "24.0.0") + + +async def test_async_malformed_capabilities_falls_back_to_version( + fake_qsv: Callable[[str], str], +) -> None: + caps = await AsyncQsv(fake_qsv(MALFORMED_CAPABILITIES)).capabilities() + assert (caps.binary, caps.version) == ("qsvlite", "24.0.0") + + +def test_unrecognized_version_raises_qsv_error(fake_qsv: Callable[[str], str]) -> None: + qsv = Qsv(fake_qsv(UNRECOGNIZED_VERSION), min_version="1.0.0") + with pytest.raises(QsvError, match="could not determine the qsv version"): + qsv.run("count", "x.csv") + + +async def test_async_unrecognized_version_raises_qsv_error( + fake_qsv: Callable[[str], str], +) -> None: + qsv = AsyncQsv(fake_qsv(UNRECOGNIZED_VERSION), min_version="1.0.0") + with pytest.raises(QsvError, match="could not determine the qsv version"): + await qsv.run("count", "x.csv")