From 45903f92a61b8aa2423673a8cbb843fd5431f9f3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 22 Feb 2026 19:29:33 +0000 Subject: [PATCH 1/5] Create and mount application level data direcory rather than just a scan directory --- generate_http_api_data.py | 2 +- ofm_config_full.json | 7 +- ofm_config_manual.json | 5 +- ofm_config_simulation.json | 6 +- .../server/__init__.py | 60 ++++++------- .../server/serve_static_files.py | 26 +++--- .../things/__init__.py | 56 ++++++++++++ .../things/smart_scan.py | 12 +-- tests/unit_tests/test_serve_static_files.py | 27 ++---- tests/unit_tests/test_server_cli.py | 3 +- tests/unit_tests/test_server_config.py | 87 +++++-------------- tests/unit_tests/test_smart_scan.py | 62 +++++++------ 12 files changed, 187 insertions(+), 166 deletions(-) diff --git a/generate_http_api_data.py b/generate_http_api_data.py index 86c09323..8171f0ab 100755 --- a/generate_http_api_data.py +++ b/generate_http_api_data.py @@ -36,7 +36,7 @@ def main() -> None: with open(config, "r", encoding="utf-8") as file_obj: config_dict = json.load(file_obj) - config_dict["things"]["smart_scan"]["kwargs"]["scans_folder"] = "/tmp/scans" + config_dict["application_config"]["data_folder"] = "/tmp/data" server = lt.ThingServer(things=config_dict["things"]) test_client = TestClient(server.app) diff --git a/ofm_config_full.json b/ofm_config_full.json index c43eb8ad..27024a2c 100644 --- a/ofm_config_full.json +++ b/ofm_config_full.json @@ -14,7 +14,6 @@ "smart_scan": { "class": "openflexure_microscope_server.things.smart_scan:SmartScanThing", "kwargs": { - "scans_folder": "/var/openflexure/scans/", "default_workflow": "histo_scan_workflow" } }, @@ -26,5 +25,9 @@ "bg_channel_deviations_luv": "openflexure_microscope_server.things.background_detect:ChannelDeviationLUV" }, "settings_folder": "/var/openflexure/settings/", - "log_folder": "/var/openflexure/logs/" + "application_config": { + "log_folder": "/var/openflexure/logs/", + "data_folder": "/var/openflexure/data/" + } + } diff --git a/ofm_config_manual.json b/ofm_config_manual.json index cff014e4..4498cf81 100644 --- a/ofm_config_manual.json +++ b/ofm_config_manual.json @@ -4,5 +4,8 @@ "system": "openflexure_microscope_server.things.system:OpenFlexureSystem" }, "settings_folder": "./openflexure/settings/", - "log_folder": "./openflexure/logs/" + "application_config": { + "log_folder": "./openflexure/logs/", + "data_folder": "./openflexure/data/" + } } \ No newline at end of file diff --git a/ofm_config_simulation.json b/ofm_config_simulation.json index 962dffae..b04988d4 100644 --- a/ofm_config_simulation.json +++ b/ofm_config_simulation.json @@ -9,7 +9,6 @@ "smart_scan": { "class": "openflexure_microscope_server.things.smart_scan:SmartScanThing", "kwargs": { - "scans_folder": "./openflexure/scans/", "default_workflow": "histo_scan_workflow" } }, @@ -20,5 +19,8 @@ "bg_channel_deviations_luv": "openflexure_microscope_server.things.background_detect:ChannelDeviationLUV" }, "settings_folder": "./openflexure/settings/", - "log_folder": "./openflexure/logs/" + "application_config": { + "log_folder": "./openflexure/logs/", + "data_folder": "./openflexure/data/" + } } \ No newline at end of file diff --git a/src/openflexure_microscope_server/server/__init__.py b/src/openflexure_microscope_server/server/__init__.py index 71ef6362..d203a9b1 100644 --- a/src/openflexure_microscope_server/server/__init__.py +++ b/src/openflexure_microscope_server/server/__init__.py @@ -11,7 +11,8 @@ from pathlib import Path from typing import Any, Callable, Optional import uvicorn -from fastapi.middleware.cors import CORSMiddleware # vue3 migration +from fastapi.middleware.cors import CORSMiddleware +from pydantic import BaseModel from uvicorn.main import Server import labthings_fastapi as lt @@ -35,6 +36,15 @@ DEVELOPER_MODE = os.getenv("OFM_SERVER_DEV_MODE", "false").lower() == "true" _TEMPLATE_PATH = Path(__file__).with_name("fallback.html.jinja") +class OFMApplicationData(BaseModel): + """Application data for the OpenFlexure Microscope.""" + + log_folder: str + """The directory to save the logs in.""" + data_folder: str + """The directory for Things to save data in.""" + + def set_shutdown_function(shutdown_function: Callable[[], None]) -> None: """Ensure a function is called before the shutdown. @@ -62,10 +72,10 @@ def set_shutdown_function(shutdown_function: Callable[[], None]) -> None: def customise_server( - server: lt.ThingServer, log_folder: str, scans_folder: Optional[str] + server: lt.ThingServer, application_config: OFMApplicationData ) -> None: """Customise the server with additional endpoints, etc.""" - configure_logging(log_folder) + configure_logging(application_config.log_folder) if DEVELOPER_MODE: # Allow CORS in developer mode for easier testing with the webapp @@ -78,27 +88,13 @@ def customise_server( ) add_v2_endpoints(server) - add_static_files(server.app, scans_folder) + add_static_files(server.app, application_config.data_folder) # Add an endpoint to get the logs - (directly calling the FastAPI decorator) server.app.get("/log/")(retrieve_log) server.app.get("/logfile/")(retrieve_log_from_file) -def _get_scans_dir(config: dict) -> Optional[str]: - """Read the config and return the scans directory. - - Return is None if there is no smart_scan thing loaded. - """ - if "smart_scan" in config["things"]: - try: - return config["things"]["smart_scan"]["kwargs"]["scans_folder"] - except KeyError as e: - msg = "Configuration error: smart scan should have scans_folder kwarg set" - raise RuntimeError(msg) from e - return None - - def serve_from_cli(argv: Optional[list[str]] = None) -> None: """Start the server from the command line.""" args = lt.cli.parse_args(argv) @@ -112,12 +108,10 @@ def serve_from_cli(argv: Optional[list[str]] = None) -> None: lt_config = None server = None try: - lt_config, internal_config = _full_config_from_args(args) + lt_config, application_config = _full_config_from_args(args) server = lt.ThingServer.from_config(lt_config) - customise_server( - server, internal_config["log_folder"], internal_config["scans_folder"] - ) + customise_server(server, application_config) def shutdown_call() -> None: try: @@ -172,7 +166,9 @@ def serve_from_cli(argv: Optional[list[str]] = None) -> None: raise e -def _full_config_from_args(args: Namespace) -> tuple[ThingServerConfig, dict[str, Any]]: +def _full_config_from_args( + args: Namespace, +) -> tuple[ThingServerConfig, OFMApplicationData]: """Load configuration from LabThings args allowing patching. This returns the labthings ThingServerConfig model and a dictionary of the config @@ -181,16 +177,14 @@ def _full_config_from_args(args: Namespace) -> tuple[ThingServerConfig, dict[str This provides similar functionarlity to lt.cli.config_from_args except allows the configuration file to specify a base config, and optionally patches. """ - internal_config = {"log_folder": "./openflexure/logs", "scans_folder": None} - # If no config file specified let LabThings handle it. + # Don't allow configuration to be set as an argument as then we cannot handle + # application_config if not args.config: - return lt.cli.config_from_args(args), internal_config + raise RuntimeError( + "OpenFlexure Microscope Server must have a configuration file specified." + ) patched_config = load_patched_config(args.config) - log_folder = patched_config.pop("log_folder", None) - if log_folder is not None: - internal_config["log_folder"] = log_folder - scans_folder = _get_scans_dir(patched_config) - if scans_folder is not None: - internal_config["scans_folder"] = scans_folder - return ThingServerConfig(**patched_config), internal_config + application_config = OFMApplicationData(**patched_config.pop("application_config")) + + return ThingServerConfig(**patched_config), application_config diff --git a/src/openflexure_microscope_server/server/serve_static_files.py b/src/openflexure_microscope_server/server/serve_static_files.py index 5d9d7822..81338cc8 100644 --- a/src/openflexure_microscope_server/server/serve_static_files.py +++ b/src/openflexure_microscope_server/server/serve_static_files.py @@ -1,7 +1,6 @@ """Add endpoints for static files to the underlying FastAPI server.""" import os -from typing import Optional from fastapi import FastAPI from fastapi.responses import FileResponse, RedirectResponse @@ -36,7 +35,7 @@ def add_static_file(app: FastAPI, fname: str, folder: str) -> None: ) -def add_static_files(app: FastAPI, scans_folder: Optional[str]) -> None: +def add_static_files(app: FastAPI, data_folder: str) -> None: """Add the static files responsible for the webapp app to the FastAPI app. Note that any file in the root of the static dir will not be cached. However, the @@ -45,7 +44,7 @@ def add_static_files(app: FastAPI, scans_folder: Optional[str]) -> None: important file not to cache is "index.html". :param app: The FastAPI app to add to, in this case the OpenFlexure server - :param scans_folder: The directory for the scans. + :param data_folder: The directory for any data. """ check_static_dir() @@ -65,16 +64,17 @@ def add_static_files(app: FastAPI, scans_folder: Optional[str]) -> None: name=f"static_{fname}", ) - # If scans folder is None, there is not smart scan thing. So nothing to mount. - if scans_folder is not None: - # Mount the scan directory to .../scans/, to allow dzi viewing - if not os.path.isdir(scans_folder): - os.makedirs(scans_folder) - app.mount( - "/scans/", - StaticFiles(directory=scans_folder), - name="scans", - ) + # We need a data folder + if data_folder is None: + raise ValueError("No data folder is set, cannot start server") + # Mount the scan directory to .../data/, to allow dzi viewing + if not os.path.isdir(data_folder): + os.makedirs(data_folder) + app.mount( + "/data/", + StaticFiles(directory=data_folder), + name="data", + ) def check_static_dir() -> None: diff --git a/src/openflexure_microscope_server/things/__init__.py b/src/openflexure_microscope_server/things/__init__.py index 35b09029..1780e703 100644 --- a/src/openflexure_microscope_server/things/__init__.py +++ b/src/openflexure_microscope_server/things/__init__.py @@ -3,3 +3,59 @@ The microscope can be extended to be used with other hardware by creating a package with other Things and including them in the LabThings-FastAPI config file. """ + +import posixpath +from typing import Optional, Self + +from starlette.routing import Mount +from starlette.staticfiles import StaticFiles + +import labthings_fastapi as lt + + +class OFMThing(lt.Thing): + """A custom LabThings Thing class for the OpenFlexure Microscope.""" + + _data_dir: Optional[str] = None + + def __enter__(self) -> Self: + """Set the data directory when the Thing is entered.""" + self._data_dir = get_data_directory_from_server(self) + return self + + @property + def data_dir(self) -> str: + """The data directory for this thing.""" + if self._data_dir is None: + raise RuntimeError( + "No data directory set. Has the LabThings server been started?" + ) + return self._data_dir + + +def get_data_directory_from_server(thing: lt.Thing) -> str: + """Get the data directory from the server. + + :param thing: The Thing to get the data directory for. + + :return: The data directory as a string: + :raise RuntimeError: If not able to get the data directory for any reason. + """ + server = thing._thing_server_interface._server() + if server is None: + raise RuntimeError("No server found to communicate with.") + routes = server.app.routes + try: + route_paths = [route.path if hasattr(route, "path") else "" for route in routes] + data_index = route_paths.index("/data") + except ValueError as e: + raise RuntimeError("Could not find data directory") from e + mount = routes[data_index] + if not isinstance(mount, Mount): + raise RuntimeError("Data directory isn't a starlette.routing.Mount.") + if not isinstance(mount.app, StaticFiles): + raise RuntimeError("Data is not mounted as static files.") + app_data_dir = mount.app.directory + if app_data_dir is None: + raise RuntimeError("Data directory is not set.") + return posixpath.join(str(app_data_dir), thing.path.strip("/")) diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index d587de3c..57ba5190 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -32,6 +32,7 @@ from pydantic import BaseModel, PlainSerializer import labthings_fastapi as lt from openflexure_microscope_server import scan_directories, stitching +from openflexure_microscope_server.things import OFMThing from openflexure_microscope_server.utilities import coerce_thing_selector # Things @@ -120,7 +121,7 @@ def _scan_running( return scan_running_wrapper -class SmartScanThing(lt.Thing): +class SmartScanThing(OFMThing): """A Thing for scanning samples and interacting with past scans. SmartScanThing exposes all functionality for automatically scanning samples, @@ -135,23 +136,22 @@ class SmartScanThing(lt.Thing): def __init__( self, thing_server_interface: lt.ThingServerInterface, - scans_folder: str, default_workflow: str, ) -> None: """Initialise a SmartScanThing saving to and loading from the input directory. - :param scans_folder: This is the path to the directory where all scans will be - saved. Any scans already in this directory will be accessible through the - HTTP interface. + :param default_workflow: The default workflows that smart scan uses if nothing + is set in settings. """ super().__init__(thing_server_interface) - self._scan_dir_manager = scan_directories.ScanDirectoryManager(scans_folder) self._scan_lock = threading.Lock() self._default_workflow = default_workflow self._workflow_name = default_workflow def __enter__(self) -> Self: """Open hardware connection when the Thing context manager is opened.""" + super().__enter__() + self._scan_dir_manager = scan_directories.ScanDirectoryManager(self.data_dir) valid_name = coerce_thing_selector( thing_mapping=self._all_workflows, selected=self.workflow_name, diff --git a/tests/unit_tests/test_serve_static_files.py b/tests/unit_tests/test_serve_static_files.py index 84b22c37..ba5fb693 100644 --- a/tests/unit_tests/test_serve_static_files.py +++ b/tests/unit_tests/test_serve_static_files.py @@ -81,7 +81,7 @@ def test_add_static_with_no_static_dir(mocker): ) # Should raise as the mock static dir does not exist with pytest.raises(FileNotFoundError): - serve_static_files.add_static_files(mock_app, scans_folder=None) + serve_static_files.add_static_files(mock_app, data_folder=None) assert mock_app.get.call_count == 0 @@ -126,7 +126,8 @@ def test_add_static_files(mock_static_dir, mocker): ) # Get the wrapper function from the mocked decorator wrapper = mock_app.get.return_value - serve_static_files.add_static_files(mock_app, scans_folder=None) + with tempfile.TemporaryDirectory() as datadir: + serve_static_files.add_static_files(mock_app, data_folder=datadir) # Get should have been called twice to create a route for index assert mock_app.get.call_count == 2 @@ -151,23 +152,11 @@ def test_add_static_files(mock_static_dir, mocker): assert second_wrapped().headers["Pragma"] == "no-cache" # Also should have mounted both dirs - assert mock_app.mount.call_count == 1 - mounted_path = mock_app.mount.call_args.args[0] - mounted_dir = mock_app.mount.call_args.args[1].directory + assert mock_app.mount.call_count == 2 + mounted_path = mock_app.mount.call_args_list[0].args[0] + mounted_dir = mock_app.mount.call_args_list[0].args[1].directory assert "/assets/" in mounted_path assert os.path.join(mock_static_dir, "assets") in mounted_dir - -def test_add_static_files_with_scan_dir(mock_static_dir, mocker): - """Check the scan dir mounts if supplied.""" - mock_app = mocker.Mock() - mocker.patch( - "openflexure_microscope_server.server.serve_static_files.STATIC_PATH", - mock_static_dir, - ) - with tempfile.TemporaryDirectory() as scandir: - serve_static_files.add_static_files(mock_app, scans_folder=scandir) - # The scan dir should be the last to mount so can use call args. It should mount - # at /scans/ - assert mock_app.mount.call_args.args[0] == "/scans/" - assert mock_app.mount.call_args.args[1].directory == scandir + assert mock_app.mount.call_args_list[1].args[0] == "/data/" + assert mock_app.mount.call_args_list[1].args[1].directory == datadir diff --git a/tests/unit_tests/test_server_cli.py b/tests/unit_tests/test_server_cli.py index 7901fc69..b73dc80f 100644 --- a/tests/unit_tests/test_server_cli.py +++ b/tests/unit_tests/test_server_cli.py @@ -15,7 +15,8 @@ from .test_server_config import SIM_CONFIG def test_no_config(): """Check that an error is thrown if no configuration is set for the microspe via CLI.""" - with pytest.raises(RuntimeError, match="No configuration"): + msg = "OpenFlexure Microscope Server must have a configuration file specified." + with pytest.raises(RuntimeError, match=msg): ofm_server.serve_from_cli([]) diff --git a/tests/unit_tests/test_server_config.py b/tests/unit_tests/test_server_config.py index 61a3bb85..6ae8a1d5 100644 --- a/tests/unit_tests/test_server_config.py +++ b/tests/unit_tests/test_server_config.py @@ -1,9 +1,7 @@ """Test server booting and shut down.""" -import json import os from argparse import Namespace -from copy import deepcopy import pytest @@ -12,11 +10,13 @@ from labthings_fastapi.server.config_model import ThingServerConfig # Import as ofm server to attempt to minimise confusion with server as a var in other # functions and also FastAPI `Server`. from openflexure_microscope_server import server as ofm_server +from openflexure_microscope_server.server import OFMApplicationData THIS_DIR = os.path.dirname(os.path.abspath(__file__)) REPO_ROOT = os.path.dirname(os.path.dirname(THIS_DIR)) FULL_CONFIG = os.path.join(REPO_ROOT, "ofm_config_full.json") SIM_CONFIG = os.path.join(REPO_ROOT, "ofm_config_simulation.json") +MANUAL_CONFIG = os.path.join(REPO_ROOT, "ofm_config_manual.json") @pytest.mark.parametrize("side_effect", [None, RuntimeError("Mock")]) @@ -68,8 +68,11 @@ def test_customise_server(mocker): # The wrapper returned for app.get so we can see what functions are decorated. wrapper = mock_app.get.return_value + application_data = OFMApplicationData( + log_folder="mock_log_folder", data_folder="mock_data_folder" + ) # Finally we can run it! - ofm_server.customise_server(mock_server, "mock_log_folder", "mock_scan_folder") + ofm_server.customise_server(mock_server, application_data) # Check each internal customisation function is called assert mocked_add_static.call_count == 1 @@ -88,67 +91,26 @@ def test_customise_server(mocker): assert mocked_retrieve_log_file -@pytest.mark.parametrize( - ("config_file", "expected_scan_dir"), - [(FULL_CONFIG, "/var/openflexure/scans/"), (SIM_CONFIG, "./openflexure/scans/")], -) -def test_get_scans_dir_ok(config_file, expected_scan_dir): - """Test the _get_scans_dir function and also check the standard config files.""" - with open(config_file, "r", encoding="utf-8") as f_obj: - config_dict = json.load(f_obj) - assert ofm_server._get_scans_dir(config_dict) == expected_scan_dir - - -def test_get_scans_dir_no_smart_scan(): - """Test _get_scans_dir with no SmartScanThing.""" - # Load the standard config dict - with open(FULL_CONFIG, "r", encoding="utf-8") as f_obj: - config_dict = json.load(f_obj) - # Delete smart scan - del config_dict["things"]["smart_scan"] - # No SmartScanThing, should return None - assert ofm_server._get_scans_dir(config_dict) is None - - -def test_get_scans_dir_bad_smart_scan_config(): - """Test _get_scans_dir with a SmartScanThing that doesn't set a config dir.""" - # Load the standard config dict - with open(FULL_CONFIG, "r", encoding="utf-8") as f_obj: - config_dict = json.load(f_obj) - - # Copy the dictionary - broken_config = deepcopy(config_dict) - # Delete all the smart scan kwargs - del broken_config["things"]["smart_scan"]["kwargs"] - # Creates an Error - with pytest.raises(RuntimeError): - ofm_server._get_scans_dir(broken_config) - - # Same thing should happen if just the scans_folder key is deleted - broken_config = deepcopy(config_dict) - del broken_config["things"]["smart_scan"]["kwargs"] - # Creates an Error - with pytest.raises(RuntimeError): - ofm_server._get_scans_dir(broken_config) - - def test_full_config_from_args_json(): - """Check that _full_config_from_args returns as expected if json is supplied.""" + """Check that _full_config_from_args errors if json config is supplied.""" + msg = "OpenFlexure Microscope Server must have a configuration file specified." args = Namespace( config=None, json='{"things":{"example": "labthings_fastapi.example_things:MyThing"}}', ) - lt_conf, ofm_conf = ofm_server._full_config_from_args(args) - # If json is supplied OFM config is default. As the json is passed dircectly to - # The Pydantic Mocel in LabThings. No custom data allowed. - assert isinstance(lt_conf, ThingServerConfig) - assert "log_folder" in ofm_conf - assert ofm_conf["log_folder"] == "./openflexure/logs" - assert "scans_folder" in ofm_conf - assert ofm_conf["scans_folder"] is None + with pytest.raises(RuntimeError, match=msg): + ofm_server._full_config_from_args(args) -def test_full_config_from_args_full(mocker): +@pytest.mark.parametrize( + ("config", "log_dir", "data_dir"), + [ + (FULL_CONFIG, "/var/openflexure/logs/", "/var/openflexure/data/"), + (SIM_CONFIG, "./openflexure/logs/", "./openflexure/data/"), + (MANUAL_CONFIG, "./openflexure/logs/", "./openflexure/data/"), + ], +) +def test_full_config_from_args(config, log_dir, data_dir, mocker): """Check that _full_config_from_args returns as expected if json is supplied.""" # Mock picamera library so LabThings can create its model. mocker.patch.dict( @@ -159,12 +121,11 @@ def test_full_config_from_args_full(mocker): "picamera2.outputs": mocker.Mock(), }, ) - args = Namespace(config=FULL_CONFIG, json=None) - lt_conf, ofm_conf = ofm_server._full_config_from_args(args) + args = Namespace(config=config, json=None) + lt_conf, application_config = ofm_server._full_config_from_args(args) # If json is supplied OFM config is default. As the json is passed dircectly to # The Pydantic Mocel in LabThings. No custom data allowed. assert isinstance(lt_conf, ThingServerConfig) - assert "log_folder" in ofm_conf - assert ofm_conf["log_folder"] == "/var/openflexure/logs/" - assert "scans_folder" in ofm_conf - assert ofm_conf["scans_folder"] == "/var/openflexure/scans/" + assert isinstance(application_config, OFMApplicationData) + assert application_config.log_folder == log_dir + assert application_config.data_folder == data_dir diff --git a/tests/unit_tests/test_smart_scan.py b/tests/unit_tests/test_smart_scan.py index f89010e3..40d5dcef 100644 --- a/tests/unit_tests/test_smart_scan.py +++ b/tests/unit_tests/test_smart_scan.py @@ -51,25 +51,42 @@ def _clear_scan_dir() -> None: @pytest.fixture -def smart_scan_thing(): +def smart_scan_thing(mocker): """Return a smart scan thing as a fixture.""" + mocker.patch( + "openflexure_microscope_server.things.get_data_directory_from_server", + return_value=SCAN_DIR, + ) return create_thing_without_server( SmartScanThing, - scans_folder=SCAN_DIR, default_workflow="mock-_all_workflows", mock_all_slots=True, ) -def custom_smart_scan_thing(default_workflow, all_workflows): +@pytest.fixture +def entered_smart_scan_thing(smart_scan_thing): + """Yield a smart scan thing as a fixture that has been entred. + + This will make a scan directory manager. This fixture also clears the scan dir + """ + _clear_scan_dir() + with smart_scan_thing: + yield smart_scan_thing + + +def custom_smart_scan_thing(default_workflow, all_workflows, mocker): """Set up a custom smart scan thing with workflows adjusted. This allows setting a default workflow and to adjust all workflows from simple single item mock from `mock_all_slots`. """ + mocker.patch( + "openflexure_microscope_server.things.get_data_directory_from_server", + return_value=SCAN_DIR, + ) smart_scan_thing = create_thing_without_server( SmartScanThing, - scans_folder=SCAN_DIR, default_workflow=default_workflow, mock_all_slots=True, ) @@ -81,12 +98,13 @@ def custom_smart_scan_thing(default_workflow, all_workflows): return smart_scan_thing -def test_initial_properties(smart_scan_thing): +def test_initial_properties(entered_smart_scan_thing): """Check the initial values of properties. Test properties of SmartScanThing are available without a ThingServer and return expected default values. """ + smart_scan_thing = entered_smart_scan_thing assert smart_scan_thing._scan_dir_manager.base_dir == SCAN_DIR assert smart_scan_thing.latest_scan_name is None @@ -168,9 +186,9 @@ SELECTOR_CASES = [ @pytest.mark.parametrize("case", SELECTOR_CASES) -def test_workflow_set_on_enter(case, check_side_effect): +def test_workflow_set_on_enter(case, check_side_effect, mocker): """Check workflow is set on enter.""" - smart_scan_thing = custom_smart_scan_thing(case.default_wf, case.workflows) + smart_scan_thing = custom_smart_scan_thing(case.default_wf, case.workflows, mocker) with check_side_effect(case.side_effect, match=case.match): # Load in "loaded" as the sever would smart_scan_thing._workflow_name = case.loaded_wf @@ -178,14 +196,13 @@ def test_workflow_set_on_enter(case, check_side_effect): assert smart_scan_thing._workflow_name == case.expected_wf -def test_setting_workflows(caplog): +def test_setting_workflows(caplog, mocker): """Check that setting workflow works, or warns if incorrect.""" workflows = { "foo": mock.MagicMock(spec=ScanWorkflow), "bar": mock.MagicMock(spec=ScanWorkflow), } - - smart_scan_thing = custom_smart_scan_thing("foo", workflows) + smart_scan_thing = custom_smart_scan_thing("foo", workflows, mocker) with caplog.at_level(logging.WARNING), smart_scan_thing: assert smart_scan_thing._workflow_name == "foo" assert smart_scan_thing._workflow is workflows["foo"] @@ -225,15 +242,13 @@ def test_inaccessible_scan_methods(smart_scan_thing): smart_scan_thing.ongoing_scan -def test_private_delete_scan(smart_scan_thing, caplog): +def test_private_delete_scan(entered_smart_scan_thing, caplog): """Test the private _delete_scan method deletes directories or warns if it can't.""" - _clear_scan_dir() + smart_scan_thing = entered_smart_scan_thing with caplog.at_level(logging.INFO): fake_scan_name = "fake_scan_0001" fake_scan_path = os.path.join(SCAN_DIR, fake_scan_name) - # Make the outer scan dir, but not the one to delete - os.makedirs(SCAN_DIR) # Attempt to delete the fake scan. Expect it to fail and provide a warning deleted = smart_scan_thing._delete_scan(fake_scan_name) assert not deleted @@ -251,16 +266,13 @@ def test_private_delete_scan(smart_scan_thing, caplog): assert len(caplog.records) == 1 -def test_public_delete_scan(smart_scan_thing, caplog): +def test_public_delete_scan(entered_smart_scan_thing, caplog): """Test the delete_scan API call deletes directories or warns if it can't.""" - _clear_scan_dir() + smart_scan_thing = entered_smart_scan_thing with caplog.at_level(logging.INFO): fake_scan_name = "fake_scan_0001" fake_scan_path = os.path.join(SCAN_DIR, fake_scan_name) - # Make the outer scan dir, but not the one to delete - os.makedirs(SCAN_DIR) - # Attempt to delete the fake scan. Expect it to fail with pytest.raises(HTTPException) as exc_info: smart_scan_thing.delete_scan(fake_scan_name) @@ -280,9 +292,9 @@ def test_public_delete_scan(smart_scan_thing, caplog): assert len(caplog.records) == 1 -def test_delete_all_scans(smart_scan_thing, caplog): +def test_delete_all_scans(entered_smart_scan_thing, caplog): """Check the delete_all_scan API really does delete all the scans.""" - _clear_scan_dir() + smart_scan_thing = entered_smart_scan_thing with caplog.at_level(logging.INFO): fake_scan_names = [ "fake_scan_0001", @@ -346,15 +358,15 @@ def _run_only_outer_scan( return smart_scan_thing, exec_info -def test_outer_scan(smart_scan_thing, mocker): +def test_outer_scan(entered_smart_scan_thing, mocker): """Test setup and teardown of the scan.""" - mock_ss_thing, exec_info = _run_only_outer_scan(smart_scan_thing, mocker) + mock_ss_thing, exec_info = _run_only_outer_scan(entered_smart_scan_thing, mocker) assert exec_info is None # Checked the mocked _run_scan was run exactly once assert mock_ss_thing._run_scan.call_count == 1 -def test_outer_scan_wo_sample_skip(smart_scan_thing, mocker): +def test_outer_scan_wo_sample_skip(entered_smart_scan_thing, mocker): """Test setup and teardown of the scan.""" def _set_skip_background(mock_ss_thing): @@ -363,7 +375,7 @@ def test_outer_scan_wo_sample_skip(smart_scan_thing, mocker): mock_ss_thing.__dict__["skip_background"] = False mock_ss_thing, exec_info = _run_only_outer_scan( - smart_scan_thing, mocker, _set_skip_background + entered_smart_scan_thing, mocker, _set_skip_background ) assert exec_info is None From 7e4ec53d78139877bfb14837d99af6154fe01eb2 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 4 Mar 2026 14:03:14 +0000 Subject: [PATCH 2/5] Update how the application configuration is read for LabThings 0.0.17 --- pyproject.toml | 2 +- .../server/__init__.py | 22 +++++------ .../things/__init__.py | 39 ++++--------------- .../things/camera/__init__.py | 2 +- tests/unit_tests/test_server_cli.py | 6 ++- tests/unit_tests/test_server_config.py | 8 ++-- tests/unit_tests/test_smart_scan.py | 20 +++++----- 7 files changed, 37 insertions(+), 62 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3e7a9f33..f53171b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ classifiers = [ "Operating System :: OS Independent", ] dependencies = [ - "labthings-fastapi==0.0.14", + "labthings-fastapi==0.0.17", "sangaboard", "camera-stage-mapping ~= 0.1.10", "opencv-python-headless ~= 4.13.0", diff --git a/src/openflexure_microscope_server/server/__init__.py b/src/openflexure_microscope_server/server/__init__.py index d203a9b1..137863bb 100644 --- a/src/openflexure_microscope_server/server/__init__.py +++ b/src/openflexure_microscope_server/server/__init__.py @@ -75,8 +75,6 @@ def customise_server( server: lt.ThingServer, application_config: OFMApplicationData ) -> None: """Customise the server with additional endpoints, etc.""" - configure_logging(application_config.log_folder) - if DEVELOPER_MODE: # Allow CORS in developer mode for easier testing with the webapp server.app.add_middleware( @@ -108,7 +106,12 @@ def serve_from_cli(argv: Optional[list[str]] = None) -> None: lt_config = None server = None try: - lt_config, application_config = _full_config_from_args(args) + lt_config = _full_config_from_args(args) + # Validate our application data + if lt_config.application_config is None: + raise ValueError("No application configuration was supplied.") + application_config = OFMApplicationData(**lt_config.application_config) + configure_logging(application_config.log_folder) server = lt.ThingServer.from_config(lt_config) customise_server(server, application_config) @@ -166,25 +169,18 @@ def serve_from_cli(argv: Optional[list[str]] = None) -> None: raise e -def _full_config_from_args( - args: Namespace, -) -> tuple[ThingServerConfig, OFMApplicationData]: +def _full_config_from_args(args: Namespace) -> ThingServerConfig: """Load configuration from LabThings args allowing patching. - This returns the labthings ThingServerConfig model and a dictionary of the config - for the microscope. + This returns the labthings ThingServerConfig model. This provides similar functionarlity to lt.cli.config_from_args except allows the configuration file to specify a base config, and optionally patches. """ - # Don't allow configuration to be set as an argument as then we cannot handle - # application_config if not args.config: raise RuntimeError( "OpenFlexure Microscope Server must have a configuration file specified." ) patched_config = load_patched_config(args.config) - application_config = OFMApplicationData(**patched_config.pop("application_config")) - - return ThingServerConfig(**patched_config), application_config + return ThingServerConfig(**patched_config) diff --git a/src/openflexure_microscope_server/things/__init__.py b/src/openflexure_microscope_server/things/__init__.py index 1780e703..65314462 100644 --- a/src/openflexure_microscope_server/things/__init__.py +++ b/src/openflexure_microscope_server/things/__init__.py @@ -7,9 +7,6 @@ with other Things and including them in the LabThings-FastAPI config file. import posixpath from typing import Optional, Self -from starlette.routing import Mount -from starlette.staticfiles import StaticFiles - import labthings_fastapi as lt @@ -20,7 +17,13 @@ class OFMThing(lt.Thing): def __enter__(self) -> Self: """Set the data directory when the Thing is entered.""" - self._data_dir = get_data_directory_from_server(self) + # Note that the `application_config` was already validated when the + # server initialised. + application_config = self._thing_server_interface.application_config + if application_config is None: + raise ValueError("No application configuration was supplied.") + app_data_dir = application_config["data_folder"] + self._data_dir = posixpath.join(str(app_data_dir), self.path.strip("/")) return self @property @@ -31,31 +34,3 @@ class OFMThing(lt.Thing): "No data directory set. Has the LabThings server been started?" ) return self._data_dir - - -def get_data_directory_from_server(thing: lt.Thing) -> str: - """Get the data directory from the server. - - :param thing: The Thing to get the data directory for. - - :return: The data directory as a string: - :raise RuntimeError: If not able to get the data directory for any reason. - """ - server = thing._thing_server_interface._server() - if server is None: - raise RuntimeError("No server found to communicate with.") - routes = server.app.routes - try: - route_paths = [route.path if hasattr(route, "path") else "" for route in routes] - data_index = route_paths.index("/data") - except ValueError as e: - raise RuntimeError("Could not find data directory") from e - mount = routes[data_index] - if not isinstance(mount, Mount): - raise RuntimeError("Data directory isn't a starlette.routing.Mount.") - if not isinstance(mount.app, StaticFiles): - raise RuntimeError("Data is not mounted as static files.") - app_data_dir = mount.app.directory - if app_data_dir is None: - raise RuntimeError("Data directory is not set.") - return posixpath.join(str(app_data_dir), thing.path.strip("/")) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 351185ee..dd4f6cdd 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -604,7 +604,7 @@ class BaseCamera(lt.Thing): return self._background_detector_name @background_detector_name.setter - def _set_background_detector_name(self, name: str) -> None: + def _set_background_detector_name(self, name: Optional[str]) -> None: """Validate and set background_detector_name.""" if name not in self._all_background_detectors: self.logger.warning(f"{name} is not a valid background detector name.") diff --git a/tests/unit_tests/test_server_cli.py b/tests/unit_tests/test_server_cli.py index b73dc80f..8f37e412 100644 --- a/tests/unit_tests/test_server_cli.py +++ b/tests/unit_tests/test_server_cli.py @@ -36,6 +36,8 @@ def test_successful_start(mocker, caplog): "openflexure_microscope_server.server.lt.ThingServer.from_config", return_value=mock_server, ) + # Also mock configure_logging or it will touch a load of code and log. + mock_log_configure = mocker.patch.object(ofm_server, "configure_logging") # Also mock customisation or it will try to access the hard drive and make files. mock_customise = mocker.patch.object(ofm_server, "customise_server") # And mock uvicorn.run as we don't want to start a server process @@ -44,7 +46,9 @@ def test_successful_start(mocker, caplog): # Run the mock CLI ofm_server.serve_from_cli(["-c", SIM_CONFIG]) - # Check that the server was customised and the run + # Check that the logging was configures, the server was customised, and uvicorn was + # run + assert mock_log_configure.call_count == 1 assert mock_customise.call_count == 1 assert mock_uvicorn_run.call_count == 1 # Read the log config that was set diff --git a/tests/unit_tests/test_server_config.py b/tests/unit_tests/test_server_config.py index 6ae8a1d5..1aab586d 100644 --- a/tests/unit_tests/test_server_config.py +++ b/tests/unit_tests/test_server_config.py @@ -60,7 +60,6 @@ def test_customise_server(mocker): # but also to limit excess coverage reporting. mocked_add_static = mocker.patch.object(ofm_server, "add_static_files") mocked_v2_endpoints = mocker.patch.object(ofm_server, "add_v2_endpoints") - mocked_configure_logging = mocker.patch.object(ofm_server, "configure_logging") mocked_retrieve_log = mocker.patch.object(ofm_server, "retrieve_log") mocked_retrieve_log_file = mocker.patch.object(ofm_server, "retrieve_log_from_file") @@ -77,7 +76,6 @@ def test_customise_server(mocker): # Check each internal customisation function is called assert mocked_add_static.call_count == 1 assert mocked_v2_endpoints.call_count == 1 - assert mocked_configure_logging.call_count == 1 # Check app.get adds two routes assert mock_app.get.call_count == 2 assert wrapper.call_count == 2 @@ -122,10 +120,12 @@ def test_full_config_from_args(config, log_dir, data_dir, mocker): }, ) args = Namespace(config=config, json=None) - lt_conf, application_config = ofm_server._full_config_from_args(args) + lt_conf = ofm_server._full_config_from_args(args) # If json is supplied OFM config is default. As the json is passed dircectly to # The Pydantic Mocel in LabThings. No custom data allowed. assert isinstance(lt_conf, ThingServerConfig) - assert isinstance(application_config, OFMApplicationData) + # Check the configuration validates + application_config = OFMApplicationData(**lt_conf.application_config) + # And has the correct values assert application_config.log_folder == log_dir assert application_config.data_folder == data_dir diff --git a/tests/unit_tests/test_smart_scan.py b/tests/unit_tests/test_smart_scan.py index 40d5dcef..e8abf2d5 100644 --- a/tests/unit_tests/test_smart_scan.py +++ b/tests/unit_tests/test_smart_scan.py @@ -41,7 +41,7 @@ from openflexure_microscope_server.things.smart_scan import ( # Use our own dir in the root temp dir not a dynamically generated one so we # have some control of when it is deleted -SCAN_DIR = os.path.join(tempfile.gettempdir(), "scans") +SCAN_DIR = os.path.join(tempfile.gettempdir(), "smartscanthing") def _clear_scan_dir() -> None: @@ -53,15 +53,15 @@ def _clear_scan_dir() -> None: @pytest.fixture def smart_scan_thing(mocker): """Return a smart scan thing as a fixture.""" - mocker.patch( - "openflexure_microscope_server.things.get_data_directory_from_server", - return_value=SCAN_DIR, - ) - return create_thing_without_server( + thing = create_thing_without_server( SmartScanThing, default_workflow="mock-_all_workflows", mock_all_slots=True, ) + type(thing._thing_server_interface).application_config = mocker.PropertyMock( + return_value={"data_folder": tempfile.gettempdir()} + ) + return thing @pytest.fixture @@ -81,15 +81,15 @@ def custom_smart_scan_thing(default_workflow, all_workflows, mocker): This allows setting a default workflow and to adjust all workflows from simple single item mock from `mock_all_slots`. """ - mocker.patch( - "openflexure_microscope_server.things.get_data_directory_from_server", - return_value=SCAN_DIR, - ) smart_scan_thing = create_thing_without_server( SmartScanThing, default_workflow=default_workflow, mock_all_slots=True, ) + interface_type = type(smart_scan_thing._thing_server_interface) + interface_type.application_config = mocker.PropertyMock( + return_value={"data_folder": tempfile.gettempdir()} + ) # Pop the existing mock workflow and add specified ones (if any) smart_scan_thing._all_workflows.pop("mock-_all_workflows") for key, thing in all_workflows.items(): From ec27b7d5c53f91fcd922d364e4c4d3ef8b5bee62 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 4 Mar 2026 14:09:53 +0000 Subject: [PATCH 3/5] Update integration tests for application_config --- tests/integration_tests/test_actions.py | 5 ++++- tests/shared_utils/lt_test_utils.py | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/test_actions.py b/tests/integration_tests/test_actions.py index 220ea6e8..4f2afda0 100644 --- a/tests/integration_tests/test_actions.py +++ b/tests/integration_tests/test_actions.py @@ -33,7 +33,10 @@ def test_env(): """Yield a server with a very basic configuration.""" with open(SIM_CONFIG, "r", encoding="utf-8") as f_obj: config_dict = json.load(f_obj) - with LabThingsTestEnv(things=config_dict["things"]) as env: + with LabThingsTestEnv( + things=config_dict["things"], + application_config=config_dict["application_config"], + ) as env: yield env diff --git a/tests/shared_utils/lt_test_utils.py b/tests/shared_utils/lt_test_utils.py index 44793b0e..19b61723 100644 --- a/tests/shared_utils/lt_test_utils.py +++ b/tests/shared_utils/lt_test_utils.py @@ -34,6 +34,7 @@ class LabThingsTestEnv: self, things: Mapping[str, lt.Thing | str], settings_folder: Optional[str] = None, + application_config: Optional[Mapping[str, Any]] = None, ) -> None: """Initialise the test environment. @@ -47,6 +48,7 @@ class LabThingsTestEnv: self._test_client: Optional[TestClient] self._things_config = things self._settings_folder = settings_folder + self._application_config = application_config self._tmp_dir_obj: Optional[tempfile.TemporaryDirectory] = None def __enter__(self) -> Self: @@ -55,7 +57,9 @@ class LabThingsTestEnv: self._tmp_dir_obj = tempfile.TemporaryDirectory() self._settings_folder = self._tmp_dir_obj.name self._server = lt.ThingServer( - things=self._things_config, settings_folder=self._settings_folder + things=self._things_config, + settings_folder=self._settings_folder, + application_config=self._application_config, ) self._test_client = TestClient(self._server.app) self._test_client.__enter__() From 3a9340f6e0ec27cb4dfbb7e3a0bf90942bc085a2 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 4 Mar 2026 14:20:07 +0000 Subject: [PATCH 4/5] Update scan directory in webapp --- .../tabContentComponents/scanListComponents/scanViewer.vue | 2 +- webapp/src/components/tabContentComponents/scanListContent.vue | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/tabContentComponents/scanListComponents/scanViewer.vue b/webapp/src/components/tabContentComponents/scanListComponents/scanViewer.vue index b061c8d3..322ed2fc 100644 --- a/webapp/src/components/tabContentComponents/scanListComponents/scanViewer.vue +++ b/webapp/src/components/tabContentComponents/scanListComponents/scanViewer.vue @@ -81,7 +81,7 @@ export default { computed: { selectedScanDZI() { if (this.selectedScan && this.selectedScan.dzi) { - return `${this.baseUri}/scans/${this.selectedScan.name}/images/${this.selectedScan.dzi}`; + return `${this.baseUri}/data/smart_scan/${this.selectedScan.name}/images/${this.selectedScan.dzi}`; } return null; }, diff --git a/webapp/src/components/tabContentComponents/scanListContent.vue b/webapp/src/components/tabContentComponents/scanListContent.vue index 0b798af9..821a18e8 100644 --- a/webapp/src/components/tabContentComponents/scanListContent.vue +++ b/webapp/src/components/tabContentComponents/scanListContent.vue @@ -119,7 +119,7 @@ export default { }, selectedScanDZI() { if (this.selectedScan && this.selectedScan.dzi != "") { - return `${this.$store.getters.baseUri}/scans/${this.selectedScan.name}/images/${this.selectedScan.dzi}`; + return `${this.$store.getters.baseUri}/data/smart_scan/${this.selectedScan.name}/images/${this.selectedScan.dzi}`; } else { return null; } From a7a61971d3740c01c7d675063dbe140d5ff8cde3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 4 Mar 2026 17:27:59 +0000 Subject: [PATCH 5/5] Update picamera tests --- picamera_coverage.zip | Bin 54038 -> 54038 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/picamera_coverage.zip b/picamera_coverage.zip index 72bb9e27685ee0c14575b50a6a75c282ac815f6f..bd21c4667099fa1b3fb6feaff98cfd967c3581fb 100644 GIT binary patch delta 215 zcmbQXjCtBJW}yIYW)=|!5Gd3)I>wWR5O!Q(-b2!Ba>u< tG!qMB!<1ypR1-6kl(dwzL~{#+RHfPgZ$>5&W>m*c?!9Dzu>9;LPXLL1L)8EP delta 212 zcmbQXjCtBJW}yIYW)=|!5J;*@j^WT@y0KA6r9niAf&VA}NB#r+X?!pE*78;JrSd6l zb`;>?+w9+Wo{@u%iG`7qkBMos<^(|p4v@}J`3VmuJ6%XqH#Rjiv@lIgO}4NwHa9gi zO-f5iF-kTzFi12oH#bi-Of*VPGBz>TeEvcMqg`s6S(0IrnMq=riMeI6v7vF2vALnS qVWOe2kx5FbnVGq