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():