From ab85b423bf33f82c7c88e98b2e7b34e21406ffbd Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 15:31:19 +0100 Subject: [PATCH 01/18] Add Windows installation notes and information about Python versions to README and readthedocs --- README.md | 11 +++++++++++ docs/source/index.rst | 13 ++++++++++++- docs/source/tutorial/installing_labthings.rst | 18 +++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9395a8ec..077e90f3 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,17 @@ Documentation, including install instructions, is available on [readthedocs]. See [readthedocs] for installation instructions that are automatically tested. You can install this package with `pip install labthings-fastapi`. + +### Installation Notes + +`labthings-fastapi` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current `pydantic-core` dependencies. + +> **Note: Windows Installations** +> +> Installing on Windows requires Visual Studio with the **"Desktop development with C++"** workload enabled. This is necessary because `pydantic` relies on Rust, which in turn requires C++ build tools to compile. +> +> *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* + ## Developer notes For the latest development version, clone this repository and run `pip install -e .[dev]`. diff --git a/docs/source/index.rst b/docs/source/index.rst index a7a1001f..c4db32a6 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -50,6 +50,15 @@ Installation ``pip install labthings-fastapi`` +``labthings-fastapi`` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current ``pydantic-core`` dependencies. + +.. note:: + **Windows Installations** + + Installing on Windows requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. + + *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* + Indices and tables ================== @@ -59,4 +68,6 @@ Indices and tables .. _python-labthings: https://github.com/labthings/python-labthings/ .. _FastAPI: https://fastapi.tiangolo.com/ -.. _pydantic: https://pydantic-docs.helpmanual.io/ \ No newline at end of file +.. _pydantic: https://pydantic-docs.helpmanual.io/ +.. _Visual Studio: https://visualstudio.microsoft.com/ +.. _Rust: https://www.rust-lang.org/ \ No newline at end of file diff --git a/docs/source/tutorial/installing_labthings.rst b/docs/source/tutorial/installing_labthings.rst index fc82302f..d6d30c0e 100644 --- a/docs/source/tutorial/installing_labthings.rst +++ b/docs/source/tutorial/installing_labthings.rst @@ -25,4 +25,20 @@ then install labthings with: It is also possible to install LabThings from source, by cloning the GitHub repository and running ``pip install -e .[dev]``, but this is only recommended if you intend to alter the LabThings-FastAPI library; it is best to use the published package unless you have a good reason not to. -.. _PyPI: https://pypi.org/project/labthings-fastapi/ \ No newline at end of file + +Installation Notes +++++++++++++++++++ + +``labthings-fastapi`` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current ``pydantic-core`` dependencies. + +.. note:: + **Windows Installations** + + Installing on Windows requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. + + *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* + + +.. _PyPI: https://pypi.org/project/labthings-fastapi/ +.. _Visual Studio: https://visualstudio.microsoft.com/ +.. _Rust: https://www.rust-lang.org/ \ No newline at end of file From 1ef0e57acd412f99f8c280ac6238dd24381cf594 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 15:52:31 +0100 Subject: [PATCH 02/18] Add a contributing.md file --- CONTRIBUTING.md | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 15 +------ 2 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..33c862d9 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,103 @@ +# Contributing to labthings-fastapi + +First off, thank you for considering contributing to `labthings-fastapi`! We welcome contributions from everyone, whether it's reporting a bug, suggesting a feature, improving documentation, or writing code. + +This document outlines the processes for getting help, reporting issues, and contributing to the codebase. + +## Seeking Support + +If you have a question about how to use `labthings-fastapi`, or if you are running into trouble: + +* **Check the Documentation:** Please review the official documentation on [readthedocs]. +* **GitHub Discussions / Issues:** If you cannot find the answer, feel free to open an issue on our [GitHub Issues page]. +* **OpenFlexure Community:** Because `labthings-fastapi` is the underlying framework for v3 of the [OpenFlexure Microscope software], you may also find support by engaging with the broader [OpenFlexure Forum]. + +## Reporting Issues or Bugs + +If you find a bug or have a feature request, please report it by opening an issue on our [GitHub Issues page]. + +When reporting an issue, please include as much detail as possible: + +* **Description:** A clear and concise description of what the bug is. +* **Reproduction steps:** How can we reproduce the problem? (A minimal reproducible example is highly appreciated). +* **Expected behaviour:** What did you expect to happen? +* **Environment:** Include your OS, Python version, and `labthings-fastapi` version. Include full error tracebacks if applicable. + +## Contributing Code or Documentation + +We welcome pull requests for bug fixes, new features, and documentation improvements. + +### 1. Local Development Setup + +To work on the code, you will need to clone the repository and install the development dependencies. +Please see the [installation notes](./README.md#installation-notes) for more detail about compatible Python versions and Windows installation. + +```bash +# Clone the repository +git clone https://github.com/labthings/labthings-fastapi.git +cd labthings-fastapi + +# Install the package in editable mode with development dependencies +pip install -e .[dev] +``` + +### 2. Linting and Testing + +We use several tools to maintain code quality. All of these run in CI with [GitHub Actions], but you should run them locally before submitting a Pull Request. Both `ruff` and `flake8` are configured from [`pyproject.toml`]. + +* **Linting:** We use [`ruff`] for fast linting and formatting. We highly recommend setting up a pre-commit hook to ensure [`ruff`] passes on every commit. + ```bash + ruff check . + ``` + +* **Docstrings:** [`flake8`] is primarily used to enable stricter checks on docstrings. + ```bash + flake8 src + ``` + +* **Type Checking:** We use [`mypy`] for static type checking. + ```bash + mypy src + ``` + +* **Testing:** We use [`pytest`] for our test suite. Ensure all tests pass locally. + ```bash + pytest + ``` + +### 3. Managing Dependencies + +Dependencies are defined in [`pyproject.toml`]. If you need to compile a `dev-requirements.txt` file (e.g., for reproducible CI/CD or local isolated environments), you can do so using [`uv`]: + +```bash +uv pip compile --extra dev pyproject.toml --output-file dev-requirements.txt +``` + +*(If you're not using `uv`, regular `pip-compile` from `pip-tools` will achieve the same thing).* + +### 4. Submitting a Pull Request (PR) + +All changes to the codebase must go via pull requests. + +1. Create a new branch for your feature or bugfix (`git checkout -b feature-name`). +2. Commit your changes with clear, descriptive commit messages. +3. Push your branch to your fork or the main repository. +4. Open a Pull Request against the `main` branch. + +**Pull Request Guidelines:** + +* Code should only be merged once all the checks in the CI test job are passing. +* **Unpinned Dependencies:** Note that we have a specific CI job called `test-with-unpinned-dependencies`. It is acceptable to merge code if only this specific job fails, provided the failure is due to upstream dependency issues. We prefer to deal with upstream dependency issues in a separate PR, particularly when the required fixes are distinct from the code in your current PR. +* Update documentation (`docs/` or docstrings) if your changes modify existing behavior or add new features. + +[readthedocs]: https://labthings-fastapi.readthedocs.io/ +[GitHub Issues page]: https://github.com/labthings/labthings-fastapi/issues +[OpenFlexure Forum]: https://openflexure.discourse.group/ +[OpenFlexure Microscope software]: https://gitlab.com/openflexure/openflexure-microscope-server/ +[GitHub Actions]: https://github.com/labthings/labthings-fastapi/actions +[`ruff`]: https://docs.astral.sh/ruff/ +[`pyproject.toml`]: ./pyproject.toml +[`flake8`]: https://flake8.pycqa.org/en/latest/ +[`mypy`]: https://mypy-lang.org/ +[`pytest`]: https://docs.pytest.org/en/stable/ +[`uv`]: https://docs.astral.sh/uv/ \ No newline at end of file diff --git a/README.md b/README.md index 077e90f3..b0b30d3d 100644 --- a/README.md +++ b/README.md @@ -22,19 +22,7 @@ See [readthedocs] for installation instructions that are automatically tested. Y > > *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* -## Developer notes - -For the latest development version, clone this repository and run `pip install -e .[dev]`. - -The code is linted with `ruff .`, type checked with `mypy`, and tested with `pytest`. These all run in CI with GitHub Actions. We recommend a [pre-commit hook] to ensure `ruff` passes on every commit. `flake8` is also run in CI, primarily to enable stricter checks on docstrings. It is run as `flake8 src`. `ruff` and `flake8` are both configured from `pyproject.toml`. - -All changes to the codebase should go via pull requests, and should only be merged once all the checks in the `test` job are passing. It is preferable to merge code where the `test-with-unpinned-dependencies` job fails, and deal with the dependency issues in another PR, particularly where the required changes are distinct from the code in the PR. - -Dependencies are defined in `pyproject.toml` and can be compiled to `dev-requirements.txt` with: -``` -uv pip compile --extra dev pyproject.toml --output-file dev-requirements.txt -``` -If you're not using `uv`, just regular `pip-compile` from `pip-tools` should do the same thing. +For instructions on how to set up a development environment, run tests, and contribute to this project, please see [CONTRIBUTING.md]. ## Demo @@ -45,3 +33,4 @@ See [readthedocs] for a runnable demo. [OpenFlexure Microscope software]: https://gitlab.com/openflexure/openflexure-microscope-server/ [pre-commit hook]: https://openflexure.org/contribute#use-git-hooks-for-ci-checks [readthedocs]: https://labthings-fastapi.readthedocs.io/ +[CONTRIBUTING.md]: ./CONTRIBUTING.md From 6e7179e2fbb8e8b272e5f24e015bcdfc9ab13872 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 15:59:35 +0100 Subject: [PATCH 03/18] Enable sphinx to parse markdown files --- docs/source/conf.py | 3 +++ docs/source/index.rst | 1 + 2 files changed, 4 insertions(+) diff --git a/docs/source/conf.py b/docs/source/conf.py index 9af5f824..44df21ee 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -27,8 +27,11 @@ "autoapi.extension", "sphinx_rtd_theme", "sphinx_toolbox.decorators", + "myst_parser", ] +myst_heading_anchors = 3 + templates_path = ["_templates"] exclude_patterns = [] diff --git a/docs/source/index.rst b/docs/source/index.rst index c4db32a6..6c720499 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -5,6 +5,7 @@ Documentation for LabThings-FastAPI :maxdepth: 2 :caption: Contents: + CONTRIBUTING.md quickstart/quickstart.rst tutorial/index.rst structure.rst From b0d05a488852f227e36c35574a613315c86ff581 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 16:05:46 +0100 Subject: [PATCH 04/18] Add myst-parser to pyproject.toml --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 155a0425..84e28e48 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,6 +42,7 @@ dev = [ "sphinx-autoapi", "sphinx-toolbox", "sphobjinv", + "myst-parser", "tomli; python_version < '3.11'", "codespell", ] From 9e770b030dbf4c253335d1383897f72b548f52e7 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 16:09:16 +0100 Subject: [PATCH 05/18] Update dev-requirements --- dev-requirements.txt | 78 +++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 8b910ac6..cbec2a38 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,5 +1,9 @@ -# This file was autogenerated by uv via the following command: -# uv pip compile --extra dev pyproject.toml --output-file dev-requirements.txt +# +# This file is autogenerated by pip-compile with Python 3.13 +# by the following command: +# +# pip-compile --extra=dev --output-file=dev-requirements.txt pyproject.toml +# alabaster==1.0.0 # via sphinx annotated-doc==0.0.4 @@ -10,8 +14,8 @@ annotated-types==0.7.0 # via pydantic anyio==4.13.0 # via - # labthings-fastapi (pyproject.toml) # httpx + # labthings-fastapi (pyproject.toml) # starlette # watchfiles apeye==1.4.1 @@ -31,7 +35,7 @@ babel==2.18.0 # via sphinx beautifulsoup4==4.14.3 # via sphinx-toolbox -cachecontrol==0.14.4 +cachecontrol[filecache]==0.14.4 # via sphinx-toolbox certifi==2026.5.20 # via @@ -50,7 +54,14 @@ click==8.4.1 # uvicorn codespell==2.4.2 # via labthings-fastapi (pyproject.toml) -coverage==7.14.1 +colorama==0.4.6 + # via + # click + # pytest + # sphinx + # typer + # uvicorn +coverage[toml]==7.14.1 # via pytest-cov detect-installer==0.1.0 # via fastapi-cloud-cli @@ -62,6 +73,7 @@ docstring-parser-fork==0.0.14 # via pydoclint docutils==0.21.2 # via + # myst-parser # restructuredtext-lint # sphinx # sphinx-prompt @@ -78,13 +90,9 @@ email-validator==2.3.0 # via # fastapi # pydantic -exceptiongroup==1.3.1 - # via - # anyio - # pytest -fastapi==0.135.4 +fastapi[all]==0.135.4 # via labthings-fastapi (pyproject.toml) -fastapi-cli==0.0.24 +fastapi-cli[standard]==0.0.24 # via fastapi fastapi-cloud-cli==0.18.0 # via fastapi-cli @@ -96,10 +104,10 @@ filelock==3.29.0 # sphinx-toolbox flake8==7.3.0 # via - # labthings-fastapi (pyproject.toml) # flake8-pyproject # flake8-rst # flake8-rst-docstrings + # labthings-fastapi (pyproject.toml) # pydoclint flake8-pyproject==1.2.4 # via labthings-fastapi (pyproject.toml) @@ -119,9 +127,9 @@ httptools==0.8.0 # via uvicorn httpx==0.28.1 # via - # labthings-fastapi (pyproject.toml) # fastapi # fastapi-cloud-cli + # labthings-fastapi (pyproject.toml) idna==3.16 # via # anyio @@ -141,6 +149,7 @@ itsdangerous==2.2.0 jinja2==3.1.6 # via # fastapi + # myst-parser # sphinx # sphinx-autoapi # sphinx-jinja2-compat @@ -153,13 +162,18 @@ jsonschema-specifications==2025.9.1 librt==0.11.0 # via mypy markdown-it-py==4.2.0 - # via rich + # via + # mdit-py-plugins + # myst-parser + # rich markupsafe==3.0.3 # via # jinja2 # sphinx-jinja2-compat mccabe==0.7.0 # via flake8 +mdit-py-plugins==0.6.1 + # via myst-parser mdurl==0.1.2 # via markdown-it-py msgpack==1.1.2 @@ -168,6 +182,8 @@ mypy==1.20.2 # via labthings-fastapi (pyproject.toml) mypy-extensions==1.1.0 # via mypy +myst-parser==5.1.0 + # via labthings-fastapi (pyproject.toml) natsort==8.4.0 # via domdf-python-tools numpy==2.2.6 @@ -188,11 +204,11 @@ pluggy==1.6.0 # pytest-cov pycodestyle==2.14.0 # via flake8 -pydantic==2.13.4 +pydantic[email]==2.13.4 # via - # labthings-fastapi (pyproject.toml) # fastapi # fastapi-cloud-cli + # labthings-fastapi (pyproject.toml) # pydantic-extra-types # pydantic-settings pydantic-core==2.46.4 @@ -201,7 +217,7 @@ pydantic-extra-types==2.11.1 # via fastapi pydantic-settings==2.14.1 # via fastapi -pydoclint==0.8.4 +pydoclint[flake8]==0.8.4 # via labthings-fastapi (pyproject.toml) pyflakes==3.4.0 # via flake8 @@ -231,6 +247,7 @@ python-multipart==0.0.29 pyyaml==6.0.3 # via # fastapi + # myst-parser # sphinx-autoapi # uvicorn referencing==0.37.0 @@ -277,8 +294,9 @@ soupsieve==2.8.4 # via beautifulsoup4 sphinx==8.1.3 # via - # labthings-fastapi (pyproject.toml) # autodocsumm + # labthings-fastapi (pyproject.toml) + # myst-parser # sphinx-autoapi # sphinx-autodoc-typehints # sphinx-prompt @@ -316,6 +334,8 @@ sphinxcontrib-serializinghtml==2.0.0 # via sphinx sphobjinv==2.4 # via labthings-fastapi (pyproject.toml) +standard-imghdr==3.10.14 + # via sphinx-jinja2-compat starlette==1.1.0 # via fastapi tabulate==0.10.0 @@ -323,15 +343,7 @@ tabulate==0.10.0 tinycss2==1.5.1 # via dict2css tomli==2.4.1 - # via - # labthings-fastapi (pyproject.toml) - # coverage - # fastapi-cli - # flake8-pyproject - # mypy - # pydoclint - # pytest - # sphinx + # via labthings-fastapi (pyproject.toml) typer==0.26.1 # via # fastapi-cli @@ -340,23 +352,17 @@ types-jsonschema==4.26.0.20260518 # via labthings-fastapi (pyproject.toml) typing-extensions==4.15.0 # via - # labthings-fastapi (pyproject.toml) - # anyio - # astroid # beautifulsoup4 # domdf-python-tools - # exceptiongroup # fastapi + # labthings-fastapi (pyproject.toml) # mypy # pydantic # pydantic-core # pydantic-extra-types - # referencing # rich-toolkit # sphinx-toolbox - # starlette # typing-inspection - # uvicorn typing-inspection==0.4.2 # via # fastapi @@ -367,13 +373,11 @@ urllib3==2.7.0 # requests # sentry-sdk # sphinx-prompt -uvicorn==0.48.0 +uvicorn[standard]==0.48.0 # via # fastapi # fastapi-cli # fastapi-cloud-cli -uvloop==0.22.1 - # via uvicorn watchfiles==1.2.0 # via uvicorn webencodings==0.5.1 From 657f42d008655974517c8a33b2d07f5fa687eb60 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 16:11:02 +0100 Subject: [PATCH 06/18] Cap myst-parser version --- dev-requirements.txt | 5 +++-- pyproject.toml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index cbec2a38..9768c052 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -27,6 +27,7 @@ astroid==4.1.2 attrs==26.1.0 # via # jsonschema + # myst-parser # referencing # sphobjinv autodocsumm==0.2.15 @@ -161,7 +162,7 @@ jsonschema-specifications==2025.9.1 # via jsonschema librt==0.11.0 # via mypy -markdown-it-py==4.2.0 +markdown-it-py==3.0.0 # via # mdit-py-plugins # myst-parser @@ -182,7 +183,7 @@ mypy==1.20.2 # via labthings-fastapi (pyproject.toml) mypy-extensions==1.1.0 # via mypy -myst-parser==5.1.0 +myst-parser==4.0.1 # via labthings-fastapi (pyproject.toml) natsort==8.4.0 # via domdf-python-tools diff --git a/pyproject.toml b/pyproject.toml index 84e28e48..25be8fbc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,7 @@ dev = [ "sphinx-autoapi", "sphinx-toolbox", "sphobjinv", - "myst-parser", + "myst-parser<5", "tomli; python_version < '3.11'", "codespell", ] From d835ef58bc4adc88428301d5a87544b9df68c4b5 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 16:14:41 +0100 Subject: [PATCH 07/18] Fix pip audit issues --- dev-requirements.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 9768c052..bab342d7 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -27,7 +27,6 @@ astroid==4.1.2 attrs==26.1.0 # via # jsonschema - # myst-parser # referencing # sphobjinv autodocsumm==0.2.15 @@ -243,7 +242,7 @@ python-dotenv==1.2.2 # via # pydantic-settings # uvicorn -python-multipart==0.0.29 +python-multipart==0.0.32 # via fastapi pyyaml==6.0.3 # via @@ -337,7 +336,7 @@ sphobjinv==2.4 # via labthings-fastapi (pyproject.toml) standard-imghdr==3.10.14 # via sphinx-jinja2-compat -starlette==1.1.0 +starlette==1.3.1 # via fastapi tabulate==0.10.0 # via sphinx-toolbox From cce1d79a7dc482ba769379312c46f579200513cb Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 16:22:36 +0100 Subject: [PATCH 08/18] Create stub file --- docs/contributing.md | 1 + docs/source/index.rst | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 docs/contributing.md diff --git a/docs/contributing.md b/docs/contributing.md new file mode 100644 index 00000000..36617e2d --- /dev/null +++ b/docs/contributing.md @@ -0,0 +1 @@ +```{include} ../CONTRIBUTING.md diff --git a/docs/source/index.rst b/docs/source/index.rst index 6c720499..2d6c523f 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -5,7 +5,7 @@ Documentation for LabThings-FastAPI :maxdepth: 2 :caption: Contents: - CONTRIBUTING.md + contributing.md quickstart/quickstart.rst tutorial/index.rst structure.rst From c24ae416a1d0fe73b57c53bcec55fef723d30a6d Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 16:29:54 +0100 Subject: [PATCH 09/18] Fix file location --- docs/contributing.md | 1 - docs/source/contributing.md | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 docs/contributing.md create mode 100644 docs/source/contributing.md diff --git a/docs/contributing.md b/docs/contributing.md deleted file mode 100644 index 36617e2d..00000000 --- a/docs/contributing.md +++ /dev/null @@ -1 +0,0 @@ -```{include} ../CONTRIBUTING.md diff --git a/docs/source/contributing.md b/docs/source/contributing.md new file mode 100644 index 00000000..b2d2646a --- /dev/null +++ b/docs/source/contributing.md @@ -0,0 +1 @@ +```{include} ../../CONTRIBUTING.md From 06eece79ac08424d9c4e83c1a94315b9250cedfe Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Fri, 19 Jun 2026 17:43:50 +0100 Subject: [PATCH 10/18] Update contributing.md to tell users they need to fork the repo --- CONTRIBUTING.md | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 33c862d9..856a3d09 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,14 +55,19 @@ We use several tools to maintain code quality. All of these run in CI with [GitH flake8 src ``` +* **Spelling:** We use [`codespell`] to prevent common spelling mistakes in code and documentation. + ```bash + codespell . + ``` + * **Type Checking:** We use [`mypy`] for static type checking. ```bash - mypy src + mypy ``` -* **Testing:** We use [`pytest`] for our test suite. Ensure all tests pass locally. +* **Testing:** We use [`pytest`] for our test suite and test coverage. Ensure all tests pass locally. ```bash - pytest + pytest --cov=src ``` ### 3. Managing Dependencies @@ -77,12 +82,25 @@ uv pip compile --extra dev pyproject.toml --output-file dev-requirements.txt ### 4. Submitting a Pull Request (PR) -All changes to the codebase must go via pull requests. - -1. Create a new branch for your feature or bugfix (`git checkout -b feature-name`). -2. Commit your changes with clear, descriptive commit messages. -3. Push your branch to your fork or the main repository. -4. Open a Pull Request against the `main` branch. +All changes to the codebase must go via pull requests. Unless you are a core maintainer with write access, please use the standard fork-and-branch workflow: + +1. **Fork the repository** to your own GitHub account using the "Fork" button at the top of the repository page. +2. **Clone your fork** locally and set up the upstream remote: + ```bash + git clone https://github.com/YOUR-USERNAME/labthings-fastapi.git + cd labthings-fastapi + git remote add upstream https://github.com/labthings/labthings-fastapi.git + ``` +3. **Create a new branch** for your feature or bugfix: + ```bash + git checkout -b feature-name + ``` +4. **Commit your changes** with clear, descriptive commit messages. +5. **Push your branch** up to your fork: + ```bash + git push origin feature-name + ``` +6. **Open a Pull Request** against the `main` branch of the `labthings/labthings-fastapi` repository. **Pull Request Guidelines:** From 95de93727748386e9c31b07be2019c519b566c49 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 29 Jun 2026 15:31:23 +0100 Subject: [PATCH 11/18] Update Windows notes to be Windows with ARM specific --- README.md | 4 ++-- docs/source/index.rst | 4 ++-- docs/source/tutorial/installing_labthings.rst | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b0b30d3d..3cb7c009 100644 --- a/README.md +++ b/README.md @@ -16,9 +16,9 @@ See [readthedocs] for installation instructions that are automatically tested. Y `labthings-fastapi` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current `pydantic-core` dependencies. -> **Note: Windows Installations** +> **Note: Windows Installations on devices with ARM processors** > -> Installing on Windows requires Visual Studio with the **"Desktop development with C++"** workload enabled. This is necessary because `pydantic` relies on Rust, which in turn requires C++ build tools to compile. +> Installing on Windows devices with ARM processors requires Visual Studio with the **"Desktop development with C++"** workload enabled. This is necessary because `pydantic` relies on Rust, which in turn requires C++ build tools to compile. > > *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* diff --git a/docs/source/index.rst b/docs/source/index.rst index 2d6c523f..c84dc22e 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -54,9 +54,9 @@ Installation ``labthings-fastapi`` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current ``pydantic-core`` dependencies. .. note:: - **Windows Installations** + **Windows Installations on devices with ARM processors** - Installing on Windows requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. + Installing on Windows devices with ARM processors requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* diff --git a/docs/source/tutorial/installing_labthings.rst b/docs/source/tutorial/installing_labthings.rst index d6d30c0e..ec1dbf8c 100644 --- a/docs/source/tutorial/installing_labthings.rst +++ b/docs/source/tutorial/installing_labthings.rst @@ -32,9 +32,9 @@ Installation Notes ``labthings-fastapi`` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current ``pydantic-core`` dependencies. .. note:: - **Windows Installations** + **Windows Installations on devices with ARM Processors** - Installing on Windows requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. + Installing on Windows devices with ARM processors requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* From 2dac89b9c4be9f08940c48b21bbac1308965c74e Mon Sep 17 00:00:00 2001 From: Beth <167304066+bprobert97@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:32:40 +0100 Subject: [PATCH 12/18] Update CONTRIBUTING.md Co-authored-by: Richard Bowman --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 856a3d09..44613037 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -105,7 +105,7 @@ All changes to the codebase must go via pull requests. Unless you are a core mai **Pull Request Guidelines:** * Code should only be merged once all the checks in the CI test job are passing. -* **Unpinned Dependencies:** Note that we have a specific CI job called `test-with-unpinned-dependencies`. It is acceptable to merge code if only this specific job fails, provided the failure is due to upstream dependency issues. We prefer to deal with upstream dependency issues in a separate PR, particularly when the required fixes are distinct from the code in your current PR. +* **Unpinned Dependencies:** Note that we have a specific CI job called `test-with-unpinned-dependencies`. It is acceptable to merge code if only this specific job fails, provided the failure is due to upstream dependency issues. We prefer to deal with upstream dependency issues in a separate PR, particularly when the required fixes are distinct from the code in your current PR. The same applies to the `pip-audit` job. * Update documentation (`docs/` or docstrings) if your changes modify existing behavior or add new features. [readthedocs]: https://labthings-fastapi.readthedocs.io/ From 485e4d739f94b82b22e3e804370ee8a844d9b80d Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 29 Jun 2026 15:36:57 +0100 Subject: [PATCH 13/18] Make windows note a single source doc Fix slashes Add in more ticks Improve the docstring for `BlobModel` This revised docstring should now make sense as the description of the `Blob` schema in the HTTP API. This isn't quite as comprehensive as passing through the description of a custom `Blob` subclass, but I think it's enough to close off #357. Fixes #357 Add a media_type argument to `Blob` This allows a more specific media type to be specified when creating a `Blob` instance. This would allow `Blob.from_bytes` to set any media type, or would allow a `Blob` subclass where the first part of the media type is specified (e.g. `image/*`) to have any media type starting with `image/`. This is tested in `test_blob_output.py` Improve exception docstring Fix D421 failures Ruff now complains if property docstrings start with a verb, and I started lots of them with "return". This is now fixed. https://docs.astral.sh/ruff/rules/property-docstring-starts-with-verb/ Fix ASYNC119 failure This gets rid of a linter error relating to async context manager clenup. The `buffer_for_reading` function was intended to leave open the possibility of better management of the "ringbuffer" used by the `MJPEGStream`. In fact, this doesn't provide any extra functionality, and isn't even a real ringbuffer. I've removed the problematic context manager as it doesn't do anything - there's already a function that provides what's needed in a way that Ruff is happy with. Use a `requirements.in` so `-e .` appears in our lockfile. This means I don't constantly have to reinstall labthings_fastapi after doing a `pip sync`. This commit does not change any package versions. Update pinned packages This should fix the `pip_audit` failure about `starlette` and also updates `ruff` and various other packages. Add errors to invocations as ProblemDetails. The Web Thing specification (on which our implementation of invocation serialisation is based) has an `error` key following RFC 9457. This adds code to serialise errors form invocations in that format. This should hopefully reduce the need to parse logs in order to give a helpful error in the python client. It also paves the way for keeping some stuff out of the log, like actions failing to start because the global lock was busy. Return a `None` URL for unrecognised exceptions. This will cover any exception that's neither a builtin nor a LabThings exception. I have also added unit testing for problem_details in addition to using it in other tests. More explicit test for status code I've added a test case with a non-default status code. Make use of ProblemDetails when handling errors. `ThingClient` now looks for the `error` field on invocations, and will use it to raise helpful error messages. This is tested fairly thoroughly, and includes tests where the field is missing, to encourage backwards-compatibility. Fix a failing test The change in exceptions raised by client code broke a test of the global lock. This is now fixed. Cannot have single source with markdown --- dev-requirements.in | 1 + dev-requirements.txt | 60 +++--- docs/source/_windows_arm_note.rst | 6 + docs/source/index.rst | 7 +- docs/source/tutorial/installing_labthings.rst | 8 +- src/labthings_fastapi/actions.py | 22 ++- src/labthings_fastapi/base_descriptor.py | 4 +- src/labthings_fastapi/client/__init__.py | 62 +++++-- src/labthings_fastapi/exceptions.py | 13 ++ src/labthings_fastapi/invocations.py | 2 + src/labthings_fastapi/outputs/blob.py | 64 ++++++- src/labthings_fastapi/outputs/mjpeg_stream.py | 50 +---- src/labthings_fastapi/problem_details.py | 63 +++++++ src/labthings_fastapi/server/__init__.py | 2 +- src/labthings_fastapi/testing.py | 4 +- src/labthings_fastapi/thing.py | 2 +- tests/test_action_logging.py | 24 +++ tests/test_action_manager.py | 2 +- tests/test_blob_output.py | 85 +++++++-- tests/test_global_lock.py | 7 +- tests/test_problem_details.py | 45 +++++ tests/test_thing_client_errors.py | 172 ++++++++++++++++++ 22 files changed, 572 insertions(+), 133 deletions(-) create mode 100644 dev-requirements.in create mode 100644 docs/source/_windows_arm_note.rst create mode 100644 src/labthings_fastapi/problem_details.py create mode 100644 tests/test_problem_details.py create mode 100644 tests/test_thing_client_errors.py diff --git a/dev-requirements.in b/dev-requirements.in new file mode 100644 index 00000000..aefbcb6d --- /dev/null +++ b/dev-requirements.in @@ -0,0 +1 @@ +-e .[dev] diff --git a/dev-requirements.txt b/dev-requirements.txt index bab342d7..42a3ba85 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -12,7 +12,7 @@ annotated-doc==0.0.4 # typer annotated-types==0.7.0 # via pydantic -anyio==4.13.0 +anyio==4.14.0 # via # httpx # labthings-fastapi (pyproject.toml) @@ -33,11 +33,11 @@ autodocsumm==0.2.15 # via sphinx-toolbox babel==2.18.0 # via sphinx -beautifulsoup4==4.14.3 +beautifulsoup4==4.15.0 # via sphinx-toolbox cachecontrol[filecache]==0.14.4 # via sphinx-toolbox -certifi==2026.5.20 +certifi==2026.6.17 # via # httpcore # httpx @@ -94,11 +94,11 @@ fastapi[all]==0.135.4 # via labthings-fastapi (pyproject.toml) fastapi-cli[standard]==0.0.24 # via fastapi -fastapi-cloud-cli==0.18.0 +fastapi-cloud-cli==0.20.0 # via fastapi-cli fastar==0.11.0 # via fastapi-cloud-cli -filelock==3.29.0 +filelock==3.29.4 # via # cachecontrol # sphinx-toolbox @@ -110,11 +110,11 @@ flake8==7.3.0 # labthings-fastapi (pyproject.toml) # pydoclint flake8-pyproject==1.2.4 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi flake8-rst==0.8.0 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi flake8-rst-docstrings==0.4.0 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi h11==0.16.0 # via # httpcore @@ -155,7 +155,7 @@ jinja2==3.1.6 # sphinx-jinja2-compat jsonschema==4.26.0 # via - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # sphobjinv jsonschema-specifications==2025.9.1 # via jsonschema @@ -176,10 +176,10 @@ mdit-py-plugins==0.6.1 # via myst-parser mdurl==0.1.2 # via markdown-it-py -msgpack==1.1.2 +msgpack==1.2.1 # via cachecontrol mypy==1.20.2 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi mypy-extensions==1.1.0 # via mypy myst-parser==4.0.1 @@ -187,7 +187,7 @@ myst-parser==4.0.1 natsort==8.4.0 # via domdf-python-tools numpy==2.2.6 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi packaging==26.2 # via # pytest @@ -195,8 +195,8 @@ packaging==26.2 pathspec==1.1.1 # via mypy pillow==12.2.0 - # via labthings-fastapi (pyproject.toml) -platformdirs==4.9.6 + # via labthings-fastapi +platformdirs==4.10.0 # via apeye pluggy==1.6.0 # via @@ -215,7 +215,7 @@ pydantic-core==2.46.4 # via pydantic pydantic-extra-types==2.11.1 # via fastapi -pydantic-settings==2.14.1 +pydantic-settings==2.14.2 # via fastapi pydoclint[flake8]==0.8.4 # via labthings-fastapi (pyproject.toml) @@ -229,15 +229,15 @@ pygments==2.20.0 # sphinx # sphinx-prompt # sphinx-tabs -pytest==9.0.3 +pytest==9.1.1 # via - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # pytest-cov # pytest-mock pytest-cov==7.1.0 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi pytest-mock==3.15.1 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi python-dotenv==1.2.2 # via # pydantic-settings @@ -266,7 +266,7 @@ rich==15.0.0 # via # rich-toolkit # typer -rich-toolkit==0.19.10 +rich-toolkit==0.20.1 # via # fastapi-cli # fastapi-cloud-cli @@ -280,15 +280,15 @@ rpds-py==0.30.0 # referencing ruamel-yaml==0.19.1 # via sphinx-toolbox -ruff==0.15.14 - # via labthings-fastapi (pyproject.toml) -sentry-sdk==2.60.0 +ruff==0.15.18 + # via labthings-fastapi +sentry-sdk==2.63.0 # via fastapi-cloud-cli shellingham==1.5.4 # via typer six==1.17.0 # via html5lib -snowballstemmer==3.1.0 +snowballstemmer==3.1.1 # via sphinx soupsieve==2.8.4 # via beautifulsoup4 @@ -305,7 +305,7 @@ sphinx==8.1.3 # sphinx-toolbox # sphinxcontrib-jquery sphinx-autoapi==3.8.0 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi sphinx-autodoc-typehints==3.0.1 # via sphinx-toolbox sphinx-jinja2-compat==0.4.1 @@ -313,11 +313,11 @@ sphinx-jinja2-compat==0.4.1 sphinx-prompt==1.9.0 # via sphinx-toolbox sphinx-rtd-theme==3.1.0 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi sphinx-tabs==3.5.0 # via sphinx-toolbox sphinx-toolbox==4.2.0 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi sphinxcontrib-applehelp==2.0.0 # via sphinx sphinxcontrib-devhelp==2.0.0 @@ -349,7 +349,7 @@ typer==0.26.1 # fastapi-cli # fastapi-cloud-cli types-jsonschema==4.26.0.20260518 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi typing-extensions==4.15.0 # via # beautifulsoup4 @@ -386,5 +386,5 @@ webencodings==0.5.1 # tinycss2 websockets==16.0 # via uvicorn -zeroconf==0.149.16 - # via labthings-fastapi (pyproject.toml) +zeroconf==0.150.0 + # via labthings-fastapi diff --git a/docs/source/_windows_arm_note.rst b/docs/source/_windows_arm_note.rst new file mode 100644 index 00000000..9fb2c390 --- /dev/null +++ b/docs/source/_windows_arm_note.rst @@ -0,0 +1,6 @@ +.. note:: + **Windows Installations on devices with ARM Processors** + + Installing on Windows devices with ARM processors requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. + + *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* \ No newline at end of file diff --git a/docs/source/index.rst b/docs/source/index.rst index c84dc22e..6ddf88a6 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -53,12 +53,7 @@ Installation ``labthings-fastapi`` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current ``pydantic-core`` dependencies. -.. note:: - **Windows Installations on devices with ARM processors** - - Installing on Windows devices with ARM processors requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. - - *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* +.. include:: _windows_arm_note.rst Indices and tables ================== diff --git a/docs/source/tutorial/installing_labthings.rst b/docs/source/tutorial/installing_labthings.rst index ec1dbf8c..d73d9a01 100644 --- a/docs/source/tutorial/installing_labthings.rst +++ b/docs/source/tutorial/installing_labthings.rst @@ -31,13 +31,7 @@ Installation Notes ``labthings-fastapi`` supports **Python 3.10, 3.11, 3.12, and 3.13**. The upper limit is strictly capped at 3.13 due to current ``pydantic-core`` dependencies. -.. note:: - **Windows Installations on devices with ARM Processors** - - Installing on Windows devices with ARM processors requires `Visual Studio`_ with the **"Desktop development with C++"** workload enabled. This is necessary because ``pydantic`` relies on Rust_, which in turn requires C++ build tools to compile. - - *If you are using a centrally managed machine, you will need administrator privileges to install these system-level dependencies.* - +.. include:: ../_windows_arm_note.rst .. _PyPI: https://pypi.org/project/labthings-fastapi/ .. _Visual Studio: https://visualstudio.microsoft.com/ diff --git a/src/labthings_fastapi/actions.py b/src/labthings_fastapi/actions.py index 06c1e8eb..4c66ccb0 100644 --- a/src/labthings_fastapi/actions.py +++ b/src/labthings_fastapi/actions.py @@ -42,6 +42,7 @@ from pydantic import BaseModel, create_model from labthings_fastapi.message_broker import Message +from labthings_fastapi.problem_details import ProblemDetails from .middleware.url_for import URLFor @@ -153,7 +154,9 @@ def __init__( self._request_time: datetime.datetime = datetime.datetime.now() self._start_time: Optional[datetime.datetime] = None # Task start time self._end_time: Optional[datetime.datetime] = None # Task end time - self._exception: Optional[Exception] = None # Propagate exceptions helpfully + self._exception: Optional[BaseException] = ( + None # Propagate exceptions helpfully + ) self._log: deque = deque(maxlen=log_len) # log entries for this thread @property @@ -163,13 +166,13 @@ def id(self) -> uuid.UUID: @property def output(self) -> Any: - """Return value of the Action. If the Action is still running, returns None.""" + """The Action's output (return value) or `None` if it's still running.""" with self._status_lock: return RootModelWrapper.unwrap(self._output_model_instance) @property def output_model_instance(self) -> BaseModel | None: - """Return value of the Action, as a model, or None.""" + """The Action's output (return value), as a model, or None.""" with self._status_lock: return self._output_model_instance @@ -188,6 +191,15 @@ def status(self) -> InvocationStatus: with self._status_lock: return self._status + @property + def error_model(self) -> ProblemDetails | None: + """A description of the error, if an error has happened.""" + with self._status_lock: + if not self._exception: + return None + else: + return ProblemDetails.from_exception(self._exception) + @property def action(self) -> ActionDescriptor: """The `.ActionDescriptor` object running in this thread. @@ -269,6 +281,7 @@ def response(self) -> InvocationModel: input=self.input, output=self.output_model_instance, log=self.log, + error=self.error_model, ) def _publish_status(self) -> None: @@ -352,10 +365,11 @@ def run(self) -> None: self._output_model_instance = output_model_instance self._status = InvocationStatus.COMPLETED self._publish_status() - except InvocationCancelledError: + except InvocationCancelledError as e: logger.info(f"Invocation {self.id} was cancelled.") with self._status_lock: self._status = InvocationStatus.CANCELLED + self._exception = e self._publish_status() except Exception as e: # skipcq: PYL-W0703 # First log diff --git a/src/labthings_fastapi/base_descriptor.py b/src/labthings_fastapi/base_descriptor.py index fad45929..64762593 100644 --- a/src/labthings_fastapi/base_descriptor.py +++ b/src/labthings_fastapi/base_descriptor.py @@ -101,12 +101,12 @@ def __init__(self, obj: Owner | None, cls: type[Owner] | None = None) -> None: @property def owning_class(self) -> type[Owner]: - """Retrieve the class this info object is describing.""" + """The class this info object is describing.""" return self._descriptor_cls @property def owning_object(self) -> Owner | None: - """Retrieve the object to which this info object is bound, if present.""" + """The object to which this info object is bound, if present.""" return self._bound_to_obj @property diff --git a/src/labthings_fastapi/client/__init__.py b/src/labthings_fastapi/client/__init__.py index 566852c5..74305020 100644 --- a/src/labthings_fastapi/client/__init__.py +++ b/src/labthings_fastapi/client/__init__.py @@ -13,14 +13,17 @@ import httpx from urllib.parse import urlparse, urljoin -from pydantic import BaseModel, TypeAdapter +from pydantic import BaseModel, TypeAdapter, ValidationError from ..outputs.blob import Blob, RemoteBlobData from ..exceptions import ( FailedToInvokeActionError, + InvocationCancelledError, ServerActionError, ClientPropertyError, + GlobalLockBusyError, ) +from labthings_fastapi.problem_details import ProblemDetails, docs_url __all__ = ["ThingClient", "poll_invocation"] ACTION_RUNNING_KEYWORDS = ["idle", "pending", "running"] @@ -191,7 +194,7 @@ def set_property(self, path: str, value: Any) -> None: raise ClientPropertyError(f"Failed to set property {path}: {err_msg}") - def invoke_action(self, path: str, **kwargs: Any) -> Any: + def invoke_action(self, path: str, **kwargs: Any) -> Any: # noqa: DOC503 r"""Invoke an action on the Thing. This method will make the initial POST request to invoke an action, @@ -212,6 +215,8 @@ def invoke_action(self, path: str, **kwargs: Any) -> Any: :raise FailedToInvokeActionError: if the action fails to start. :raise ServerActionError: is raised if the action does not complete successfully. + :raise GlobalLockBusyError: if the action fails because of the global lock. + :raise InvocationCancelledError: if the action is cancelled. """ for k in kwargs.keys(): value = kwargs[k] @@ -243,8 +248,9 @@ def invoke_action(self, path: str, **kwargs: Any) -> Any: ) ) return invocation["output"] - message = _construct_invocation_error_message(invocation) - raise ServerActionError(message) + # Note that flake8 is confused by the error below - this is why we ignore + # error DOC503 for this function. + raise _invocation_error(invocation) def follow_link(self, response: dict, rel: str) -> httpx.Response: """Follow a link in a response object, by its `rel` attribute. @@ -474,7 +480,7 @@ def _construct_failed_to_invoke_message(path: str, response: httpx.Response) -> return message -def _construct_invocation_error_message(invocation: Mapping[str, Any]) -> str: +def _invocation_error(invocation: Mapping[str, Any]) -> BaseException: """Format an error for ThingClient to raise if an invocation ends in and error. :param invocation: The invocation dictionary returned. @@ -484,19 +490,49 @@ def _construct_invocation_error_message(invocation: Mapping[str, Any]) -> str: action_name = invocation["action"].split("/")[-1] err_message = "Unknown error" - - if len(invocation.get("log", [])) > 0: - last_log = invocation["log"][-1] + log = invocation.get("log", []) + last_log = log[-1] if log else None + traceback: str | None = None + + # If there's a log item, use the traceback and possibly the message. + # we don't currently check if this message actually is an error. + # We'll overwrite the message later, if "error" is populated. + if last_log: err_message = last_log.get("message", err_message) exception_type = last_log.get("exception_type") if exception_type is not None: err_message = f"[{exception_type}]: {err_message}" + # If there's a traceback, put it i traceback = last_log.get("traceback") - if traceback is not None: - err_message += "\n\nSERVER TRACEBACK START:\n\n" - err_message += traceback - err_message += "\n\nSERVER TRACEBACK END\n\n" - return f"Action {action_name} (ID: {inv_id}) failed with error:\n{err_message}" + # If there's an error specified, use that in preference to the message + # extracted from the logs. + try: + pd = ProblemDetails.model_validate(invocation.get("error", None)) + if pd.type == docs_url(GlobalLockBusyError): + # GlobalLockBusyError is worth handling specially: it's likely to + # happen quite a bit, and "we couldn't start the action because + # we were busy" feels like it doesn't want to be lumped in with + # other failures. + return GlobalLockBusyError(pd.detail) + if pd.type == docs_url(InvocationCancelledError): + # Similarly, InvocationCancelledError means the action was cancelled, + # so it feels right to raise this instead + return InvocationCancelledError(pd.detail) + if pd.detail: + err_message = pd.detail + if pd.title: + err_message = f"[{pd.title}]: {err_message}" + except ValidationError: + pass # If it's not a valid problem details object, ignore it and move on. + + # Append the server traceback, if we have one. + if traceback is not None: + err_message += "\n\nSERVER TRACEBACK START:\n\n" + err_message += traceback + err_message += "\n\nSERVER TRACEBACK END\n\n" + return ServerActionError( + f"Action {action_name} (ID: {inv_id}) failed with error:\n{err_message}" + ) diff --git a/src/labthings_fastapi/exceptions.py b/src/labthings_fastapi/exceptions.py index d20239ba..4482a597 100644 --- a/src/labthings_fastapi/exceptions.py +++ b/src/labthings_fastapi/exceptions.py @@ -350,6 +350,17 @@ class NoBlobManagerError(RuntimeError): """ +class MediaTypeMismatchError(ValueError): + r"""Raised if a `Blob` is created with a media type that doesn't match its class. + + This error indicates that the ``media_type` specified when creating a `Blob` + instance doesn't match its host class. The base `~blob.Blob` class does not impose + any constraints. This error usually appears if a specific blob subclass is being + created, for example with ``BlobSubclass.from_bytes(data, media_type)`` and the + supplied ``media_type`` doesn't match the type defined in ``BlobSubclass``\ . + """ + + class NoUrlForContextError(RuntimeError): """Raised if URLFor is serialised without a url_for context variable being set. @@ -461,6 +472,8 @@ class GlobalLockBusyError(TimeoutError): property setter. """ + status_code = 409 + class MessageDroppedWarning(RuntimeWarning): """A message was dropped by the message broker. diff --git a/src/labthings_fastapi/invocations.py b/src/labthings_fastapi/invocations.py index c9512a1f..299228ed 100644 --- a/src/labthings_fastapi/invocations.py +++ b/src/labthings_fastapi/invocations.py @@ -18,6 +18,7 @@ ) from labthings_fastapi.middleware.url_for import URLFor +from labthings_fastapi.problem_details import ProblemDetails from .thing_description._model import Links @@ -120,6 +121,7 @@ class GenericInvocationModel(InvocationSummary, Generic[InputT, OutputT]): input: InputT output: OutputT log: Sequence[LogRecordModel] + error: ProblemDetails | None = None InvocationModel = GenericInvocationModel[Any, Any] diff --git a/src/labthings_fastapi/outputs/blob.py b/src/labthings_fastapi/outputs/blob.py index 2b358b35..cf5233a5 100644 --- a/src/labthings_fastapi/outputs/blob.py +++ b/src/labthings_fastapi/outputs/blob.py @@ -108,6 +108,7 @@ def get_image(self) -> MyImageBlob: from pydantic.json_schema import JsonSchemaValue from pydantic_core import core_schema from typing_extensions import Self +from labthings_fastapi.exceptions import MediaTypeMismatchError from labthings_fastapi.middleware.url_for import url_for @@ -469,7 +470,11 @@ class BlobModel(BaseModel): """A model for JSON-serialised `.Blob` objects. This model describes the JSON representation of a `.Blob` - and does not offer any useful functionality. + and is used to describe the `.Blob` object in JSON responses. + + The binary data may be retrieved with a ``GET`` request to the URL + specified in ``href`` which should return it directly in the body of + the response. """ href: str @@ -738,7 +743,7 @@ def data(self) -> BlobData: @property def content(self) -> bytes: - """Return the the output as a `bytes` object. + """The blob's content as a `bytes` object in memory. This property may return the `bytes` object, or if we have a file it will read the file and return the contents. Client objects may use @@ -774,7 +779,31 @@ def open(self) -> io.IOBase: return self.data.open() @classmethod - def from_bytes(cls, data: bytes) -> Self: + def validate_media_type(cls, media_type: str | None) -> str: + r"""Check a specified media type is valid, or use the class default. + + If a media type is specified when creating a `.Blob`, this function checks + that it's compatible with the class media type. The check is quite basic, + and is defined in `match_media_type`\ . + + If ``media_type`` is not compatible with the class, we raise an exception. + If `None` is supplied, we return the class media type. + + :param media_type: the media type to validate. + :return: a valid media type, or the class media type. + :raises MediaTypeMismatchError: if the media type doesn't match the class. + """ + if media_type is None: + return cls.media_type + if match_media_types(media_type, cls.media_type): + return media_type + raise MediaTypeMismatchError( + f"Can't create a {cls.__qualname__} as media type '{media_type}' " + f"doesn't match '{cls.media_type}'." + ) + + @classmethod + def from_bytes(cls, data: bytes, media_type: str | None = None) -> Self: """Create a `.Blob` from a bytes object. This is the recommended way to create a `.Blob` from data that is held @@ -782,13 +811,17 @@ def from_bytes(cls, data: bytes) -> Self: ``media_type``. :param data: the data as a `bytes` object. + :param media_type: the media type of the supplied data, defaults to + the ``media_type`` attribute of this class. :return: a `.Blob` wrapping the supplied data. """ - return cls(BlobBytes(data, media_type=cls.media_type)) + return cls(BlobBytes(data, media_type=cls.validate_media_type(media_type))) @classmethod - def from_temporary_directory(cls, folder: TemporaryDirectory, file: str) -> Self: + def from_temporary_directory( + cls, folder: TemporaryDirectory, file: str, media_type: str | None = None + ) -> Self: """Create a `.Blob` from a file in a temporary directory. This is the recommended way to create a `.Blob` from data that is @@ -803,6 +836,8 @@ def from_temporary_directory(cls, folder: TemporaryDirectory, file: str) -> Self :param folder: a `tempfile.TemporaryDirectory` where the file is saved. :param file: the path to the file, relative to the ``folder``. + :param media_type: the media type of the supplied data, defaults to + the ``media_type`` attribute of this class. :return: a `.Blob` wrapping the file. """ @@ -810,14 +845,14 @@ def from_temporary_directory(cls, folder: TemporaryDirectory, file: str) -> Self return cls( BlobFile( file_path, - media_type=cls.media_type, + media_type=cls.validate_media_type(media_type), # Prevent the temporary directory from being cleaned up _temporary_directory=folder, ), ) @classmethod - def from_file(cls, file: str) -> Self: + def from_file(cls, file: str, media_type: str | None = None) -> Self: """Create a `.Blob` from a regular file. This is the recommended way to create a `.Blob` from a file, if that @@ -832,15 +867,22 @@ def from_file(cls, file: str) -> Self: `.Blob` with `from_temporary_directory` instead. :param file: is the path to the file. This file must exist. + :param media_type: the media type of the supplied data, defaults to + the ``media_type`` attribute of this class. :return: a `.Blob` object referencing the specified file. """ return cls( - BlobFile(file, media_type=cls.media_type), + BlobFile(file, media_type=cls.validate_media_type(media_type)), ) @classmethod - def from_url(cls, href: str, client: httpx.Client | None = None) -> Self: + def from_url( + cls, + href: str, + client: httpx.Client | None = None, + media_type: str | None = None, + ) -> Self: """Create a `.Blob` that references data at a URL. This is the recommended way to create a `.Blob` that references @@ -850,12 +892,14 @@ def from_url(cls, href: str, client: httpx.Client | None = None) -> Self: :param href: the URL where the data may be downloaded. :param client: if supplied, this `httpx.Client` will be used to download the data. + :param media_type: the media type of the supplied data, defaults to + the ``media_type`` attribute of this class. :return: a `.Blob` object referencing the specified URL. """ return cls( RemoteBlobData( - media_type=cls.media_type, + media_type=cls.validate_media_type(media_type), href=href, client=client, ), diff --git a/src/labthings_fastapi/outputs/mjpeg_stream.py b/src/labthings_fastapi/outputs/mjpeg_stream.py index 2688fe21..16966a35 100644 --- a/src/labthings_fastapi/outputs/mjpeg_stream.py +++ b/src/labthings_fastapi/outputs/mjpeg_stream.py @@ -12,7 +12,6 @@ from typing import ( Any, AsyncGenerator, - AsyncIterator, Literal, Optional, TYPE_CHECKING, @@ -20,8 +19,6 @@ overload, ) from typing_extensions import Self -from copy import copy -from contextlib import asynccontextmanager import threading import anyio import logging @@ -203,36 +200,12 @@ async def ringbuffer_entry(self, i: int) -> RingbufferEntry: raise ValueError("the ith frame has been overwritten") return entry - @asynccontextmanager - async def buffer_for_reading(self, i: int) -> AsyncIterator[bytes]: - """Yield the ith frame as a bytes object. - - Retrieve frame ``i`` from the ringbuffer. - - This allows async code access to a frame in the ringbuffer. - The frame will not be copied, and should not be written to. - The frame may not exist after the function has completed (i.e. - after any ``with`` statement has finished). - - Using a context manager is intended to allow future versions of this - code to manage access to the ringbuffer (e.g. allowing buffer reuse). - Currently, buffers are always created as fresh `bytes` objects, so - this context manager does not provide additional functionality - over `.MJPEGStream.ringbuffer_entry`. - - :param i: The index of the frame to read - - :yield: The frame's data as `bytes`, along with timestamp and index. - """ - entry = await self.ringbuffer_entry(i) - yield entry.frame - async def next_frame(self) -> int: """Wait for the next frame, and return its index. This async function will yield until a new frame arrives, then return its index. The index may then be used to retrieve the new frame - with `.MJPEGStream.buffer_for_reading`. + with `.MJPEGStream.ringbuffer_entry`. :return: the index of the next frame to arrive. @@ -253,8 +226,8 @@ async def grab_frame(self) -> bytes: :return: The next JPEG frame, as a `bytes` object. """ i = await self.next_frame() - async with self.buffer_for_reading(i) as frame: - return copy(frame) + entry = await self.ringbuffer_entry(i) + return entry.frame async def next_frame_size(self) -> int: """Wait for the next frame and return its size. @@ -264,20 +237,13 @@ async def next_frame_size(self) -> int: :return: The size of the next JPEG frame, in bytes. """ i = await self.next_frame() - async with self.buffer_for_reading(i) as frame: - return len(frame) + entry = await self.ringbuffer_entry(i) + return len(entry.frame) async def frame_async_generator(self) -> AsyncGenerator[bytes, None]: """Yield frames as bytes objects. - This generator will return frames from the MJPEG stream. These are - taken from the ringbuffer by `.MJPEGStream.buffer_for_reading` and - so should have any buffer-management considerations taken care of. - - Code using this generator should complete as quickly as possible, - because future implementations may hold a lock while this function - yields. If lengthy processing is required, please copy the buffer - and continue processing elsewhere. + This generator will return frames from the MJPEG stream. Note that this will wait for a new frame each time. There is no guarantee that we won't skip frames. @@ -288,8 +254,8 @@ async def frame_async_generator(self) -> AsyncGenerator[bytes, None]: while self._streaming: try: i = await self.next_frame() - async with self.buffer_for_reading(i) as frame: - yield frame + entry = await self.ringbuffer_entry(i) + yield entry.frame except StopAsyncIteration: break except Exception as e: # noqa: BLE001 diff --git a/src/labthings_fastapi/problem_details.py b/src/labthings_fastapi/problem_details.py new file mode 100644 index 00000000..118b8bc4 --- /dev/null +++ b/src/labthings_fastapi/problem_details.py @@ -0,0 +1,63 @@ +"""JSON representations of errors as per RFC 9457. + +This module defines a model and supporting functions that help to create +"Problem Details" objects to represent errors in HTTP responses. +""" + +from typing_extensions import Self + +from pydantic import BaseModel, ConfigDict +from labthings_fastapi import exceptions + + +class ProblemDetails(BaseModel): + """A model to describe an error that occurred in an invocation.""" + + model_config = ConfigDict(extra="allow") + + detail: str | None = None + """A human-readable string with details of the error.""" + type: str | None = None + """A URI giving a unique reference for the error.""" + status: int | None = None + """An HTTP status code describing the error.""" + title: str | None = None + """A human-readable title for the error.""" + instance: str | None = None + """A URI that may or may not give details about this specific instance.""" + + @classmethod + def from_exception(cls, exc: BaseException) -> Self: + r"""Generate a `ProblemDetails` model from an exception instance. + + :param exc: the exception instance to be described. + :return: a `ProblemDetails` object describing ``exc``\ . + """ + return cls( + detail=str(exc), + type=docs_url(type(exc)), + title=exc.__class__.__name__, + status=getattr(exc, "status_code", 500), + ) + + +# This URL should describe all exceptions in this module. +DOCS_URL = ( + "https://labthings-fastapi.readthedocs.io/en/latest/" + "autoapi/labthings_fastapi/exceptions/index.html" +) +PYTHON_DOCS_URL = "https://docs.python.org/3/library/exceptions.html" + + +def docs_url(exc: type[BaseException]) -> str | None: + """Return a URL identifying a LabThings exception. + + :param exc: An exception class. + :return: a URL pointing to that exception in the docs, or + `None` if the exception is not in this module. + """ + if exc.__module__ == exceptions.__name__: + return f"{DOCS_URL}#{exc.__module__}.{exc.__qualname__}" + if exc.__module__ == "builtins": + return f"{PYTHON_DOCS_URL}#{exc.__name__}" + return None diff --git a/src/labthings_fastapi/server/__init__.py b/src/labthings_fastapi/server/__init__.py index d45e3fe2..05afbe29 100644 --- a/src/labthings_fastapi/server/__init__.py +++ b/src/labthings_fastapi/server/__init__.py @@ -291,7 +291,7 @@ def things(self) -> Mapping[str, Thing]: @property def application_config(self) -> Mapping[str, Any] | None: - """Return the application configuration from the config file. + """The application configuration from the config file. :return: The custom configuration as specified in the configuration file. diff --git a/src/labthings_fastapi/testing.py b/src/labthings_fastapi/testing.py index da890973..c95fa3fd 100644 --- a/src/labthings_fastapi/testing.py +++ b/src/labthings_fastapi/testing.py @@ -153,7 +153,7 @@ def _action_manager(self) -> ActionManager: @property def application_config(self) -> None: - """Return an empty application configuration when mocking. + """An empty application configuration when mocking. :return: None """ @@ -161,7 +161,7 @@ def application_config(self) -> None: @property def global_lock(self) -> GlobalLock | None: - """Return a global lock.""" + """A global lock.""" return self._global_lock diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py index fb945505..b3a06a93 100644 --- a/src/labthings_fastapi/thing.py +++ b/src/labthings_fastapi/thing.py @@ -338,7 +338,7 @@ def save_settings(self) -> None: @property def thing_state(self) -> Mapping: - """Return a dictionary summarising our current state. + """A dictionary summarising our current state. This is intended to be an easy way to collect metadata from a Thing that summarises its state. It might be used, for example, to record metadata diff --git a/tests/test_action_logging.py b/tests/test_action_logging.py index 3bf4eb9a..f929fabd 100644 --- a/tests/test_action_logging.py +++ b/tests/test_action_logging.py @@ -65,6 +65,16 @@ def test_unhandled_error_logs(caplog, client): assert caplog.records[0].levelname == "ERROR" # There is a traceback assert caplog.records[0].exc_info is not None + # Check the "error" property is populated correctly. + problem_details = invocation["error"] + assert isinstance(problem_details, dict) + assert problem_details == { + "type": "https://docs.python.org/3/library/exceptions.html#RuntimeError", + "detail": "I was asked to raise this error.", + "title": "RuntimeError", + "status": 500, # this is the default status + "instance": None, + } def test_invocation_error_logs(caplog, client): @@ -78,6 +88,20 @@ def test_invocation_error_logs(caplog, client): assert caplog.records[0].levelname == "ERROR" # There is not a traceback assert caplog.records[0].exc_info is None + # Check the "error" property is populated correctly. + problem_details = invocation["error"] + assert isinstance(problem_details, dict) + assert problem_details == { + "type": ( + "https://labthings-fastapi.readthedocs.io/en/latest/autoapi/" + "labthings_fastapi/exceptions/index.html#labthings_fastapi." + "exceptions.InvocationError" + ), + "detail": "This is an error, but I handled it!", + "title": "InvocationError", + "status": 500, # this is the default status + "instance": None, + } def test_logrecordmodel(): diff --git a/tests/test_action_manager.py b/tests/test_action_manager.py index 4b0603b3..f851616d 100644 --- a/tests/test_action_manager.py +++ b/tests/test_action_manager.py @@ -75,6 +75,6 @@ def test_actions_list(client): r2.raise_for_status() invocations = r2.json() # Some keys aren't present in the list for performance/safety reasons - for k in ["input", "output", "log"]: + for k in ["input", "output", "log", "error"]: del invocation[k] assert invocations == [invocation] diff --git a/tests/test_blob_output.py b/tests/test_blob_output.py index 830bbe84..07bd5573 100644 --- a/tests/test_blob_output.py +++ b/tests/test_blob_output.py @@ -12,7 +12,10 @@ from pydantic_core import PydanticSerializationError import pytest import labthings_fastapi as lt -from labthings_fastapi.exceptions import FailedToInvokeActionError +from labthings_fastapi.exceptions import ( + FailedToInvokeActionError, + MediaTypeMismatchError, +) from labthings_fastapi.testing import create_thing_without_server, use_dummy_url_for @@ -117,25 +120,83 @@ def test_invalid_media_type_parsing(media_type, msg): with pytest.raises(ValueError, match=msg): lt.blob.parse_media_type(media_type) + # This error should also appear when we create a Blob + with pytest.raises(ValueError, match=msg): + _ = lt.blob.Blob.from_bytes(b"", media_type=media_type) + + +MEDIA_TYPES_FOR_MATCHING = [ + ("text/plain", "text/plain", True), + ("text/html", "text/*", True), + ("image/png", "image/*", True), + ("application/json", "*/*", True), + ("text/plain", "text/html", False), + ("image/jpeg", "image/png", False), + ("application/xml", "application/json", False), + ("text/plain", "image/*", False), +] + @pytest.mark.parametrize( ("data_media_type", "blob_media_type", "expected"), - [ - ("text/plain", "text/plain", True), - ("text/html", "text/*", True), - ("image/png", "image/*", True), - ("application/json", "*/*", True), - ("text/plain", "text/html", False), - ("image/jpeg", "image/png", False), - ("application/xml", "application/json", False), - ("text/plain", "image/*", False), - ], + MEDIA_TYPES_FOR_MATCHING, ) -def test_media_type_matching(data_media_type, blob_media_type, expected): +def test_media_type_matching(data_media_type, blob_media_type, expected, mocker): """Check that media type matching works as expected.""" assert lt.blob.match_media_types(data_media_type, blob_media_type) is expected +@pytest.mark.parametrize( + ("data_media_type", "blob_media_type", "expected"), + MEDIA_TYPES_FOR_MATCHING, +) +def test_validate_media_type(data_media_type, blob_media_type, expected): + """Check that the data type validator class method on Blob works correctly.""" + + class BlobSubclass(lt.blob.Blob): + media_type: str = blob_media_type + + if expected: + assert BlobSubclass.validate_media_type(data_media_type) == data_media_type + else: + with pytest.raises(MediaTypeMismatchError): + BlobSubclass.validate_media_type(data_media_type) + + +@pytest.mark.parametrize( + ("data_media_type", "blob_media_type", "expected"), + MEDIA_TYPES_FOR_MATCHING, +) +def test_media_type_validated(data_media_type, blob_media_type, expected, mocker): + """Check that the class methods used to create a Blob validate the media type.""" + + class BlobSubclass(lt.blob.Blob): + media_type: str = blob_media_type + + if not expected: + tmpdir = mocker.Mock(spec=TemporaryDirectory) + tmpdir.name = "folder/path" + + # The media type should be checked when creating a blob with class methods + with pytest.raises(MediaTypeMismatchError): + BlobSubclass.from_bytes(b"", media_type=data_media_type) + with pytest.raises(MediaTypeMismatchError): + BlobSubclass.from_file("file/path", media_type=data_media_type) + with pytest.raises(MediaTypeMismatchError): + BlobSubclass.from_temporary_directory( + tmpdir, + "file/path", + media_type=data_media_type, + ) + with pytest.raises(MediaTypeMismatchError): + BlobSubclass.from_url("https://whatever/", media_type=data_media_type) + else: + # If the media types match, the instance produced should have the media type + # of the + blob = BlobSubclass.from_bytes(b"", media_type=data_media_type) + assert blob.media_type == data_media_type + + def test_blobdata_base_class(): """Check that BlobData/LocalBlobData abstract methods raise the right error.""" bd = lt.blob.BlobData("*/*") diff --git a/tests/test_global_lock.py b/tests/test_global_lock.py index 52142ab4..2de82726 100644 --- a/tests/test_global_lock.py +++ b/tests/test_global_lock.py @@ -494,7 +494,10 @@ def test_global_lock_log(caplog): # This should fail with a message about the global # lock, but no traceback. caplog.clear() - with pytest.raises(ServerActionError, match="Global lock was busy"): + with pytest.raises( + GlobalLockBusyError, + match="The global lock could not be acquired", + ): checker.increment_fprop2() matches = [r for r in caplog.records if "Global lock was busy" in r.message] assert len(matches) == 1 @@ -506,6 +509,6 @@ def test_global_lock_log(caplog): # does. This should print a stack trace, as the # exception is not handled. caplog.clear() - with pytest.raises(ServerActionError, match="GlobalLockBusyError"): + with pytest.raises(GlobalLockBusyError): checker.increment_prop1() assert "Traceback" in caplog.text diff --git a/tests/test_problem_details.py b/tests/test_problem_details.py new file mode 100644 index 00000000..1c4598db --- /dev/null +++ b/tests/test_problem_details.py @@ -0,0 +1,45 @@ +"""Test we can generate problem details to describe exceptions nicely.""" + +import pytest + +from labthings_fastapi import problem_details, exceptions + +DOCS_URL = ( + "https://labthings-fastapi.readthedocs.io/en/latest/" + "autoapi/labthings_fastapi/exceptions/index.html" + "#labthings_fastapi.exceptions." +) +PYTHON_DOCS = "https://docs.python.org/3/library/exceptions.html" + + +class CustomError(Exception): + """A custom exception.""" + + +ERRORS = [ + # Some standard-library exceptions + (BaseException, f"{PYTHON_DOCS}#BaseException", 500), + (Exception, f"{PYTHON_DOCS}#Exception", 500), + (RuntimeError, f"{PYTHON_DOCS}#RuntimeError", 500), + # Some LabThings exceptions + (exceptions.FeatureNotAvailableError, f"{DOCS_URL}FeatureNotAvailableError", 500), + (exceptions.InvocationCancelledError, f"{DOCS_URL}InvocationCancelledError", 500), + (exceptions.GlobalLockBusyError, f"{DOCS_URL}GlobalLockBusyError", 409), + # Custom exceptions should be None + (CustomError, None, 500), +] + + +@pytest.mark.parametrize(("err", "url", "_code"), ERRORS) +def test_docs_url(err, url, _code): + """Check URLs for built-in and LabThings errors, and that we get None for others.""" + assert problem_details.docs_url(err) == url + + +@pytest.mark.parametrize(("err", "url", "code"), ERRORS) +def test_pd_from_exception(err, url, code): + pd = problem_details.ProblemDetails.from_exception(err("Message")) + assert pd.type == url + assert pd.detail == "Message" + assert pd.title == err.__name__ + assert pd.status == code diff --git a/tests/test_thing_client_errors.py b/tests/test_thing_client_errors.py new file mode 100644 index 00000000..a3e9f821 --- /dev/null +++ b/tests/test_thing_client_errors.py @@ -0,0 +1,172 @@ +"""Test how errors in actions and properties are reported to the client.""" + +from collections.abc import Iterator +import threading + +import pytest + +import labthings_fastapi as lt +from labthings_fastapi.exceptions import ( + GlobalLockBusyError, + InvocationCancelledError, + ServerActionError, +) +from labthings_fastapi.client import poll_invocation + + +class CustomError(Exception): + """An error that's neither part of the standard library nor LabThings.""" + + +class ErrorThing(lt.Thing): + @lt.action + def raise_custom_error(self) -> None: + """Raise a custom Exception.""" + raise CustomError("custom error message.") + + @lt.action + def raise_runtime_error(self) -> None: + """Raise a RuntimeError.""" + raise RuntimeError("runtime error message.") + + @lt.action + def raise_global_lock_error(self) -> None: + """Raise a GlobaLockBusyError after the action's started.""" + raise GlobalLockBusyError("this action has already started.") + + @lt.action + def raise_invocation_cancelled_error(self) -> None: + """Raise an InvocationCancelledError, pretend to be cancelled.""" + raise InvocationCancelledError("pretending to be cancelled.") + + +@pytest.fixture +def server() -> lt.ThingServer: + """Make a server with the Thing attached.""" + return lt.ThingServer.from_things({"thing": ErrorThing}, enable_global_lock=True) + + +@pytest.fixture +def thing(server: lt.ThingServer) -> ErrorThing: + """extract the thing from the server.""" + thing = server.things["thing"] + assert isinstance(thing, ErrorThing) + return thing + + +@pytest.fixture +def client(server: lt.ThingServer) -> Iterator[lt.ThingClient]: + """Connect a ThingClient to the thing.""" + with server.test_client() as tc: + yield lt.ThingClient.from_url("/thing/", client=tc) + + +@pytest.fixture +def hold_global_lock(thing): + """Hold the global lock in a background thread. + + This allows us to test for actions that fail to start. + """ + _event = threading.Event() + _global_lock = thing._thing_server_interface.global_lock + assert _global_lock + _global_lock.default_timeout = 0.001 # reduce time spent waiting + + def _hold_lock(): + try: + thing._thing_server_interface.global_lock.acquire() + _event.wait() + finally: + thing._thing_server_interface.global_lock.release() + + thread = threading.Thread(target=_hold_lock) + thread.start() + try: + yield + finally: + _event.set() + thread.join() + + +_original_poll_invocation = poll_invocation + + +def poll_and_delete_error(client, invocation): + """Wrap poll_invocation to hide the `error` key.""" + output = _original_poll_invocation(client, invocation) + del output["error"] + return output + + +@pytest.fixture +def delete_error_info(mocker): + mocker.patch("labthings_fastapi.client.poll_invocation", poll_and_delete_error) + + +def test_custom_error(client): + """Test that the custom error is properly reported.""" + with pytest.raises(ServerActionError, match="[CustomError]: custom error message."): + client.raise_custom_error() + + +def test_custom_error_old(client, delete_error_info): + """Test that the custom error is properly reported if the `error` key is missing.""" + with pytest.raises(ServerActionError, match="[CustomError]: custom error message."): + client.raise_custom_error() + + +def test_runtime_error(client): + """Test that the custom error is properly reported.""" + with pytest.raises( + ServerActionError, match="[RuntimeError]: runtime error message." + ): + client.raise_runtime_error() + + +def test_runtime_error_old(client, delete_error_info): + """Test that the custom error is properly reported if the `error` key is missing.""" + with pytest.raises( + ServerActionError, match="[RuntimeError]: runtime error message." + ): + client.raise_runtime_error() + + +def test_global_lock_error(client): + """Test a global lock error encountered during the action.""" + with pytest.raises(GlobalLockBusyError, match="this action has already started."): + client.raise_global_lock_error() + + +def test_global_lock_error_old(client, delete_error_info): + """Test a global lock error encountered during the action (no error key).""" + with pytest.raises( + ServerActionError, + match="[GlobalLockBusyError]: this action has already started.", + ): + client.raise_global_lock_error() + + +def test_invocation_cancelled_error(client): + """Test a simulated cancellation.""" + with pytest.raises(InvocationCancelledError, match="pretending to be cancelled."): + client.raise_invocation_cancelled_error() + + +def test_invocation_cancelled_error_old(client, delete_error_info): + """Test a simulated cancellation without the `error` key in the response.""" + with pytest.raises(ServerActionError, match="was cancelled."): + client.raise_invocation_cancelled_error() + + +def test_failure_to_start(client, hold_global_lock): + """Test the error that happens if the global lock is busy.""" + with pytest.raises( + GlobalLockBusyError, match="The global lock could not be acquired" + ): + client.raise_runtime_error() # The action is unimportant as it never starts + + +def test_failure_to_start_old(client, hold_global_lock, delete_error_info): + """Test the error that happens if the global lock is busy, without `error`.""" + with pytest.raises(ServerActionError, match="didn't run raise_runtime_error"): + client.raise_runtime_error() # The action is unimportant as it never starts From a83ec60f57003e6632c6a8ba8756b24173850192 Mon Sep 17 00:00:00 2001 From: Beth <167304066+bprobert97@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:59:54 +0100 Subject: [PATCH 14/18] Update CONTRIBUTING.md Co-authored-by: Richard Bowman --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 44613037..e210577d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,6 +47,7 @@ We use several tools to maintain code quality. All of these run in CI with [GitH * **Linting:** We use [`ruff`] for fast linting and formatting. We highly recommend setting up a pre-commit hook to ensure [`ruff`] passes on every commit. ```bash + ruff format --check ruff check . ``` From 2d7a21b58042b1158d2dc92645016298cdb813ec Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 29 Jun 2026 16:01:29 +0100 Subject: [PATCH 15/18] Add --cov=src to pyproject.toml --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 25be8fbc..dd1ffdca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,6 +71,7 @@ addopts = [ "--cov-report=xml:coverage.xml", "--cov-report=html:htmlcov", "--cov-report=lcov", + "--cov=src" ] markers = [ "slow: marks tests as slow (deselect with '-m \"not slow\"')", From 3a63ede6718f2433019192afa3a24347be181c4d Mon Sep 17 00:00:00 2001 From: Beth <167304066+bprobert97@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:02:08 +0100 Subject: [PATCH 16/18] Update CONTRIBUTING.md Co-authored-by: Richard Bowman --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e210577d..29a6d8ae 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -61,7 +61,7 @@ We use several tools to maintain code quality. All of these run in CI with [GitH codespell . ``` -* **Type Checking:** We use [`mypy`] for static type checking. +* **Type Checking:** We use [`mypy`] for static type checking. It is configured in `pyproject.toml`. ```bash mypy ``` From 392d1de36a073737d8ee8d04ee6ee5149a62f631 Mon Sep 17 00:00:00 2001 From: Beth Probert Date: Mon, 29 Jun 2026 16:22:14 +0100 Subject: [PATCH 17/18] Fix pip install command --- .github/workflows/test.yml | 2 +- CONTRIBUTING.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 683408fc..b48496a3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -103,7 +103,7 @@ jobs: python-version: ${{ matrix.python }} - name: Install Dependencies - run: pip install -e .[dev] + run: pip install -r dev-requirements.txt - name: Print installed packages run: pip freeze diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 29a6d8ae..5784c876 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,7 +38,7 @@ git clone https://github.com/labthings/labthings-fastapi.git cd labthings-fastapi # Install the package in editable mode with development dependencies -pip install -e .[dev] +pip install -r dev-requirements.txt ``` ### 2. Linting and Testing From 89561101b2d19c57a3c62746d4e3e495d9246304 Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Mon, 29 Jun 2026 16:32:41 +0100 Subject: [PATCH 18/18] Regenerate `dev-requirements.txt` so it matches `main` --- dev-requirements.txt | 61 +++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 42a3ba85..7ca9f741 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,9 +1,7 @@ -# -# This file is autogenerated by pip-compile with Python 3.13 -# by the following command: -# -# pip-compile --extra=dev --output-file=dev-requirements.txt pyproject.toml -# +# This file was autogenerated by uv via the following command: +# uv pip compile dev-requirements.in -o dev-requirements.txt +-e . + # via -r dev-requirements.in alabaster==1.0.0 # via sphinx annotated-doc==0.0.4 @@ -15,7 +13,7 @@ annotated-types==0.7.0 anyio==4.14.0 # via # httpx - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # starlette # watchfiles apeye==1.4.1 @@ -35,7 +33,7 @@ babel==2.18.0 # via sphinx beautifulsoup4==4.15.0 # via sphinx-toolbox -cachecontrol[filecache]==0.14.4 +cachecontrol==0.14.4 # via sphinx-toolbox certifi==2026.6.17 # via @@ -53,15 +51,8 @@ click==8.4.1 # rich-toolkit # uvicorn codespell==2.4.2 - # via labthings-fastapi (pyproject.toml) -colorama==0.4.6 - # via - # click - # pytest - # sphinx - # typer - # uvicorn -coverage[toml]==7.14.1 + # via labthings-fastapi +coverage==7.14.3 # via pytest-cov detect-installer==0.1.0 # via fastapi-cloud-cli @@ -90,9 +81,9 @@ email-validator==2.3.0 # via # fastapi # pydantic -fastapi[all]==0.135.4 - # via labthings-fastapi (pyproject.toml) -fastapi-cli[standard]==0.0.24 +fastapi==0.135.4 + # via labthings-fastapi +fastapi-cli==0.0.27 # via fastapi fastapi-cloud-cli==0.20.0 # via fastapi-cli @@ -107,7 +98,7 @@ flake8==7.3.0 # flake8-pyproject # flake8-rst # flake8-rst-docstrings - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # pydoclint flake8-pyproject==1.2.4 # via labthings-fastapi @@ -129,8 +120,8 @@ httpx==0.28.1 # via # fastapi # fastapi-cloud-cli - # labthings-fastapi (pyproject.toml) -idna==3.16 + # labthings-fastapi +idna==3.18 # via # anyio # apeye-core @@ -183,7 +174,7 @@ mypy==1.20.2 mypy-extensions==1.1.0 # via mypy myst-parser==4.0.1 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi natsort==8.4.0 # via domdf-python-tools numpy==2.2.6 @@ -204,11 +195,11 @@ pluggy==1.6.0 # pytest-cov pycodestyle==2.14.0 # via flake8 -pydantic[email]==2.13.4 +pydantic==2.13.4 # via # fastapi # fastapi-cloud-cli - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # pydantic-extra-types # pydantic-settings pydantic-core==2.46.4 @@ -217,8 +208,8 @@ pydantic-extra-types==2.11.1 # via fastapi pydantic-settings==2.14.2 # via fastapi -pydoclint[flake8]==0.8.4 - # via labthings-fastapi (pyproject.toml) +pydoclint==0.8.7 + # via labthings-fastapi pyflakes==3.4.0 # via flake8 pygments==2.20.0 @@ -295,7 +286,7 @@ soupsieve==2.8.4 sphinx==8.1.3 # via # autodocsumm - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # myst-parser # sphinx-autoapi # sphinx-autodoc-typehints @@ -333,7 +324,7 @@ sphinxcontrib-qthelp==2.0.0 sphinxcontrib-serializinghtml==2.0.0 # via sphinx sphobjinv==2.4 - # via labthings-fastapi (pyproject.toml) + # via labthings-fastapi standard-imghdr==3.10.14 # via sphinx-jinja2-compat starlette==1.3.1 @@ -342,9 +333,7 @@ tabulate==0.10.0 # via sphinx-toolbox tinycss2==1.5.1 # via dict2css -tomli==2.4.1 - # via labthings-fastapi (pyproject.toml) -typer==0.26.1 +typer==0.26.7 # via # fastapi-cli # fastapi-cloud-cli @@ -355,7 +344,7 @@ typing-extensions==4.15.0 # beautifulsoup4 # domdf-python-tools # fastapi - # labthings-fastapi (pyproject.toml) + # labthings-fastapi # mypy # pydantic # pydantic-core @@ -373,11 +362,13 @@ urllib3==2.7.0 # requests # sentry-sdk # sphinx-prompt -uvicorn[standard]==0.48.0 +uvicorn==0.49.0 # via # fastapi # fastapi-cli # fastapi-cloud-cli +uvloop==0.22.1 + # via uvicorn watchfiles==1.2.0 # via uvicorn webencodings==0.5.1