From b2d8ee98ef6b3064c73e55bae70feb440ce752dc Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 24 Jun 2026 09:43:21 +0100 Subject: [PATCH 01/13] chore(tests): Update tiled to `v0.2.12` (#1488) Update policy for new tiled version We are currently on [v0.2.4](https://github.com/bluesky/tiled/blob/main/CHANGELOG.md#v024-2026-02-18) --- tests/system_tests/compose.yaml | 2 +- tests/system_tests/services/tiled_config/dls.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/system_tests/compose.yaml b/tests/system_tests/compose.yaml index a90323b48c..7a73ef5306 100644 --- a/tests/system_tests/compose.yaml +++ b/tests/system_tests/compose.yaml @@ -46,7 +46,7 @@ services: - label=disable tiled: - image: ghcr.io/bluesky/tiled:0.2.4 + image: ghcr.io/bluesky/tiled:0.2.12 network_mode: host # Port 8407 environment: - PYTHONPATH=/deploy/ diff --git a/tests/system_tests/services/tiled_config/dls.py b/tests/system_tests/services/tiled_config/dls.py index 5ab3580d40..875b559d5a 100644 --- a/tests/system_tests/services/tiled_config/dls.py +++ b/tests/system_tests/services/tiled_config/dls.py @@ -33,10 +33,10 @@ def _check_principal(principal: Principal | None): detail="Principal is None", headers={"WWW-Authenticate": "Bearer"}, ) - if principal.type != PrincipalType.external: + if principal.type != PrincipalType.user: raise HTTPException( status_code=HTTP_401_UNAUTHORIZED, - detail=f"Principal of type {PrincipalType.external}" + detail=f"Principal of type {PrincipalType.user}" f" required but given {principal.type}", headers={"WWW-Authenticate": "Bearer"}, ) @@ -121,7 +121,7 @@ def build_input( if ( isinstance(principal, Principal) - and principal.type is PrincipalType.external + and principal.type is PrincipalType.user and principal.access_token is not None ): _input["token"] = principal.access_token.get_secret_value() From dd3f40df85507569d5b3e09ce680313af890e1c8 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 24 Jun 2026 10:20:50 +0100 Subject: [PATCH 02/13] feat: Validate tiled ServiceAccount config at startup (#1548) Ensure that the tiled auth configuration is valid and fail fast instead of when the first data is written. --- helm/blueapi/config_schema.json | 7 ++ helm/blueapi/values.schema.json | 7 ++ src/blueapi/config.py | 1 + src/blueapi/service/authorization.py | 29 +++++- src/blueapi/service/main.py | 3 +- .../unit_tests/service/test_authorization.py | 93 ++++++++++++++++++- tests/unit_tests/test_config.py | 1 + 7 files changed, 137 insertions(+), 4 deletions(-) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index 39a1b1aabe..dd8a48433a 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -345,8 +345,15 @@ "default": "account", "title": "Audience", "type": "string" + }, + "tiled_service_account_check": { + "title": "Tiled Service Account Check", + "type": "string" } }, + "required": [ + "tiled_service_account_check" + ], "title": "OpaConfig", "type": "object", "$id": "OpaConfig" diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index f78dbfb848..60310135cc 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -755,6 +755,9 @@ "$id": "OpaConfig", "title": "OpaConfig", "type": "object", + "required": [ + "tiled_service_account_check" + ], "properties": { "audience": { "title": "Audience", @@ -768,6 +771,10 @@ "format": "uri", "maxLength": 2083, "minLength": 1 + }, + "tiled_service_account_check": { + "title": "Tiled Service Account Check", + "type": "string" } }, "additionalProperties": false diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 538d141589..c56415bfe6 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -299,6 +299,7 @@ class Tag(StrEnum): class OpaConfig(BlueapiBaseModel): root: HttpUrl = HttpUrl("http://localhost:8181") audience: str = "account" + tiled_service_account_check: str class ApplicationConfig(BlueapiBaseModel): diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index aabd4a692a..a4a7b5c985 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -5,7 +5,8 @@ from aiohttp import ClientSession -from blueapi.config import OpaConfig +from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount +from blueapi.service.authentication import TiledAuth LOGGER = logging.getLogger(__name__) @@ -45,3 +46,29 @@ def for_config( return aclosing(cls(instrument, config)) LOGGER.info("No OPA config provided - not creating OpaClient") return nullcontext() + + async def require_tiled_service_account(self, token: str): + if not await self._call_opa( + self._config.tiled_service_account_check, + {"token": token, "beamline": self._instrument}, + ): + raise ValueError( + f"Tiled service account is not valid for '{self._instrument}'" + ) + + +async def validate_tiled_config( + tiled: ServiceAccount | str | None, oidc: OIDCConfig | None, opa: OpaClient | None +): + if not isinstance(tiled, ServiceAccount): + # can't validate an API key + return + + if not opa or not oidc: + LOGGER.info("Missing OPA or OIDC configuration required to validate tiled auth") + return + + LOGGER.info("Validating tiled configuration") + tiled.token_url = oidc.token_endpoint + auth = TiledAuth(tiled) + await opa.require_tiled_service_account(auth.get_access_token()) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index cc937d307d..a84a85b294 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -40,7 +40,7 @@ from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum -from .authorization import OpaClient +from .authorization import OpaClient, validate_tiled_config from .model import ( DeviceModel, DeviceResponse, @@ -98,6 +98,7 @@ async def inner(app: FastAPI): setup_runner(config) async with OpaClient.for_config(meta and meta.instrument, config.opa) as opa: app.state.authz = opa + await validate_tiled_config(config.tiled.authentication, config.oidc, opa) yield teardown_runner() diff --git a/tests/unit_tests/service/test_authorization.py b/tests/unit_tests/service/test_authorization.py index 608269d20b..2491985806 100644 --- a/tests/unit_tests/service/test_authorization.py +++ b/tests/unit_tests/service/test_authorization.py @@ -1,11 +1,13 @@ -from unittest.mock import AsyncMock, MagicMock, patch +from contextlib import AbstractContextManager, nullcontext +from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest from pydantic import HttpUrl -from blueapi.config import OpaConfig +from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount from blueapi.service.authorization import ( OpaClient, + validate_tiled_config, ) # Reusable client patch decorator @@ -20,9 +22,50 @@ def opa_config() -> OpaConfig: return OpaConfig( root=HttpUrl("http://auth.example.com"), + tiled_service_account_check="/auth/tiled", ) +@patch_client_session +@pytest.mark.parametrize( + "result,context", + [ + (False, pytest.raises(ValueError, match="Tiled service account is not valid ")), + (True, nullcontext()), + ], +) +async def test_tiled_service_account( + session: MagicMock, + opa_config: OpaConfig, + result: bool, + context: AbstractContextManager, +): + session.return_value.post = AsyncMock( + return_value=MagicMock(json=AsyncMock(return_value={"result": result})) + ) + + client = OpaClient(instrument="p99", config=opa_config) + + session.assert_called_once_with(base_url="http://auth.example.com/") + with context: + await client.require_tiled_service_account(token="foo_bar") + session().post.assert_called_once_with( + "/auth/tiled", + json={"input": {"token": "foo_bar", "beamline": "p99", "audience": "account"}}, + ) + + +@patch_client_session +async def test_exception_raised_when_opa_fails( + session: MagicMock, opa_config: OpaConfig +): + session.return_value.post = AsyncMock(side_effect=RuntimeError("Connection failed")) + async with OpaClient.for_config("p45", opa_config) as client: + assert client is not None + with pytest.raises(RuntimeError, match="Connection failed"): + await client.require_tiled_service_account(token="foo_bar") + + @patch_client_session async def test_session_closed(session: MagicMock, opa_config: OpaConfig): async with OpaClient.for_config("p45", opa_config): @@ -60,3 +103,49 @@ async def test_opa_adds_input_fields(session: MagicMock, opa_config: OpaConfig): "foo/bar", json={"input": {"beamline": "p45", "audience": "account", "foo": "bar"}}, ) + + +async def test_validate_tiled_config(): + opa = MagicMock(spec=OpaClient) + tiled = ServiceAccount() + oidc = Mock(spec=OIDCConfig) + oidc.token_endpoint = "token-endpoint" + with patch("blueapi.service.authorization.TiledAuth") as auth: + auth.return_value.get_access_token.return_value = "tiled-token" + await validate_tiled_config(tiled, oidc, opa) + + auth.assert_called_once_with(tiled) + opa.require_tiled_service_account.assert_called_once_with("tiled-token") + + +@pytest.mark.parametrize( + "tiled_auth,oidc,opa_client", + [ + (None, None, MagicMock(spec=OpaClient)), + ( + None, + OIDCConfig(well_known_url="http://example.com", client_id="test-client"), + MagicMock(spec=OpaClient), + ), + ("api_key", None, MagicMock(spec=OpaClient)), + ( + "api_key", + OIDCConfig(well_known_url="http://example.com", client_id="test-client"), + MagicMock(spec=OpaClient), + ), + (ServiceAccount(), None, MagicMock(spec=OpaClient)), + ( + ServiceAccount(), + OIDCConfig(well_known_url="http://example.com", client_id="test-client"), + None, + ), + ], +) +async def test_validate_tiled_config_with_missing_config( + tiled_auth: ServiceAccount | str | None, + oidc: OIDCConfig | None, + opa_client: MagicMock | None, +): + assert await validate_tiled_config(tiled_auth, oidc, opa_client) is None + if opa_client is not None: + opa_client.require_tiled_service_account.assert_not_called() diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 97a4ce3806..5cbb00c1b7 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -340,6 +340,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): "opa": { "root": "http://opa.example.com/", "audience": "account", + "tiled_service_account_check": "v1/tiled_service_account", }, }, { From 4caaf190c036e92b92258774b666fbca22954f9e Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 24 Jun 2026 13:26:30 +0100 Subject: [PATCH 03/13] refactor: Stop DeviceRef extending str (#1557) This was originally to make serialization behave correctly when the task was passed to requests. As we are using pydantic to dump the model to JSON before passing it, we can add the custom serialization there to get the device-to-string conversion that we need. --- src/blueapi/client/client.py | 16 +++++----- src/blueapi/client/rest.py | 11 ++++++- tests/unit_tests/client/test_client.py | 4 +-- tests/unit_tests/client/test_rest.py | 42 ++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 68637594fa..034b9f6a98 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -120,23 +120,23 @@ def __repr__(self) -> str: return f"DeviceCache({len(self._cache)} devices)" -class DeviceRef(str): +class DeviceRef: + name: str model: DeviceModel _cache: DeviceCache - def __new__(cls, name: str, cache: DeviceCache, model: DeviceModel): - instance = super().__new__(cls, name) - instance.model = model - instance._cache = cache - return instance + def __init__(self, name: str, cache: DeviceCache, model: DeviceModel): + self.name = name + self.model = model + self._cache = cache def __getattr__(self, name) -> "DeviceRef": if name.startswith("_"): raise AttributeError(f"No child device named {name}") - return self._cache[f"{self}.{name}"] + return self._cache[f"{self.name}.{name}"] def __repr__(self): - return f"Device({self})" + return f"Device({self.name})" class Plan: diff --git a/src/blueapi/client/rest.py b/src/blueapi/client/rest.py index cacbc1656c..0bddb5c87c 100644 --- a/src/blueapi/client/rest.py +++ b/src/blueapi/client/rest.py @@ -11,8 +11,10 @@ start_as_current_span, ) from pydantic import BaseModel, TypeAdapter, ValidationError +from pydantic_core import PydanticSerializationError from blueapi import __version__ +from blueapi.client import client from blueapi.config import RestConfig from blueapi.service.authentication import JWTAuth, SessionManager from blueapi.service.model import ( @@ -241,7 +243,7 @@ def create_task(self, task: TaskRequest) -> TaskResponse: TaskResponse, method="POST", get_exception=_create_task_exceptions, - data=task.model_dump(), + data=task.model_dump(mode="json", fallback=_task_model_fallback), ) def clear_task(self, task_id: str) -> TaskResponse: @@ -363,3 +365,10 @@ def __getattr__(name: str): class ServiceUnavailableError(Exception): pass + + +def _task_model_fallback(obj: Any) -> Any: + """Fallback method for serializing TaskRequests""" + if isinstance(obj, client.DeviceRef): + return obj.name + raise PydanticSerializationError(f"Object of type {type(obj)} not serializable") diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index 169d727f62..125fc165c2 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -202,9 +202,9 @@ def test_get_child_device(mock_rest: Mock, client: BlueapiClient): else None ) foo = client.devices.foo - assert foo == "foo" + assert foo.name == "foo" x = client.devices.foo.x - assert x == "foo.x" + assert x.name == "foo.x" def test_state_property(client: BlueapiClient): diff --git a/tests/unit_tests/client/test_rest.py b/tests/unit_tests/client/test_rest.py index 0de1c94277..c3386ad552 100644 --- a/tests/unit_tests/client/test_rest.py +++ b/tests/unit_tests/client/test_rest.py @@ -8,9 +8,11 @@ import requests import responses from packaging.version import Version +from pydantic_core import PydanticSerializationError from responses import DELETE, GET, PUT, matchers from blueapi import __version__ +from blueapi.client.client import DeviceRef from blueapi.client.rest import ( BlueapiRestClient, BlueskyRemoteControlError, @@ -29,6 +31,7 @@ DeviceModel, EnvironmentResponse, PlanModel, + TaskRequest, TaskResponse, TasksListResponse, WorkerTask, @@ -77,6 +80,45 @@ def test_rest_error_code( rest.get_plans() +def test_create_task_serialization(): + rest = Mock(spec=BlueapiRestClient) + request = TaskRequest( + name="demo", + instrument_session="cm12345-1", + params={"devices": [DeviceRef(name="foo", cache=Mock(), model=Mock())]}, + ) + + BlueapiRestClient.create_task(rest, request) + + rest._request_and_deserialize.assert_called_once_with( + "/tasks", + TaskResponse, + method="POST", + get_exception=_create_task_exceptions, + data={ + "name": "demo", + "instrument_session": "cm12345-1", + "params": {"devices": ["foo"]}, + }, + ) + + +def test_create_task_serialization_error(): + class CustomType: + pass + + rest = Mock(spec=BlueapiRestClient) + request = TaskRequest( + name="demo", + instrument_session="cm12345-1", + params={"devices": [CustomType()]}, + ) + + with pytest.raises(PydanticSerializationError, match="not serializable"): + BlueapiRestClient.create_task(rest, request) + rest._request_and_deserialize.assert_not_called() + + @pytest.mark.parametrize( "code,content,expected_exception", [ From a7b5f35b78a3147fc1ed1c4cc2ffc099af79eff8 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 24 Jun 2026 13:39:18 +0100 Subject: [PATCH 04/13] fix: Install debugpy in container instead of adding it (#1528) Running `uv add debugpy` in the container modifies pyproject.toml and uv.lock so `git describe` reports local changes in the repo and appends a `dev0+g.d` suffix to the version used for the blueapi in the container Using `uv pip install debugpy` makes it available in the environment without modifying any tracked files. This has to be run after the `uv sync` to stop it being removed again. --- Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 41b79c25f3..b2486f1cbb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,7 +7,7 @@ RUN apt-get update -y && apt-get install -y --no-install-recommends \ graphviz \ && apt-get dist-clean -# Install helm for the dev container. This is the recommended +# Install helm for the dev container. This is the recommended # approach per the docs: https://helm.sh/docs/intro/install RUN curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3; \ chmod 700 get_helm.sh; \ @@ -27,12 +27,12 @@ RUN chmod o+wrX . # Tell uv sync to install python in a known location so we can copy it out later ENV UV_PYTHON_INSTALL_DIR=/python -RUN uv add debugpy - # Sync the project without its dev dependencies RUN --mount=type=cache,target=/root/.cache/uv \ uv sync --locked --no-editable --no-dev --managed-python +RUN uv pip install debugpy + # The runtime stage copies the built venv into a runtime container FROM ubuntu:resolute AS runtime From 707abfb50c59c2e0c250da16128138f165bc787d Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 24 Jun 2026 13:49:39 +0100 Subject: [PATCH 05/13] chore!: Remove dodal dependency (#1534) Remove DeviceSource and DodalSource and patch up tests --------- Co-authored-by: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com> --- docs/how-to/add-plans-and-devices.md | 2 +- helm/blueapi/config_schema.json | 58 ---------- helm/blueapi/values.schema.json | 54 --------- pyproject.toml | 2 +- src/blueapi/config.py | 21 +--- src/blueapi/core/bluesky_types.py | 3 +- src/blueapi/core/context.py | 68 +----------- src/blueapi/utils/__init__.py | 19 +++- src/blueapi/utils/connect_devices.py | 99 ----------------- .../unit_tests/code_examples/device_module.py | 6 +- tests/unit_tests/core/fake_device_module.py | 17 ++- .../core/fake_device_module_failing.py | 4 + tests/unit_tests/core/test_context.py | 103 +++--------------- .../example_beamline_with_path_provider.py | 16 --- tests/unit_tests/service/test_interface.py | 64 ----------- tests/unit_tests/service/test_runner.py | 16 ++- tests/unit_tests/test_config.py | 11 +- tests/unit_tests/test_example_code.py | 2 +- .../plans_and_devices.yaml | 2 +- tests/unit_tests/worker/devices.py | 6 +- tests/unit_tests/worker/test_task_worker.py | 6 +- uv.lock | 4 +- 22 files changed, 86 insertions(+), 497 deletions(-) delete mode 100644 src/blueapi/utils/connect_devices.py delete mode 100644 tests/unit_tests/service/example_beamline_with_path_provider.py diff --git a/docs/how-to/add-plans-and-devices.md b/docs/how-to/add-plans-and-devices.md index 35f09ff7b4..702497fe2f 100644 --- a/docs/how-to/add-plans-and-devices.md +++ b/docs/how-to/add-plans-and-devices.md @@ -26,7 +26,7 @@ To add plans, you would add the following into your configuration file: ``` -Devices are added similarly, using `dodal` as the `kind`, like so: +Devices are added similarly, using `deviceManager` as the `kind`, like so: ```{literalinclude} ../../tests/unit_tests/valid_example_config/plans_and_devices.yaml :language: yaml ``` diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index dd8a48433a..070c1ad981 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -100,56 +100,6 @@ "type": "object", "$id": "DeviceManagerSource" }, - "DeviceSource": { - "additionalProperties": false, - "properties": { - "module": { - "description": "Module to be imported", - "title": "Module", - "type": "string" - }, - "kind": { - "const": "deviceFunctions", - "default": "deviceFunctions", - "title": "Kind", - "type": "string" - } - }, - "required": [ - "module" - ], - "title": "DeviceSource", - "type": "object", - "$id": "DeviceSource" - }, - "DodalSource": { - "additionalProperties": false, - "properties": { - "module": { - "description": "Module to be imported", - "title": "Module", - "type": "string" - }, - "kind": { - "const": "dodal", - "default": "dodal", - "title": "Kind", - "type": "string" - }, - "mock": { - "default": false, - "description": "If true, ophyd_async device connections are mocked", - "title": "Mock", - "type": "boolean" - } - }, - "required": [ - "module" - ], - "title": "DodalSource", - "type": "object", - "$id": "DodalSource" - }, "EnvironmentConfig": { "additionalProperties": false, "description": "Config for the RunEngine environment", @@ -168,9 +118,7 @@ "items": { "discriminator": { "mapping": { - "deviceFunctions": "DeviceSource", "deviceManager": "DeviceManagerSource", - "dodal": "DodalSource", "planFunctions": "PlanSource" }, "propertyName": "kind" @@ -179,12 +127,6 @@ { "$ref": "PlanSource" }, - { - "$ref": "DeviceSource" - }, - { - "$ref": "DodalSource" - }, { "$ref": "DeviceManagerSource" } diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 60310135cc..71c8a53954 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -533,54 +533,6 @@ }, "additionalProperties": false }, - "DeviceSource": { - "$id": "DeviceSource", - "title": "DeviceSource", - "type": "object", - "required": [ - "module" - ], - "properties": { - "kind": { - "title": "Kind", - "default": "deviceFunctions", - "const": "deviceFunctions" - }, - "module": { - "title": "Module", - "description": "Module to be imported", - "type": "string" - } - }, - "additionalProperties": false - }, - "DodalSource": { - "$id": "DodalSource", - "title": "DodalSource", - "type": "object", - "required": [ - "module" - ], - "properties": { - "kind": { - "title": "Kind", - "default": "dodal", - "const": "dodal" - }, - "mock": { - "title": "Mock", - "description": "If true, ophyd_async device connections are mocked", - "default": false, - "type": "boolean" - }, - "module": { - "title": "Module", - "description": "Module to be imported", - "type": "string" - } - }, - "additionalProperties": false - }, "EnvironmentConfig": { "$id": "EnvironmentConfig", "title": "EnvironmentConfig", @@ -618,12 +570,6 @@ { "$ref": "PlanSource" }, - { - "$ref": "DeviceSource" - }, - { - "$ref": "DodalSource" - }, { "$ref": "DeviceManagerSource" } diff --git a/pyproject.toml b/pyproject.toml index 9f4231c76c..6dda4a1922 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,6 @@ dependencies = [ "fastapi>=0.112.0", "uvicorn", "requests", - "dls-dodal>=1.69.0", "super-state-machine", # https://github.com/DiamondLightSource/blueapi/issues/553 "GitPython", "event-model==1.23.1", # https://github.com/DiamondLightSource/blueapi/issues/684 @@ -48,6 +47,7 @@ requires-python = ">=3.11" dev = [ "ophyd_async[sim]", "copier", + "dls-dodal>=1.69.0", "myst-parser", "prek", "pydata-sphinx-theme>=0.15.4", diff --git a/src/blueapi/config.py b/src/blueapi/config.py index c56415bfe6..f150afa7d3 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -48,8 +48,6 @@ def _expand_env(loader: yaml.Loader, node: yaml.ScalarNode) -> str: class SourceKind(StrEnum): PLAN_FUNCTIONS = "planFunctions" - DEVICE_FUNCTIONS = "deviceFunctions" - DODAL = "dodal" DEVICE_MANAGER = "deviceManager" @@ -63,19 +61,6 @@ class PlanSource(Source): ) -class DeviceSource(Source): - kind: Literal[SourceKind.DEVICE_FUNCTIONS] = Field( - SourceKind.DEVICE_FUNCTIONS, init=False - ) - - -class DodalSource(Source): - kind: Literal[SourceKind.DODAL] = Field(SourceKind.DODAL, init=False) - mock: bool = Field( - description="If true, ophyd_async device connections are mocked", default=False - ) - - class DeviceManagerSource(Source): kind: Literal[SourceKind.DEVICE_MANAGER] = Field( SourceKind.DEVICE_MANAGER, init=False @@ -84,7 +69,9 @@ class DeviceManagerSource(Source): description="If true, ophyd_async device connections are mocked", default=False ) name: str = Field( - default="devices", description="Name of the device manager in the module" + default="devices", + description="Name of the device manager in the module", + exclude_if=lambda v: v == "devices", ) @@ -150,7 +137,7 @@ class EnvironmentConfig(BlueapiBaseModel): sources: list[ Annotated[ - PlanSource | DeviceSource | DodalSource | DeviceManagerSource, + PlanSource | DeviceManagerSource, Field(discriminator="kind"), ] ] = [ diff --git a/src/blueapi/core/bluesky_types.py b/src/blueapi/core/bluesky_types.py index a15b4752e2..d05904a926 100644 --- a/src/blueapi/core/bluesky_types.py +++ b/src/blueapi/core/bluesky_types.py @@ -24,13 +24,12 @@ WritesExternalAssets, ) from bluesky.utils import Msg, MsgGenerator -from dodal.common import PlanGenerator from ophyd_async.core import Device as AsyncDevice from pydantic import BaseModel, Field from blueapi.utils import BlueapiBaseModel -PlanWrapper = Callable[[MsgGenerator], MsgGenerator] +PlanGenerator = Callable[..., MsgGenerator] #: An object that encapsulates the device to do useful things to produce # data (e.g. move and read) diff --git a/src/blueapi/core/context.py b/src/blueapi/core/context.py index 60df7a9d70..2e5d67de24 100644 --- a/src/blueapi/core/context.py +++ b/src/blueapi/core/context.py @@ -9,8 +9,6 @@ from bluesky.protocols import HasName from bluesky.run_engine import RunEngine -from dodal.common.beamlines.beamline_utils import get_path_provider, set_path_provider -from dodal.utils import AnyDevice, make_all_devices from ophyd_async.core import NotConnectedError, PathProvider from pydantic import ( BaseModel, @@ -26,8 +24,6 @@ from blueapi.config import ( ApplicationConfig, DeviceManagerSource, - DeviceSource, - DodalSource, EnvironmentConfig, PlanSource, ServiceAccount, @@ -151,8 +147,6 @@ def __post_init__(self, configuration: ApplicationConfig | None): ) path_provider = StartDocumentPathProvider() - # TODO: Remove this when device manager is rolled out - set_path_provider(path_provider) self.run_engine.subscribe(path_provider.run_start, "start") self.run_engine.subscribe(path_provider.run_stop, "stop") @@ -173,13 +167,6 @@ async def _update_scan_num(md: dict[str, Any]) -> int: self.run_engine.scan_id_source = _update_scan_num self.with_config(configuration.env) - if configuration.numtracker and not isinstance( - get_path_provider(), StartDocumentPathProvider - ): - raise InvalidConfigError( - "Numtracker has been configured but a path provider was imported with " - "the devices. Remove this path provider to use numtracker." - ) if (tiled_conf := configuration.tiled) is not None and tiled_conf.enabled: if configuration.env.metadata is None: @@ -232,22 +219,6 @@ def with_config(self, config: EnvironmentConfig) -> None: case PlanSource(): LOGGER.info("Including plans from %s", source.module) self.with_plan_module(mod) - case DeviceSource(): - LOGGER.info("Including devices from %s", source.module) - LOGGER.warning( - "'devices' environment kind is deprecated - please convert " - "configuration to use deviceManager" - ) - self.with_device_module(mod) - case DodalSource(mock=mock): - LOGGER.info( - "Including devices from 'dodal' source %s", source.module - ) - LOGGER.warning( - "'dodal' environment kind is deprecated - please convert " - "configuration to use deviceManager" - ) - self.with_dodal_module(mod, mock=mock) case DeviceManagerSource(mock=mock, name=name): LOGGER.info( "Including devices from 'deviceManager' source %s:%s", @@ -326,50 +297,13 @@ def with_device_manager(self, manager: DeviceManager, mock: bool = False): ): LOGGER.warning("Device manager did not build any devices") - utils.report_successful_devices(build_result.devices, mock) + utils.report_successful_devices(build_result.devices, mock, LOGGER) return build_result.devices, { **build_result.build_errors, **build_result.connection_errors, } - def with_device_module(self, module: ModuleType) -> None: - self.with_dodal_module(module) - - def with_dodal_module( - self, module: ModuleType, **kwargs - ) -> tuple[dict[str, AnyDevice], dict[str, Exception]]: - """ - Discover all device factories in the specified module, - construct devices by invoking them and register them with the device context, - Then attempt to connect to all the devices. - - Args: - module: The python module to inspect for factories - kwargs: keyword arguments that will be passed to make_all_devices() and - to connect_devices() for construction and connection respectively - Returns: - A tuple containing a map of device name to devices, and a map of - device name to any exceptions encountered. - """ - devices, exceptions = make_all_devices(module, **kwargs) - - exceptions |= utils.connect_devices(self.run_engine, module, devices, **kwargs) - - for device in devices.values(): - self.register_device(device) - - # If exceptions have occurred, we log them but we do not make blueapi - # fall over - if len(exceptions) > 0: - LOGGER.warning( - f"{len(exceptions)} exceptions occurred while instantiating devices" - ) - LOGGER.exception(NotConnectedError(exceptions)) - elif not devices: - LOGGER.warning("No devices were loaded from dodal module %s", module) - return devices, exceptions - def register_plan(self, plan: PlanGenerator) -> PlanGenerator: """ Register the argument as a plan in the context. Can be used as a decorator e.g. diff --git a/src/blueapi/utils/__init__.py b/src/blueapi/utils/__init__.py index 4b2e41f2c1..e5cc851662 100644 --- a/src/blueapi/utils/__init__.py +++ b/src/blueapi/utils/__init__.py @@ -1,9 +1,9 @@ -from collections.abc import Callable +from collections.abc import Callable, Mapping from functools import wraps -from typing import ParamSpec, TypeVar +from logging import Logger +from typing import Any, ParamSpec, TypeVar from .base_model import BlueapiBaseModel, BlueapiModelConfig, BlueapiPlanModelConfig -from .connect_devices import connect_devices, report_successful_devices from .file_permissions import get_owner_gid, is_sgid_set from .invalid_config_error import InvalidConfigError from .modules import is_function_sourced_from_module, load_module_all @@ -20,7 +20,6 @@ "BlueapiPlanModelConfig", "InvalidConfigError", "NumtrackerClient", - "connect_devices", "report_successful_devices", "is_sgid_set", "get_owner_gid", @@ -32,6 +31,18 @@ Return = TypeVar("Return") +def report_successful_devices( + devices: Mapping[str, Any], sim_backend: bool, logger: Logger +) -> None: + sim_statement = " (sim mode)" if sim_backend else "" + connected_devices = "\n".join( + sorted([f"\t{device_name}" for device_name in devices.keys()]) + ) + + logger.info(f"{len(devices)} devices connected{sim_statement}:") + logger.info(connected_devices) + + def deprecated(alternative): from warnings import warn diff --git a/src/blueapi/utils/connect_devices.py b/src/blueapi/utils/connect_devices.py deleted file mode 100644 index d9d2172cc5..0000000000 --- a/src/blueapi/utils/connect_devices.py +++ /dev/null @@ -1,99 +0,0 @@ -import logging -from collections.abc import Mapping -from types import ModuleType - -from bluesky.run_engine import RunEngine -from dodal.utils import ( - AnyDevice, - DeviceInitializationController, - collect_factories, - filter_ophyd_devices, -) -from ophyd_async.core import NotConnectedError -from ophyd_async.plan_stubs import ensure_connected - -LOGGER = logging.getLogger(__name__) - - -def report_successful_devices( - devices: Mapping[str, AnyDevice], - sim_backend: bool, -) -> None: - sim_statement = " (sim mode)" if sim_backend else "" - connected_devices = "\n".join( - sorted([f"\t{device_name}" for device_name in devices.keys()]) - ) - - LOGGER.info(f"{len(devices)} devices connected{sim_statement}:") - LOGGER.info(connected_devices) - - -def _establish_device_connections( - run_engine: RunEngine, - devices: Mapping[str, AnyDevice], - sim_backend: bool, -) -> tuple[Mapping[str, AnyDevice], Mapping[str, Exception]]: - ophyd_devices, ophyd_async_devices = filter_ophyd_devices(devices) - exceptions = {} - - # Connect ophyd devices - for name, device in ophyd_devices.items(): - try: - device.wait_for_connection() - except Exception as ex: - exceptions[name] = ex - - # Connect ophyd-async devices - try: - run_engine(ensure_connected(*ophyd_async_devices.values(), mock=sim_backend)) - except NotConnectedError as ex: - exceptions = {**exceptions, **ex.sub_errors} - - # Only return the subset of devices that haven't raised an exception - successful_devices = { - name: device for name, device in devices.items() if name not in exceptions - } - return successful_devices, exceptions - - -def connect_devices( - run_engine: RunEngine, module: ModuleType, devices: dict[str, AnyDevice], **kwargs -) -> Mapping[str, Exception]: - factories = collect_factories(module, include_skipped=False) - - def is_simulated_device(name, factory, **kwargs): - device = devices.get(name, None) - mock_flag = kwargs.get("mock", kwargs.get("fake_with_ophyd_sim", False)) - return device is not None and ( - isinstance(factory, DeviceInitializationController) - and (factory._mock or mock_flag) # noqa: SLF001 - and isinstance(device, AnyDevice) - ) - - sim_devices = { - name: devices.get(name) - for name, factory in factories.items() - if is_simulated_device(name, factory, **kwargs) - } - real_devices = { - name: device - for name, device in devices.items() - if sim_devices.get(name, None) is None and (isinstance(device, AnyDevice)) - } - - exception_map = {} - if len(real_devices) > 0: - real_devices, exceptions = _establish_device_connections( - run_engine, real_devices, False - ) - report_successful_devices(real_devices, False) - exception_map |= exceptions - if len(sim_devices) > 0: - sim_devices, exceptions = _establish_device_connections( - run_engine, - sim_devices, # type: ignore - True, - ) - report_successful_devices(sim_devices, True) - exception_map |= exceptions - return exception_map diff --git a/tests/unit_tests/code_examples/device_module.py b/tests/unit_tests/code_examples/device_module.py index bdf3ac828a..6078401a62 100644 --- a/tests/unit_tests/code_examples/device_module.py +++ b/tests/unit_tests/code_examples/device_module.py @@ -1,7 +1,9 @@ -from dodal.common.beamlines.beamline_utils import device_factory +from dodal.device_manager import DeviceManager from dodal.devices.bimorph_mirror import BimorphMirror +devices = DeviceManager() -@device_factory(mock=True) + +@devices.factory(mock=True) def oav() -> BimorphMirror: return BimorphMirror("BLXXI-BMRPH-01:", number_of_channels=8) diff --git a/tests/unit_tests/core/fake_device_module.py b/tests/unit_tests/core/fake_device_module.py index 25b97d2cf1..48bf6dfd50 100644 --- a/tests/unit_tests/core/fake_device_module.py +++ b/tests/unit_tests/core/fake_device_module.py @@ -1,11 +1,13 @@ from unittest.mock import MagicMock, NonCallableMock -from dodal.common.beamlines.beamline_utils import device_factory -from dodal.utils import OphydV1Device, OphydV2Device +from dodal.device_manager import DeviceManager, OphydV1Device, OphydV2Device from ophyd_async.core import DEFAULT_TIMEOUT, LazyMock, StandardReadable from ophyd_async.sim import SimMotor +devices = DeviceManager() + +@devices.factory(mock=True) def fake_motor_bundle_b( fake_motor_x: SimMotor, fake_motor_y: SimMotor, @@ -13,6 +15,7 @@ def fake_motor_bundle_b( return SimMotor("motor_bundle_b") +@devices.factory(mock=True) def fake_motor_x() -> SimMotor: return SimMotor("motor_x") @@ -24,7 +27,7 @@ def __init__(self, name: str = "") -> None: super().__init__(name) -@device_factory(mock=True) +@devices.factory(mock=True) def device_a() -> DeviceA: return DeviceA() @@ -38,8 +41,9 @@ def wait_for_connection( raise RuntimeError(f"{self.name}: fake connection error for tests") -def ophyd_device() -> UnconnectableOphydDevice: - return UnconnectableOphydDevice(name="ophyd_device") +@devices.v1_init(UnconnectableOphydDevice, prefix="NOT:A:REAL:DEVICE") +def ophyd_device(dev: UnconnectableOphydDevice) -> UnconnectableOphydDevice: + return dev class UnconnectableOphydAsyncDevice(OphydV2Device): @@ -52,14 +56,17 @@ async def connect( raise RuntimeError(f"{self.name}: fake connection error for tests") +@devices.factory def ophyd_async_device() -> UnconnectableOphydAsyncDevice: return UnconnectableOphydAsyncDevice(name="ophyd_async_device") +@devices.factory def fake_motor_y() -> SimMotor: return SimMotor("motor_y") +@devices.factory def fake_motor_bundle_a( fake_motor_x: SimMotor, fake_motor_y: SimMotor, diff --git a/tests/unit_tests/core/fake_device_module_failing.py b/tests/unit_tests/core/fake_device_module_failing.py index 1fb64c9451..cec0de526d 100644 --- a/tests/unit_tests/core/fake_device_module_failing.py +++ b/tests/unit_tests/core/fake_device_module_failing.py @@ -1,5 +1,9 @@ +from dodal.device_manager import DeviceManager from ophyd_async.epics.motor import Motor +devices = DeviceManager() + +@devices.factory def failing_device() -> Motor: raise TimeoutError("FooBar") diff --git a/tests/unit_tests/core/test_context.py b/tests/unit_tests/core/test_context.py index 738a4c564c..23462a1480 100644 --- a/tests/unit_tests/core/test_context.py +++ b/tests/unit_tests/core/test_context.py @@ -4,7 +4,7 @@ from pathlib import Path from types import ModuleType, NoneType from typing import Any, Generic, TypeVar, Union -from unittest.mock import ANY, MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest import responses @@ -36,8 +36,6 @@ from blueapi.config import ( ApplicationConfig, DeviceManagerSource, - DeviceSource, - DodalSource, EnvironmentConfig, MetadataConfig, OIDCConfig, @@ -48,7 +46,6 @@ from blueapi.core import BlueskyContext, is_bluesky_compatible_device from blueapi.core.context import DefaultFactory, generic_bounds, qualified_name from blueapi.core.protocols import DeviceConnectResult, DeviceManager -from blueapi.utils.connect_devices import _establish_device_connections from blueapi.utils.invalid_config_error import InvalidConfigError SIM_MOTOR_NAME = "sim" @@ -265,78 +262,29 @@ def test_override_device_name(empty_context: BlueskyContext, sim_motor: Motor): def test_add_devices_from_module(empty_context: BlueskyContext): import tests.unit_tests.core.fake_device_module as device_module - empty_context.with_device_module(device_module) + empty_context.with_device_manager(device_module.devices) # type: ignore - protocol uses Any to avoid dependency on dodal assert { - "motor_x", - "motor_y", - "motor_bundle_a", - "motor_bundle_b", + "fake_motor_x", + "fake_motor_y", + "fake_motor_bundle_a", + "fake_motor_bundle_b", "device_a", - "ophyd_device", - "ophyd_async_device", } == empty_context.devices.keys() -def test_add_failing_deivces_from_module( +def test_add_failing_devices_from_module( caplog: LogCaptureFixture, empty_context: BlueskyContext ): import tests.unit_tests.core.fake_device_module_failing as device_module caplog.set_level(10) - empty_context.with_device_module(device_module) + empty_context.with_device_manager(device_module.devices) # type: ignore - protocol uses Any to avoid dependency on dodal logs = caplog.get_records("call") - assert any("TimeoutError: FooBar" in log.message for log in logs) + assert any("TimeoutError('FooBar')" in log.message for log in logs) assert len(empty_context.devices.keys()) == 0 -def test_extra_kwargs_in_with_dodal_module_passed_to_make_all_devices( - empty_context: BlueskyContext, -): - """ - Note that this functionality is currently used by hyperion. - """ - import tests.unit_tests.core.fake_device_module as device_module - - with patch( - "blueapi.core.context.make_all_devices", - return_value=({}, {}), - ) as mock_make_all_devices: - empty_context.with_dodal_module( - device_module, some_argument=1, another_argument="two" - ) - - mock_make_all_devices.assert_called_once_with( - device_module, some_argument=1, another_argument="two" - ) - - -def test_with_dodal_module_returns_connection_exceptions(empty_context: BlueskyContext): - import tests.unit_tests.core.fake_device_module as device_module - - def connect_sim_backend(run_engine: RunEngine, devices, sim_backend): - return _establish_device_connections(run_engine, devices, True) - - with patch( - "blueapi.utils.connect_devices._establish_device_connections", - side_effect=connect_sim_backend, - ): - names_to_devices, exceptions = empty_context.with_dodal_module(device_module) - - assert set(names_to_devices.keys()) == { - "motor_y", - "ophyd_async_device", - "ophyd_device", - "device_a", - "motor_x", - "motor_bundle_a", - "motor_bundle_b", - } - assert len(exceptions) == 2 - assert isinstance(exceptions["ophyd_device"], RuntimeError) - assert isinstance(exceptions["ophyd_async_device"], RuntimeError) - - @pytest.mark.parametrize( "addr", ["sim", "sim_det", "sim.user_setpoint", ["sim"], ["sim", "user_setpoint"]] ) @@ -374,7 +322,7 @@ def test_add_devices_and_plans_from_modules_with_config( empty_context.with_config( EnvironmentConfig( sources=[ - DeviceSource( + DeviceManagerSource( module="tests.unit_tests.core.fake_device_module", ), PlanSource( @@ -384,13 +332,11 @@ def test_add_devices_and_plans_from_modules_with_config( ) ) assert { - "motor_x", - "motor_y", - "motor_bundle_a", - "motor_bundle_b", + "fake_motor_x", + "fake_motor_y", + "fake_motor_bundle_a", + "fake_motor_bundle_b", "device_a", - "ophyd_device", - "ophyd_async_device", } == empty_context.devices.keys() assert EXPECTED_PLANS == empty_context.plans.keys() @@ -407,27 +353,6 @@ def test_add_metadata_with_config( assert md in empty_context.run_engine.md.items() -@pytest.mark.parametrize("mock", [True, False]) -def test_with_config_passes_mock_to_with_dodal_module( - empty_context: BlueskyContext, - mock: bool, -): - with patch.object(empty_context, "with_dodal_module") as mock_with_dodal_module: - empty_context.with_config( - EnvironmentConfig( - sources=[ - DodalSource( - module="tests.unit_tests.core.fake_device_module", mock=mock - ), - PlanSource( - module="tests.unit_tests.core.fake_plan_module", - ), - ] - ) - ) - mock_with_dodal_module.assert_called_once_with(ANY, mock=mock) - - def test_function_spec(empty_context: BlueskyContext): spec = empty_context._type_spec_for_function(has_some_params) assert spec["foo"][0] is int diff --git a/tests/unit_tests/service/example_beamline_with_path_provider.py b/tests/unit_tests/service/example_beamline_with_path_provider.py deleted file mode 100644 index 83b016eec7..0000000000 --- a/tests/unit_tests/service/example_beamline_with_path_provider.py +++ /dev/null @@ -1,16 +0,0 @@ -from pathlib import Path - -from dodal.common.beamlines.beamline_utils import ( - set_path_provider, -) -from dodal.common.visit import LocalDirectoryServiceClient, StaticVisitPathProvider - -BL = "test" - -set_path_provider( - StaticVisitPathProvider( - BL, - Path("/tmp"), - client=LocalDirectoryServiceClient(), - ) -) diff --git a/tests/unit_tests/service/test_interface.py b/tests/unit_tests/service/test_interface.py index ccef2f0971..39115a9a21 100644 --- a/tests/unit_tests/service/test_interface.py +++ b/tests/unit_tests/service/test_interface.py @@ -9,11 +9,6 @@ from bluesky.protocols import Stoppable from bluesky.utils import MsgGenerator from bluesky_stomp.messaging import StompClient -from dodal.common.beamlines.beamline_utils import ( - clear_path_provider, - get_path_provider, - set_path_provider, -) from ophyd_async.epics.motor import Motor from pydantic import HttpUrl from pytest_httpx import HTTPXMock @@ -25,7 +20,6 @@ MetadataConfig, NumtrackerConfig, OIDCConfig, - PlanSource, ScratchConfig, StompConfig, TiledConfig, @@ -44,7 +38,6 @@ ) from blueapi.utils.invalid_config_error import InvalidConfigError from blueapi.utils.numtracker import NumtrackerClient -from blueapi.utils.path_provider import StartDocumentPathProvider from blueapi.worker.event import ( TaskResult, TaskStatus, @@ -613,63 +606,6 @@ def test_numtracker_requires_instrument_metadata(): interface.set_config(ApplicationConfig()) -def test_setup_without_numtracker_with_existing_provider_does_not_overwrite_provider(): - conf = ApplicationConfig() - mock_provider = Mock() - set_path_provider(mock_provider) - - assert get_path_provider() == mock_provider - interface.setup(conf) - assert get_path_provider() == mock_provider - - clear_path_provider() - - -def test_setup_without_numtracker_without_existing_provider_does_not_make_one(): - conf = ApplicationConfig() - interface.setup(conf) - - with pytest.raises(NameError): - get_path_provider() - - -def test_setup_with_numtracker_makes_start_document_provider(): - conf = ApplicationConfig( - env=EnvironmentConfig(metadata=MetadataConfig(instrument="p46")), - numtracker=NumtrackerConfig(), - ) - interface.setup(conf) - - path_provider = get_path_provider() - - assert isinstance(path_provider, StartDocumentPathProvider) - - clear_path_provider() - - -def test_setup_with_numtracker_raises_if_provider_is_defined_in_device_module(): - conf = ApplicationConfig( - env=EnvironmentConfig( - sources=[ - PlanSource( - module="tests.unit_tests.service.example_beamline_with_path_provider", - ), - ], - metadata=MetadataConfig(instrument="p46"), - ), - numtracker=NumtrackerConfig(), - ) - - with pytest.raises( - InvalidConfigError, - match="Numtracker has been configured but a path provider was imported" - " with the devices. Remove this path provider to use numtracker.", - ): - interface.setup(conf) - - clear_path_provider() - - @patch("blueapi.utils.numtracker.NumtrackerClient.create_scan") async def test_numtracker_create_scan_called_with_arguments_from_metadata( mock_create_scan, diff --git a/tests/unit_tests/service/test_runner.py b/tests/unit_tests/service/test_runner.py index b139baccc3..b41b3fd4c7 100644 --- a/tests/unit_tests/service/test_runner.py +++ b/tests/unit_tests/service/test_runner.py @@ -2,7 +2,7 @@ from collections.abc import Callable from multiprocessing.pool import Pool as PoolClass from typing import Any, Generic, TypeVar -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, NonCallableMock, patch import pytest from observability_utils.tracing import ( @@ -188,19 +188,27 @@ def run_rpc_function( ) +fetchable_non_callable = NonCallableMock( + __name__="fetchable_non_callable", __module__=__name__ +) + + +def wrong_return_type() -> str: + return "" + + def test_non_callable_excepts(started_runner: WorkerDispatcher): # Not a valid target on main or sub process - from tests.unit_tests.core.fake_device_module import fetchable_non_callable + # from tests.unit_tests.core.fake_device_module import fetchable_non_callable with pytest.raises( RpcError, match="fetchable_non_callable: Object in subprocess is not a function", ): - run_rpc_function(fetchable_non_callable, Mock) + run_rpc_function(fetchable_non_callable, Mock) # type: ignore - the error is the point def test_clear_message_for_wrong_return(started_runner: WorkerDispatcher): - from tests.unit_tests.core.fake_device_module import wrong_return_type with pytest.raises( ValidationError, diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 5cbb00c1b7..8498e197f3 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -230,7 +230,8 @@ def temp_yaml_config_file( { "env": { "sources": [ - {"kind": "dodal", "module": "dodal.adsim"}, + {"kind": "deviceManager", "module": "dodal.adsim"}, + {"kind": "deviceManager", "module": "dodal.ixx", "name": "manager"}, {"kind": "planFunctions", "module": "dodal.plans"}, {"kind": "planFunctions", "module": "dodal.plan_stubs.wrapped"}, ], @@ -241,7 +242,7 @@ def temp_yaml_config_file( "stomp": {"enabled": True}, "env": { "sources": [ - {"kind": "dodal", "module": "dodal.adsim"}, + {"kind": "deviceManager", "module": "dodal.adsim"}, {"kind": "planFunctions", "module": "dodal.plans"}, {"kind": "planFunctions", "module": "dodal.plan_stubs.wrapped"}, ], @@ -301,7 +302,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): "instrument": "p01", }, "sources": [ - {"kind": "dodal", "module": "dodal.adsim", "mock": True}, + {"kind": "deviceManager", "module": "dodal.adsim", "mock": True}, {"kind": "planFunctions", "module": "dodal.plans"}, { "kind": "planFunctions", @@ -357,7 +358,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): "auth_token_path": None, "env": { "sources": [ - {"kind": "dodal", "module": "dodal.adsim", "mock": False}, + {"kind": "deviceManager", "module": "dodal.adsim", "mock": False}, {"kind": "planFunctions", "module": "dodal.plans"}, { "kind": "planFunctions", @@ -439,7 +440,7 @@ def test_config_yaml_parsed_complete(temp_yaml_config_file: dict): "auth_token_path": None, "env": { "sources": [ - {"kind": "dodal", "module": "dodal.adsim"}, + {"kind": "deviceManager", "module": "dodal.adsim"}, {"kind": "planFunctions", "module": "dodal.plans"}, {"kind": "planFunctions", "module": "dodal.plan_stubs.wrapped"}, ], diff --git a/tests/unit_tests/test_example_code.py b/tests/unit_tests/test_example_code.py index e5788468f9..0d16a871f7 100644 --- a/tests/unit_tests/test_example_code.py +++ b/tests/unit_tests/test_example_code.py @@ -31,7 +31,7 @@ def test_example_device_module_is_detectable(): context = BlueskyContext() module = importlib.import_module(module_name) - context.with_dodal_module(module) + context.with_device_manager(module.devices) assert device_name in context.devices diff --git a/tests/unit_tests/valid_example_config/plans_and_devices.yaml b/tests/unit_tests/valid_example_config/plans_and_devices.yaml index d12ec638a0..c8cd7012b0 100644 --- a/tests/unit_tests/valid_example_config/plans_and_devices.yaml +++ b/tests/unit_tests/valid_example_config/plans_and_devices.yaml @@ -2,7 +2,7 @@ env: sources: - kind: planFunctions module: my_plan_library.tomography.plans - - kind: dodal + - kind: deviceManager module: dodal.beamlines.i04 mock: true - kind: deviceManager diff --git a/tests/unit_tests/worker/devices.py b/tests/unit_tests/worker/devices.py index a291c950e8..23d5f56cfe 100644 --- a/tests/unit_tests/worker/devices.py +++ b/tests/unit_tests/worker/devices.py @@ -1,7 +1,9 @@ -from dodal.common.beamlines.beamline_utils import device_factory +from dodal.device_manager import DeviceManager from ophyd_async.epics.motor import Motor +devices = DeviceManager() -@device_factory(mock=True) + +@devices.factory(mock=True) def motor() -> Motor: return Motor("FOO:") diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index ee5db34f7f..41041c5018 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -20,7 +20,7 @@ ) from ophyd_async.core import AsyncStatus -from blueapi.config import DeviceSource, EnvironmentConfig +from blueapi.config import DeviceManagerSource, EnvironmentConfig from blueapi.core import BlueskyContext, EventStream from blueapi.core.bluesky_types import DataEvent from blueapi.service.model import PlanModel @@ -110,7 +110,7 @@ def second_fake_device() -> FakeDevice: def context(fake_device: FakeDevice, second_fake_device: FakeDevice) -> BlueskyContext: ctx = BlueskyContext() ctx_config = EnvironmentConfig() - ctx_config.sources.append(DeviceSource(module="devices")) + ctx_config.sources.append(DeviceManagerSource(module="devices")) ctx.register_plan(failing_plan) ctx.register_device(fake_device) ctx.register_device(second_fake_device) @@ -122,7 +122,7 @@ def context(fake_device: FakeDevice, second_fake_device: FakeDevice) -> BlueskyC def context_without_devices() -> BlueskyContext: ctx = BlueskyContext() ctx_config = EnvironmentConfig() - ctx_config.sources.append(DeviceSource(module="devices")) + ctx_config.sources.append(DeviceManagerSource(module="devices")) ctx.with_config(ctx_config) return ctx diff --git a/uv.lock b/uv.lock index eab94a8670..9cc444d6b0 100644 --- a/uv.lock +++ b/uv.lock @@ -424,7 +424,6 @@ dependencies = [ { name = "bluesky", extra = ["plotting"] }, { name = "bluesky-stomp" }, { name = "click" }, - { name = "dls-dodal" }, { name = "event-model" }, { name = "fastapi" }, { name = "gitpython" }, @@ -450,6 +449,7 @@ dependencies = [ dev = [ { name = "copier" }, { name = "deepdiff" }, + { name = "dls-dodal" }, { name = "jwcrypto" }, { name = "mock" }, { name = "myst-parser" }, @@ -486,7 +486,6 @@ requires-dist = [ { name = "bluesky", extras = ["plotting"], specifier = ">=1.14.0" }, { name = "bluesky-stomp", specifier = ">=0.1.6" }, { name = "click", specifier = ">=8.2.0" }, - { name = "dls-dodal", specifier = ">=1.69.0" }, { name = "event-model", specifier = "==1.23.1" }, { name = "fastapi", specifier = ">=0.112.0" }, { name = "gitpython" }, @@ -512,6 +511,7 @@ requires-dist = [ dev = [ { name = "copier" }, { name = "deepdiff" }, + { name = "dls-dodal", specifier = ">=1.69.0" }, { name = "jwcrypto" }, { name = "mock" }, { name = "myst-parser" }, From 50fd3dd4ea605ee843cead45c607f15b64e3b4a1 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 24 Jun 2026 14:41:10 +0100 Subject: [PATCH 06/13] refactor: Rewrite server middleware with websocket support (#1473) The builtin fastapi support for middleware only supports http/rest requests. To enable the same middleware for websockets, a new implementation of the starlette middleware is required and it makes sense to use the same implementation for both protocols. For rest requests, there should not be any change in behaviour from this change. --- src/blueapi/service/main.py | 43 ++------ src/blueapi/service/middleware.py | 58 +++++++++++ tests/unit_tests/service/test_main.py | 4 +- tests/unit_tests/service/test_middleware.py | 108 ++++++++++++++++++++ 4 files changed, 175 insertions(+), 38 deletions(-) create mode 100644 src/blueapi/service/middleware.py create mode 100644 tests/unit_tests/service/test_middleware.py diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index a84a85b294..432cc54550 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -24,19 +24,20 @@ get_tracer, start_as_current_span, ) -from opentelemetry.context import attach from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor -from opentelemetry.propagate import get_global_textmap from opentelemetry.trace import get_tracer_provider from pydantic import ValidationError from pydantic.json_schema import SkipJsonSchema from starlette.responses import JSONResponse from super_state_machine.errors import TransitionError -from blueapi import __version__ from blueapi.config import ApplicationConfig, OIDCConfig, Tag from blueapi.service import interface from blueapi.service.authentication import Fedid, build_access_token_check +from blueapi.service.middleware import ( + ObservabilityContextPropagator, + VersionHeaders, +) from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum @@ -132,8 +133,9 @@ def get_app(config: ApplicationConfig): app.include_router(secure_router, dependencies=dependencies) app.add_exception_handler(KeyError, on_key_error_404) app.add_exception_handler(jwt.PyJWTError, on_token_error_401) - app.middleware("http")(add_version_headers) - app.middleware("http")(inject_propagated_observability_context) + + app.add_middleware(ObservabilityContextPropagator) + app.add_middleware(VersionHeaders) app.middleware("http")(log_request_details) if config.api.cors: app.add_middleware( @@ -589,15 +591,6 @@ def start(config: ApplicationConfig): ) -async def add_version_headers( - request: Request, call_next: Callable[[Request], Awaitable[Response]] -): - response = await call_next(request) - response.headers["X-API-Version"] = ApplicationConfig.REST_API_VERSION - response.headers["X-BlueAPI-Version"] = __version__ - return response - - async def log_request_details( request: Request, call_next: Callable[[Request], Awaitable[StreamingResponse]] ) -> Response: @@ -617,25 +610,3 @@ async def log_request_details( log(log_message, extra=extra) return response - - -async def inject_propagated_observability_context( - request: Request, call_next: Callable[[Request], Awaitable[Response]] -) -> Response: - """Middleware to extract any propagated observability context from the - HTTP headers and attach it to the local one. - """ - headers = request.headers - if ApplicationConfig.CONTEXT_HEADER in headers: - carrier = { - ApplicationConfig.CONTEXT_HEADER: headers[ApplicationConfig.CONTEXT_HEADER] - } - if ApplicationConfig.VENDOR_CONTEXT_HEADER in headers: - carrier[ApplicationConfig.VENDOR_CONTEXT_HEADER] = headers[ - ApplicationConfig.VENDOR_CONTEXT_HEADER - ] - ctx = get_global_textmap().extract(carrier) - - attach(ctx) - response = await call_next(request) - return response diff --git a/src/blueapi/service/middleware.py b/src/blueapi/service/middleware.py new file mode 100644 index 0000000000..b31fe0fb91 --- /dev/null +++ b/src/blueapi/service/middleware.py @@ -0,0 +1,58 @@ +import logging + +from opentelemetry.context import attach +from opentelemetry.propagate import get_global_textmap +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +from blueapi import __version__ +from blueapi.config import ApplicationConfig + +OBS_LOGGER = logging.getLogger("blueapi.service.middleware.observability") + +CONTEXT_HEADER = ApplicationConfig.CONTEXT_HEADER.encode() +VENDOR_CONTEXT_HEADER = ApplicationConfig.VENDOR_CONTEXT_HEADER.encode() + +API_VERSION = (b"x-api-version", ApplicationConfig.REST_API_VERSION.encode("utf-8")) +VERSION = (b"x-blueapi-version", __version__.encode("utf-8")) + + +class VersionHeaders: + def __init__(self, app: ASGIApp): + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send): + if scope.get("type") not in ("websocket", "http"): + return await self.app(scope, receive, send) + + async def local_send(message: Message): + if message["type"] in ("websocket.accept", "http.response.start"): + message["headers"].append(VERSION) + message["headers"].append(API_VERSION) + await send(message) + + return await self.app(scope, receive, local_send) + + +class ObservabilityContextPropagator: + def __init__(self, app: ASGIApp): + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send): + if scope.get("type") not in ("http", "websocket"): + return await self.app(scope, receive, send) + + ctx = None + v_ctx = None + for key, val in scope.get("headers", ()): + if key == CONTEXT_HEADER: + ctx = val.decode() + elif key == VENDOR_CONTEXT_HEADER: + v_ctx = val.decode() + if ctx: + OBS_LOGGER.debug("Propagating observability context: %s, %s", ctx, v_ctx) + carrier = {ApplicationConfig.CONTEXT_HEADER: ctx} + if v_ctx: + carrier[ApplicationConfig.VENDOR_CONTEXT_HEADER] = v_ctx + attach(get_global_textmap().extract(carrier)) + + return await self.app(scope, receive, send) diff --git a/tests/unit_tests/service/test_main.py b/tests/unit_tests/service/test_main.py index 1801b8756b..5a48a9dd0d 100644 --- a/tests/unit_tests/service/test_main.py +++ b/tests/unit_tests/service/test_main.py @@ -8,16 +8,16 @@ from blueapi import __version__ from blueapi.config import ApplicationConfig from blueapi.service.main import ( - add_version_headers, get_passthrough_headers, lifespan, log_request_details, ) +from blueapi.service.middleware import VersionHeaders async def test_add_version_header(): app = FastAPI() - app.middleware("http")(add_version_headers) + app.add_middleware(VersionHeaders) @app.get("/") async def root(): diff --git a/tests/unit_tests/service/test_middleware.py b/tests/unit_tests/service/test_middleware.py new file mode 100644 index 0000000000..5f6dbeac6b --- /dev/null +++ b/tests/unit_tests/service/test_middleware.py @@ -0,0 +1,108 @@ +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest +from starlette.types import ASGIApp + +from blueapi.config import ApplicationConfig +from blueapi.service.middleware import ( + API_VERSION, + CONTEXT_HEADER, + VENDOR_CONTEXT_HEADER, + VERSION, + ObservabilityContextPropagator, + VersionHeaders, +) + + +@pytest.fixture +def app(): + return AsyncMock(spec=ASGIApp) + + +@pytest.mark.parametrize( + "protocol,message_type", + [("http", "http.response.start"), ("websocket", "websocket.accept")], +) +async def test_version_headers_added(app: Mock, protocol: str, message_type: str): + vh = VersionHeaders(app) + + send = AsyncMock() + scope = {"type": protocol} + await vh(scope, Mock(), send) + + # the middleware wraps the send function so we need to extract the function + # the app was actually called with + local_send = app.call_args[0][2] + + # Calling the wrapped send method here is equivalent to what the downstream + # framework would do after the middleware has done its thing + message = {"type": message_type, "headers": []} + await local_send(message) + + # Check the headers were sent to the original send method + send.assert_called_once_with( + {"type": message_type, "headers": [VERSION, API_VERSION]} + ) + + +async def test_version_headers_ignore_non_http_or_websockets(app: Mock): + vh = VersionHeaders(app) + + scope = {"type": "other"} + send = Mock() + recv = Mock() + + await vh(scope, recv, send) + + # for non-http/ws requests, the original args are passed directly + app.assert_called_once_with(scope, recv, send) + + +async def test_obs_context_ignores_non_http_or_websockets(app: Mock): + ocp = ObservabilityContextPropagator(app) + + scope = MagicMock() + scope.__getitem__.side_effect = {"type": "other"}.__getitem__ + + with patch("blueapi.service.middleware.attach") as att: + await ocp(scope, Mock(), Mock()) + + att.assert_not_called() + scope.get.assert_called_once_with("type") + + +@pytest.mark.parametrize("protocol", ["http", "websocket"]) +async def test_obs_context_passes_context(app: Mock, protocol: str): + ocp = ObservabilityContextPropagator(app) + scope = {"type": protocol, "headers": ((CONTEXT_HEADER, b"req_context"),)} + + with patch("blueapi.service.middleware.attach") as att: + with patch("blueapi.service.middleware.get_global_textmap") as get_global: + get_global.return_value.extract.side_effect = lambda x: x + await ocp(scope, Mock(), Mock()) + + att.assert_called_once_with({ApplicationConfig.CONTEXT_HEADER: "req_context"}) + + +@pytest.mark.parametrize("protocol", ["http", "websocket"]) +async def test_obs_context_passes_vendor_context(app: Mock, protocol: str): + ocp = ObservabilityContextPropagator(app) + scope = { + "type": protocol, + "headers": ( + (CONTEXT_HEADER, b"req_context"), + (VENDOR_CONTEXT_HEADER, b"vendor_context"), + ), + } + + with patch("blueapi.service.middleware.attach") as att: + with patch("blueapi.service.middleware.get_global_textmap") as get_global: + get_global.return_value.extract.side_effect = lambda x: x + await ocp(scope, Mock(), Mock()) + + att.assert_called_once_with( + { + ApplicationConfig.CONTEXT_HEADER: "req_context", + ApplicationConfig.VENDOR_CONTEXT_HEADER: "vendor_context", + } + ) From bd58313133573092bb7f87251959318ed78fb6d2 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 25 Jun 2026 12:08:45 +0100 Subject: [PATCH 07/13] fix: Clear client caches when environment is reloaded (#1509) --- src/blueapi/client/client.py | 6 ++++++ tests/unit_tests/client/test_client.py | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 034b9f6a98..20de158929 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -3,6 +3,7 @@ import time from collections.abc import Iterable from concurrent.futures import Future +from contextlib import suppress from functools import cached_property from itertools import chain from pathlib import Path @@ -675,6 +676,11 @@ def reload_environment( """ try: status = self._rest.delete_environment() + # clear the cached plans/devices as they may have changed + with suppress(AttributeError): + del self.plans + with suppress(AttributeError): + del self.devices except ( BlueskyRequestError, ServiceUnavailableError, diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index 125fc165c2..eaf96a3b2d 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -292,6 +292,25 @@ def test_reload_environment( assert environment == NEW_ENV +def test_reload_environment_removes_caches(client: BlueapiClient, mock_rest: Mock): + mock_rest.get_environment.return_value = NEW_ENV + + _ = client.plans, client.devices + _ = client.plans, client.devices + + # rest calls only made once as plans/devices are cached + mock_rest.get_plans.assert_called_once() + mock_rest.get_devices.assert_called_once() + mock_rest.reset_mock() + + client.reload_environment() + _ = client.plans, client.devices + + # When environment is reloaded, caches are cleared so another call is required + mock_rest.get_plans.assert_called_once() + mock_rest.get_devices.assert_called_once() + + @patch("blueapi.client.client.time.time") @patch("blueapi.client.client.time.sleep") def test_reload_environment_no_timeout( From a7c7d93c7ab249d6ef7bf3de8271d9481fb6ebe3 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 25 Jun 2026 12:42:52 +0100 Subject: [PATCH 08/13] feat: run unit tests in parallel (#1566) As we have a 4 core GitHub action run we can utilise it more efficiently by running all our unit test in parallel. Currently it take the following amount of time to run unit test - python3.11: 2m vs now 59s - python3.12: 2m 1s vs now 1m 28s - python3.13: 1m 44s vs now 1m 12s It is consistently less in the future it will be a nice addition to run system test in parallel as well. It will require some more work so will do it in another PR. --- justfile | 2 +- pyproject.toml | 5 ++++- uv.lock | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/justfile b/justfile index 7d8280b5b5..269564d712 100644 --- a/justfile +++ b/justfile @@ -26,7 +26,7 @@ lint: uv run pyright src tests unit *OPTS: - uv run pytest tests/unit_tests {{ OPTS }} + uv run pytest -n logical tests/unit_tests {{ OPTS }} system *OPTS: uv run pytest tests/system_tests {{ OPTS }} diff --git a/pyproject.toml b/pyproject.toml index 6dda4a1922..bf29c45965 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,6 +75,7 @@ dev = [ "deepdiff", "tiled[minimal-server]>=0.2.4", # For system-test of dls.py "respx", + "pytest-xdist[psutil]", ] [project.scripts] @@ -102,7 +103,7 @@ filterwarnings = ["error", "ignore::DeprecationWarning"] # Doctest python code in docs, python code in src docstrings, test functions in tests testpaths = "docs src tests" asyncio_mode = "auto" -timeout = 3 +timeout = 10 [tool.coverage.run] patch = ["subprocess"] @@ -155,6 +156,8 @@ description = "Run unit tests with coverage" commands = [ [ "pytest", + "-n", + "logical", "tests/unit_tests", "--cov=blueapi", "--cov-report", diff --git a/uv.lock b/uv.lock index 9cc444d6b0..bdedf88984 100644 --- a/uv.lock +++ b/uv.lock @@ -462,6 +462,7 @@ dev = [ { name = "pytest-cov" }, { name = "pytest-httpx" }, { name = "pytest-timeout" }, + { name = "pytest-xdist", extra = ["psutil"] }, { name = "responses" }, { name = "respx" }, { name = "ruff" }, @@ -524,6 +525,7 @@ dev = [ { name = "pytest-cov" }, { name = "pytest-httpx", specifier = ">=0.35.0" }, { name = "pytest-timeout" }, + { name = "pytest-xdist", extras = ["psutil"] }, { name = "responses" }, { name = "respx" }, { name = "ruff" }, @@ -1339,6 +1341,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/fb/32/e31e3363bf48ad2ba80b644b01ad9676ce154f1b755950de81eb4ed5b6bd/event_model-1.23.1-py3-none-any.whl", hash = "sha256:e0b951b829cebcf3879beff238bb370fd997d315856bc5d5ac2a66202b854958", size = 77057, upload-time = "2025-08-28T13:26:37.228Z" }, ] +[[package]] +name = "execnet" +version = "2.1.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/bf/89/780e11f9588d9e7128a3f87788354c7946a9cbb1401ad38a48c4db9a4f07/execnet-2.1.2.tar.gz", hash = "sha256:63d83bfdd9a23e35b9c6a3261412324f964c2ec8dcd8d3c6916ee9373e0befcd", size = 166622, upload-time = "2025-11-12T09:56:37.75Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ab/84/02fc1827e8cdded4aa65baef11296a9bbe595c474f0d6d758af082d849fd/execnet-2.1.2-py3-none-any.whl", hash = "sha256:67fba928dd5a544b783f6056f449e5e3931a5c378b128bc18501f7ea79e296ec", size = 40708, upload-time = "2025-11-12T09:56:36.333Z" }, +] + [[package]] name = "fastapi" version = "0.136.1" @@ -3960,6 +3971,34 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/c4/72/02445137af02769918a93807b2b7890047c32bfb9f90371cbc12688819eb/protobuf-6.33.6-py3-none-any.whl", hash = "sha256:77179e006c476e69bf8e8ce866640091ec42e1beb80b213c3900006ecfba6901", size = 170656, upload-time = "2026-03-18T19:04:59.826Z" }, ] +[[package]] +name = "psutil" +version = "7.2.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/aa/c6/d1ddf4abb55e93cebc4f2ed8b5d6dbad109ecb8d63748dd2b20ab5e57ebe/psutil-7.2.2.tar.gz", hash = "sha256:0746f5f8d406af344fd547f1c8daa5f5c33dbc293bb8d6a16d80b4bb88f59372", size = 493740, upload-time = "2026-01-28T18:14:54.428Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/51/08/510cbdb69c25a96f4ae523f733cdc963ae654904e8db864c07585ef99875/psutil-7.2.2-cp313-cp313t-macosx_10_13_x86_64.whl", hash = "sha256:2edccc433cbfa046b980b0df0171cd25bcaeb3a68fe9022db0979e7aa74a826b", size = 130595, upload-time = "2026-01-28T18:14:57.293Z" }, + { url = "https://files.pythonhosted.org/packages/d6/f5/97baea3fe7a5a9af7436301f85490905379b1c6f2dd51fe3ecf24b4c5fbf/psutil-7.2.2-cp313-cp313t-macosx_11_0_arm64.whl", hash = "sha256:e78c8603dcd9a04c7364f1a3e670cea95d51ee865e4efb3556a3a63adef958ea", size = 131082, upload-time = "2026-01-28T18:14:59.732Z" }, + { url = "https://files.pythonhosted.org/packages/37/d6/246513fbf9fa174af531f28412297dd05241d97a75911ac8febefa1a53c6/psutil-7.2.2-cp313-cp313t-manylinux2010_x86_64.manylinux_2_12_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:1a571f2330c966c62aeda00dd24620425d4b0cc86881c89861fbc04549e5dc63", size = 181476, upload-time = "2026-01-28T18:15:01.884Z" }, + { url = "https://files.pythonhosted.org/packages/b8/b5/9182c9af3836cca61696dabe4fd1304e17bc56cb62f17439e1154f225dd3/psutil-7.2.2-cp313-cp313t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:917e891983ca3c1887b4ef36447b1e0873e70c933afc831c6b6da078ba474312", size = 184062, upload-time = "2026-01-28T18:15:04.436Z" }, + { url = "https://files.pythonhosted.org/packages/16/ba/0756dca669f5a9300d0cbcbfae9a4c30e446dfc7440ffe43ded5724bfd93/psutil-7.2.2-cp313-cp313t-win_amd64.whl", hash = "sha256:ab486563df44c17f5173621c7b198955bd6b613fb87c71c161f827d3fb149a9b", size = 139893, upload-time = "2026-01-28T18:15:06.378Z" }, + { url = "https://files.pythonhosted.org/packages/1c/61/8fa0e26f33623b49949346de05ec1ddaad02ed8ba64af45f40a147dbfa97/psutil-7.2.2-cp313-cp313t-win_arm64.whl", hash = "sha256:ae0aefdd8796a7737eccea863f80f81e468a1e4cf14d926bd9b6f5f2d5f90ca9", size = 135589, upload-time = "2026-01-28T18:15:08.03Z" }, + { url = "https://files.pythonhosted.org/packages/81/69/ef179ab5ca24f32acc1dac0c247fd6a13b501fd5534dbae0e05a1c48b66d/psutil-7.2.2-cp314-cp314t-macosx_10_15_x86_64.whl", hash = "sha256:eed63d3b4d62449571547b60578c5b2c4bcccc5387148db46e0c2313dad0ee00", size = 130664, upload-time = "2026-01-28T18:15:09.469Z" }, + { url = "https://files.pythonhosted.org/packages/7b/64/665248b557a236d3fa9efc378d60d95ef56dd0a490c2cd37dafc7660d4a9/psutil-7.2.2-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:7b6d09433a10592ce39b13d7be5a54fbac1d1228ed29abc880fb23df7cb694c9", size = 131087, upload-time = "2026-01-28T18:15:11.724Z" }, + { url = "https://files.pythonhosted.org/packages/d5/2e/e6782744700d6759ebce3043dcfa661fb61e2fb752b91cdeae9af12c2178/psutil-7.2.2-cp314-cp314t-manylinux2010_x86_64.manylinux_2_12_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:1fa4ecf83bcdf6e6c8f4449aff98eefb5d0604bf88cb883d7da3d8d2d909546a", size = 182383, upload-time = "2026-01-28T18:15:13.445Z" }, + { url = "https://files.pythonhosted.org/packages/57/49/0a41cefd10cb7505cdc04dab3eacf24c0c2cb158a998b8c7b1d27ee2c1f5/psutil-7.2.2-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:e452c464a02e7dc7822a05d25db4cde564444a67e58539a00f929c51eddda0cf", size = 185210, upload-time = "2026-01-28T18:15:16.002Z" }, + { url = "https://files.pythonhosted.org/packages/dd/2c/ff9bfb544f283ba5f83ba725a3c5fec6d6b10b8f27ac1dc641c473dc390d/psutil-7.2.2-cp314-cp314t-win_amd64.whl", hash = "sha256:c7663d4e37f13e884d13994247449e9f8f574bc4655d509c3b95e9ec9e2b9dc1", size = 141228, upload-time = "2026-01-28T18:15:18.385Z" }, + { url = "https://files.pythonhosted.org/packages/f2/fc/f8d9c31db14fcec13748d373e668bc3bed94d9077dbc17fb0eebc073233c/psutil-7.2.2-cp314-cp314t-win_arm64.whl", hash = "sha256:11fe5a4f613759764e79c65cf11ebdf26e33d6dd34336f8a337aa2996d71c841", size = 136284, upload-time = "2026-01-28T18:15:19.912Z" }, + { url = "https://files.pythonhosted.org/packages/e7/36/5ee6e05c9bd427237b11b3937ad82bb8ad2752d72c6969314590dd0c2f6e/psutil-7.2.2-cp36-abi3-macosx_10_9_x86_64.whl", hash = "sha256:ed0cace939114f62738d808fdcecd4c869222507e266e574799e9c0faa17d486", size = 129090, upload-time = "2026-01-28T18:15:22.168Z" }, + { url = "https://files.pythonhosted.org/packages/80/c4/f5af4c1ca8c1eeb2e92ccca14ce8effdeec651d5ab6053c589b074eda6e1/psutil-7.2.2-cp36-abi3-macosx_11_0_arm64.whl", hash = "sha256:1a7b04c10f32cc88ab39cbf606e117fd74721c831c98a27dc04578deb0c16979", size = 129859, upload-time = "2026-01-28T18:15:23.795Z" }, + { url = "https://files.pythonhosted.org/packages/b5/70/5d8df3b09e25bce090399cf48e452d25c935ab72dad19406c77f4e828045/psutil-7.2.2-cp36-abi3-manylinux2010_x86_64.manylinux_2_12_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:076a2d2f923fd4821644f5ba89f059523da90dc9014e85f8e45a5774ca5bc6f9", size = 155560, upload-time = "2026-01-28T18:15:25.976Z" }, + { url = "https://files.pythonhosted.org/packages/63/65/37648c0c158dc222aba51c089eb3bdfa238e621674dc42d48706e639204f/psutil-7.2.2-cp36-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:b0726cecd84f9474419d67252add4ac0cd9811b04d61123054b9fb6f57df6e9e", size = 156997, upload-time = "2026-01-28T18:15:27.794Z" }, + { url = "https://files.pythonhosted.org/packages/8e/13/125093eadae863ce03c6ffdbae9929430d116a246ef69866dad94da3bfbc/psutil-7.2.2-cp36-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:fd04ef36b4a6d599bbdb225dd1d3f51e00105f6d48a28f006da7f9822f2606d8", size = 148972, upload-time = "2026-01-28T18:15:29.342Z" }, + { url = "https://files.pythonhosted.org/packages/04/78/0acd37ca84ce3ddffaa92ef0f571e073faa6d8ff1f0559ab1272188ea2be/psutil-7.2.2-cp36-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:b58fabe35e80b264a4e3bb23e6b96f9e45a3df7fb7eed419ac0e5947c61e47cc", size = 148266, upload-time = "2026-01-28T18:15:31.597Z" }, + { url = "https://files.pythonhosted.org/packages/b4/90/e2159492b5426be0c1fef7acba807a03511f97c5f86b3caeda6ad92351a7/psutil-7.2.2-cp37-abi3-win_amd64.whl", hash = "sha256:eb7e81434c8d223ec4a219b5fc1c47d0417b12be7ea866e24fb5ad6e84b3d988", size = 137737, upload-time = "2026-01-28T18:15:33.849Z" }, + { url = "https://files.pythonhosted.org/packages/8c/c7/7bb2e321574b10df20cbde462a94e2b71d05f9bbda251ef27d104668306a/psutil-7.2.2-cp37-abi3-win_arm64.whl", hash = "sha256:8c233660f575a5a89e6d4cb65d9f938126312bca76d8fe087b947b3a1aaac9ee", size = 134617, upload-time = "2026-01-28T18:15:36.514Z" }, +] + [[package]] name = "pvxslibs" version = "1.5.1" @@ -4376,6 +4415,24 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/fa/b6/3127540ecdf1464a00e5a01ee60a1b09175f6913f0644ac748494d9c4b21/pytest_timeout-2.4.0-py3-none-any.whl", hash = "sha256:c42667e5cdadb151aeb5b26d114aff6bdf5a907f176a007a30b940d3d865b5c2", size = 14382, upload-time = "2025-05-05T19:44:33.502Z" }, ] +[[package]] +name = "pytest-xdist" +version = "3.8.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "execnet" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/78/b4/439b179d1ff526791eb921115fca8e44e596a13efeda518b9d845a619450/pytest_xdist-3.8.0.tar.gz", hash = "sha256:7e578125ec9bc6050861aa93f2d59f1d8d085595d6551c2c90b6f4fad8d3a9f1", size = 88069, upload-time = "2025-07-01T13:30:59.346Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ca/31/d4e37e9e550c2b92a9cbc2e4d0b7420a27224968580b5a447f420847c975/pytest_xdist-3.8.0-py3-none-any.whl", hash = "sha256:202ca578cfeb7370784a8c33d6d05bc6e13b4f25b5053c30a152269fd10f0b88", size = 46396, upload-time = "2025-07-01T13:30:56.632Z" }, +] + +[package.optional-dependencies] +psutil = [ + { name = "psutil" }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" From 8404420f046319a583d5e4e60211f168b30b3817 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 25 Jun 2026 14:03:58 +0100 Subject: [PATCH 09/13] fix: add missing values ApplicationConfig equality check (#1565) Currently the following configs are not checked for equality --- src/blueapi/config.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/blueapi/config.py b/src/blueapi/config.py index f150afa7d3..f58ae9b389 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -334,9 +334,14 @@ def __eq__(self, other: object) -> bool: if isinstance(other, ApplicationConfig): return ( (self.stomp == other.stomp) + & (self.tiled == other.tiled) & (self.env == other.env) & (self.logging == other.logging) & (self.api == other.api) + & (self.scratch == other.scratch) + & (self.oidc == other.oidc) + & (self.auth_token_path == other.auth_token_path) + & (self.numtracker == other.numtracker) & (self.opa == other.opa) ) return False From 0d507d12bfde6c48d2842e6fd18d7a2f7adac6b5 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 25 Jun 2026 15:52:33 +0100 Subject: [PATCH 10/13] chore: Add just task to launch repl with blueapi client (#1568) $ just repl will now launch a repl (using ptpython) with an initialized BlueapiClient named `bc` using the config from the system tests (the same used by `just serve` to run the server). --- justfile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/justfile b/justfile index 269564d712..2627d12e61 100644 --- a/justfile +++ b/justfile @@ -34,3 +34,11 @@ system *OPTS: coverage: uv run pytest tests/unit_tests --cov --cov-report html xdg-open htmlcov/index.html + +repl: + #!/usr/bin/env bash + uv run --with ptpython ptpython -i <(cat << EOF + from blueapi.client import BlueapiClient + bc = BlueapiClient.from_config_file("tests/system_tests/config.yaml").with_instrument_session("cm12345-1") + EOF + ) From 8d585a9cf04e71d390ecacb07f34b99b028fc870 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 25 Jun 2026 17:07:56 +0100 Subject: [PATCH 11/13] chore: copier update to `5.2.0` (#1563) --- .copier-answers.yml | 2 +- .github/CONTRIBUTING.md | 2 +- .github/workflows/ci.yml | 2 +- Dockerfile | 2 +- pyproject.toml | 1 + 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.copier-answers.yml b/.copier-answers.yml index bce0f1d10c..856234853d 100644 --- a/.copier-answers.yml +++ b/.copier-answers.yml @@ -1,5 +1,5 @@ # Changes here will be overwritten by Copier -_commit: 5.0.3 +_commit: 5.2.0 _src_path: https://github.com/DiamondLightSource/python-copier-template author_email: callum.forrester@diamond.ac.uk author_name: Callum Forrester diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 9429e2abcf..0c6b60c41a 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -24,4 +24,4 @@ It is recommended that developers use a [vscode devcontainer](https://code.visua This project was created using the [Diamond Light Source Copier Template](https://github.com/DiamondLightSource/python-copier-template) for Python projects. -For more information on common tasks like setting up a developer environment, running the tests, and setting a pre-commit hook, see the template's [How-to guides](https://diamondlightsource.github.io/python-copier-template/5.0.3/how-to.html). +For more information on common tasks like setting up a developer environment, running the tests, and setting a pre-commit hook, see the template's [How-to guides](https://diamondlightsource.github.io/python-copier-template/5.2.0/how-to.html). diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93bcfcf42d..05d03967b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: strategy: matrix: runs-on: ["ubuntu-latest"] # can add windows-latest, macos-latest - python-version: ["3.11", "3.12", "3.13"] + python-version: ["3.11", "3.12", "3.13", "3.14"] fail-fast: false uses: ./.github/workflows/_test.yml with: diff --git a/Dockerfile b/Dockerfile index b2486f1cbb..77df264989 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # The devcontainer should use the developer target and run as root with podman # or docker with user namespaces. -FROM ghcr.io/diamondlightsource/ubuntu-devcontainer:noble AS developer +FROM ghcr.io/diamondlightsource/ubuntu-devcontainer:resolute AS developer # Add any system dependencies for the developer/build environment here RUN apt-get update -y && apt-get install -y --no-install-recommends \ diff --git a/pyproject.toml b/pyproject.toml index bf29c45965..d737aa21c8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,6 +10,7 @@ classifiers = [ "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", ] description = "Lightweight bluesky-as-a-service wrapper application. Also usable as a library." dependencies = [ From 54b179b55df1fd43c88cf2047e49ad485fe63fa9 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Fri, 26 Jun 2026 11:10:23 +0100 Subject: [PATCH 12/13] feat: use issuer url instead of well known url (#1371) --- docs/reference/openapi.yaml | 14 +++++-- helm/blueapi/config_schema.json | 27 +++++++++++-- helm/blueapi/values.schema.json | 22 ++++++++++- src/blueapi/config.py | 47 ++++++++++++++++++----- tests/conftest.py | 22 +++++------ tests/system_tests/config.yaml | 2 +- tests/system_tests/test_blueapi_system.py | 4 +- tests/unit_tests/cli/test_cli.py | 4 +- tests/unit_tests/test_config.py | 31 +++++++++++++-- tests/unit_tests/test_helm_chart.py | 2 +- 10 files changed, 135 insertions(+), 40 deletions(-) diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index d14664f571..94a1f1540f 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -92,17 +92,25 @@ components: description: Client ID title: Client Id type: string + issuer: + anyOf: + - type: string + - type: 'null' + description: URL of OIDC provider + title: Issuer logout_redirect_endpoint: default: '' description: The oidc endpoint required to logout title: Logout Redirect Endpoint type: string well_known_url: + anyOf: + - type: string + - type: 'null' + deprecated: true description: URL to fetch OIDC config from the provider title: Well Known Url - type: string required: - - well_known_url - client_id title: OIDCConfig type: object @@ -433,7 +441,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html title: BlueAPI Control - version: 1.3.0 + version: 1.4.0 openapi: 3.1.0 paths: /api/v1/devices: diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index 070c1ad981..b2fb559609 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -242,9 +242,31 @@ "additionalProperties": false, "properties": { "well_known_url": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "deprecated": true, "description": "URL to fetch OIDC config from the provider", - "title": "Well Known Url", - "type": "string" + "title": "Well Known Url" + }, + "issuer": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "description": "URL of OIDC provider", + "title": "Issuer" }, "client_id": { "description": "Client ID", @@ -265,7 +287,6 @@ } }, "required": [ - "well_known_url", "client_id" ], "title": "OIDCConfig", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 71c8a53954..c0d99f9e5d 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -668,7 +668,6 @@ "title": "OIDCConfig", "type": "object", "required": [ - "well_known_url", "client_id" ], "properties": { @@ -683,6 +682,18 @@ "description": "Client ID", "type": "string" }, + "issuer": { + "title": "Issuer", + "description": "URL of OIDC provider", + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "logout_redirect_endpoint": { "title": "Logout Redirect Endpoint", "description": "The oidc endpoint required to logout", @@ -692,7 +703,14 @@ "well_known_url": { "title": "Well Known Url", "description": "URL to fetch OIDC config from the provider", - "type": "string" + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] } }, "additionalProperties": false diff --git a/src/blueapi/config.py b/src/blueapi/config.py index f58ae9b389..be765c9bf4 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -1,3 +1,4 @@ +import logging import os import re import textwrap @@ -6,7 +7,7 @@ from functools import cached_property from pathlib import Path from string import Template -from typing import Annotated, Any, ClassVar, Generic, Literal, TypeVar, cast +from typing import Annotated, Any, ClassVar, Generic, Literal, Self, TypeVar, cast import requests import yaml @@ -22,11 +23,14 @@ UrlConstraints, ValidationError, field_validator, + model_validator, ) from pydantic.json_schema import SkipJsonSchema from blueapi.utils import BlueapiBaseModel, InvalidConfigError +LOGGER = logging.getLogger(__name__) + LogLevel = Literal["NOTSET", "DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"] FORBIDDEN_OWN_REMOTE_URL = "https://github.com/DiamondLightSource/blueapi.git" @@ -221,18 +225,47 @@ class ScratchConfig(BlueapiBaseModel): class OIDCConfig(BlueapiBaseModel): - well_known_url: str = Field( - description="URL to fetch OIDC config from the provider" + well_known_url: str | None = Field( + description="URL to fetch OIDC config from the provider", + deprecated=True, + default=None, ) + issuer: str | None = Field(description="URL of OIDC provider", default=None) client_id: str = Field(description="Client ID") client_audience: str = Field(description="Client Audience(s)", default="blueapi") logout_redirect_endpoint: str = Field( description="The oidc endpoint required to logout", default="" ) + @model_validator(mode="after") + def check_urls(self) -> Self: + if self.issuer is None and self.well_known_url is None: + raise ValueError("Please provide 'OIDCConfig.issuer'") + if self.well_known_url: + LOGGER.warning( + DeprecationWarning( + "OIDCConfig.well_known_url is deprecated, " + "Please use OIDCConfig.issuer" + ), + ) + return self + + @cached_property + def _well_known_url(self) -> str: + if self.issuer: + if self.well_known_url: + LOGGER.warning( + DeprecationWarning( + "well_known_url and issuer are both set. " + "Defaulting to issuer URL" + ), + ) + return self.issuer + "/.well-known/openid-configuration" + return cast(str, self.well_known_url) + @cached_property def _config_from_oidc_url(self) -> dict[str, Any]: - response: requests.Response = requests.get(self.well_known_url) + response = requests.get(self._well_known_url) response.raise_for_status() return response.json() @@ -246,10 +279,6 @@ def device_authorization_endpoint(self) -> str: def token_endpoint(self) -> str: return cast(str, self._config_from_oidc_url.get("token_endpoint")) - @cached_property - def issuer(self) -> str: - return cast(str, self._config_from_oidc_url.get("issuer")) - @cached_property def authorization_endpoint(self) -> str: return cast(str, self._config_from_oidc_url.get("authorization_endpoint")) @@ -296,7 +325,7 @@ class ApplicationConfig(BlueapiBaseModel): """ #: API version to publish in OpenAPI schema - REST_API_VERSION: ClassVar[str] = "1.3.0" + REST_API_VERSION: ClassVar[str] = "1.4.0" LICENSE_INFO: ClassVar[dict[str, str]] = { "name": "Apache 2.0", diff --git a/tests/conftest.py b/tests/conftest.py index 0203a07259..78af275785 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -54,18 +54,12 @@ def exporter() -> JsonObjectSpanExporter: return exporter -@pytest.fixture -def oidc_url() -> str: - return ( - "https://auth.example.com/realms/master/oidc/.well-known/openid-configuration" - ) +ISSUER = "https://auth.example.com/realms/master" @pytest.fixture -def oidc_config(oidc_url: str) -> OIDCConfig: - return OIDCConfig( - well_known_url=oidc_url, client_id="blueapi-cli", client_audience="blueapi" - ) +def oidc_config() -> OIDCConfig: + return OIDCConfig(issuer=ISSUER, client_id="blueapi-cli", client_audience="blueapi") CACHE_FILE = "blueapi_cache" @@ -86,7 +80,7 @@ def oidc_well_known() -> dict[str, Any]: "device_authorization_endpoint": "https://example.com/device_authorization", "authorization_endpoint": "https://example.com/authorization", "token_endpoint": "https://example.com/token", - "issuer": "https://example.com", + "issuer": ISSUER, "jwks_uri": "https://example.com/realms/master/protocol/openid-connect/certs", "end_session_endpoint": "https://example.com/end_session", "id_token_signing_alg_values_supported": ["RS256"], @@ -110,6 +104,7 @@ def _make_token( rsa_private_key: str, jwt_access_token: bool = False, valid_audience: bool = True, + issuer: str = ISSUER, ) -> dict[str, str]: now = time.time() @@ -117,7 +112,7 @@ def _make_token( "aud": "blueapi" if valid_audience else "invalid_audience", "exp": now + expires_in, "iat": now + issued_in, - "iss": "https://example.com", + "iss": issuer, "sub": "jd1", "name": "Jane Doe", "fedid": "jd1", @@ -242,7 +237,6 @@ def device_code() -> str: @pytest.fixture def mock_authn_server( - oidc_url: str, oidc_well_known: dict[str, Any], oidc_config: OIDCConfig, valid_token: dict[str, Any], @@ -256,7 +250,9 @@ def mock_authn_server( json=oidc_config.model_dump(), ) # Fetch well-known OIDC flow URLs from server - requests_mock.get(oidc_url, json=oidc_well_known) + requests_mock.get( + ISSUER + "/.well-known/openid-configuration", json=oidc_well_known + ) # When device flow begins, return a device_code requests_mock.post( oidc_well_known["device_authorization_endpoint"], diff --git a/tests/system_tests/config.yaml b/tests/system_tests/config.yaml index 714933ccdf..9a35e517cb 100644 --- a/tests/system_tests/config.yaml +++ b/tests/system_tests/config.yaml @@ -22,6 +22,6 @@ tiled: client_id: "tiled-writer" client_secret: "secret" oidc: - well_known_url: "http://localhost:8081/realms/master/.well-known/openid-configuration" + issuer: "http://localhost:8081/realms/master" client_id: "ixx-cli-blueapi" client_audience: "ixx-blueapi" diff --git a/tests/system_tests/test_blueapi_system.py b/tests/system_tests/test_blueapi_system.py index bb1c41dfe1..c470f08448 100644 --- a/tests/system_tests/test_blueapi_system.py +++ b/tests/system_tests/test_blueapi_system.py @@ -225,8 +225,8 @@ def test_cannot_access_endpoints( def test_can_get_oidc_config_without_auth(client_without_auth: BlueapiClient): - assert client_without_auth.oidc_config == OIDCConfig( - well_known_url="http://localhost:8081/realms/master/.well-known/openid-configuration", + assert client_without_auth.get_oidc_config() == OIDCConfig( + issuer="http://localhost:8081/realms/master", client_id="ixx-cli-blueapi", client_audience="ixx-blueapi", ) diff --git a/tests/unit_tests/cli/test_cli.py b/tests/unit_tests/cli/test_cli.py index 5c2b418686..3e27cfc98e 100644 --- a/tests/unit_tests/cli/test_cli.py +++ b/tests/unit_tests/cli/test_cli.py @@ -1104,7 +1104,7 @@ def test_login_success( result = runner.invoke(main, ["-c", config_with_auth, "login"]) assert ( "Logging in\n" - "Please login from this URL:- https://example.com/verify\n" + "Please login from this URL:- https://auth.example.com/realms/master/verify\n" "Logged in and cached new token\n" == result.output ) assert result.exit_code == 0 @@ -1143,7 +1143,7 @@ def test_login_when_cached_token_decode_fails( result = runner.invoke(main, ["-c", config_with_auth, "login"]) assert ( "Logging in\n" - "Please login from this URL:- https://example.com/verify\n" + "Please login from this URL:- https://auth.example.com/realms/master/verify\n" "Logged in and cached new token\n" in result.output ) assert result.exit_code == 0 diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 8498e197f3..bd136d4779 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -1,5 +1,6 @@ import inspect import json +import logging import os import tempfile from collections.abc import Generator, Iterable, Mapping @@ -323,7 +324,8 @@ def test_config_yaml_parsed(temp_yaml_config_file): }, "numtracker": None, "oidc": { - "well_known_url": "https://auth.example.com/realms/sample/.well-known/openid-configuration", + "well_known_url": None, + "issuer": "https://auth.example.com/realms/sample", "client_id": "blueapi-client", "client_audience": "aud", "logout_redirect_endpoint": "", @@ -383,7 +385,8 @@ def test_config_yaml_parsed(temp_yaml_config_file): }, "numtracker": None, "oidc": { - "well_known_url": "https://auth.example.com/realms/sample/.well-known/openid-configuration", + "well_known_url": None, + "issuer": "https://auth.example.com/realms/sample", "client_id": "blueapi-client", "client_audience": "aud", "logout_redirect_endpoint": "", @@ -453,7 +456,7 @@ def test_config_yaml_parsed_complete(temp_yaml_config_file: dict): "api": {"host": "0.0.0.0", "port": 8001, "protocol": "http"}, "numtracker": None, "oidc": { - "well_known_url": "https://auth.example.com/realms/sample/.well-known/openid-configuration", + "issuer": "https://auth.example.com/realms/sample", "client_id": "blueapi-client", "client_audience": "aud", }, @@ -505,7 +508,6 @@ def test_oauth_config_model_post_init( oidc_config.authorization_endpoint == oidc_well_known["authorization_endpoint"] ) assert oidc_config.token_endpoint == oidc_well_known["token_endpoint"] - assert oidc_config.issuer == oidc_well_known["issuer"] assert oidc_config.jwks_uri == oidc_well_known["jwks_uri"] assert oidc_config.end_session_endpoint == oidc_well_known["end_session_endpoint"] @@ -546,3 +548,24 @@ def test_config_schema_updated() -> None: f"ApplicationConfig model is out of date with schema at \ {CONFIG_SCHEMA_LOCATION}. You may need to run `blueapi config-schema -u`" ) + + +def test_oidc_config_validation_error(): + with pytest.raises(ValueError, match="Please provide 'OIDCConfig.issuer'"): + ApplicationConfig( + oidc=OIDCConfig(well_known_url=None, issuer=None, client_id="blueapi") + ) + + +def test_oidc_config_support_well_known_url(): + oidc = OIDCConfig(well_known_url="url", issuer=None, client_id="blueapi") + assert oidc._well_known_url == "url" + + +def test_issuer_url_preferred_over_well_known_url(caplog): + oidc = OIDCConfig(well_known_url="url", issuer="issuer_url", client_id="blueapi") + with caplog.at_level(logging.WARN): + assert ( + oidc._well_known_url == "issuer_url" + "/.well-known/openid-configuration" + ) + assert "well_known_url and issuer are both set" in caplog.text diff --git a/tests/unit_tests/test_helm_chart.py b/tests/unit_tests/test_helm_chart.py index 4c1dcb24b3..0aa114a90a 100644 --- a/tests/unit_tests/test_helm_chart.py +++ b/tests/unit_tests/test_helm_chart.py @@ -69,7 +69,7 @@ ), logging=LoggingConfig(level="CRITICAL"), oidc=OIDCConfig( - well_known_url="foo.bar", + issuer="foo.bar", client_id="blueapi2", client_audience="blueapi++", ), From 7622ee061ded8812b3fafc0ef5139730ebdb1364 Mon Sep 17 00:00:00 2001 From: Abigail Emery Date: Fri, 26 Jun 2026 11:23:56 +0100 Subject: [PATCH 13/13] fix: Remove dodal from default application config (#1573) The default application still included dodal plans as a default. With the change to remove this dependency from the default application this would cause the server to error when trying to fetch plans. This replaces those defaults with an empty list. --- helm/blueapi/config_schema.json | 11 +---------- helm/blueapi/values.schema.json | 11 +---------- src/blueapi/config.py | 5 +---- tests/unit_tests/worker/test_task_worker.py | 16 +++++++++++++--- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index b2fb559609..2c43910a49 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -105,16 +105,7 @@ "description": "Config for the RunEngine environment", "properties": { "sources": { - "default": [ - { - "module": "dodal.plans", - "kind": "planFunctions" - }, - { - "module": "dodal.plan_stubs.wrapped", - "kind": "planFunctions" - } - ], + "default": [], "items": { "discriminator": { "mapping": { diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index c0d99f9e5d..20bdf376da 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -554,16 +554,7 @@ }, "sources": { "title": "Sources", - "default": [ - { - "kind": "planFunctions", - "module": "dodal.plans" - }, - { - "kind": "planFunctions", - "module": "dodal.plan_stubs.wrapped" - } - ], + "default": [], "type": "array", "items": { "oneOf": [ diff --git a/src/blueapi/config.py b/src/blueapi/config.py index be765c9bf4..29cf242a90 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -144,10 +144,7 @@ class EnvironmentConfig(BlueapiBaseModel): PlanSource | DeviceManagerSource, Field(discriminator="kind"), ] - ] = [ - PlanSource(module="dodal.plans"), - PlanSource(module="dodal.plan_stubs.wrapped"), - ] + ] = Field(default=[]) events: WorkerEventConfig = Field(default_factory=WorkerEventConfig) metadata: MetadataConfig | None = Field(default=None) diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index 41041c5018..f03ba7c5fe 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -20,7 +20,7 @@ ) from ophyd_async.core import AsyncStatus -from blueapi.config import DeviceManagerSource, EnvironmentConfig +from blueapi.config import DeviceManagerSource, EnvironmentConfig, PlanSource from blueapi.core import BlueskyContext, EventStream from blueapi.core.bluesky_types import DataEvent from blueapi.service.model import PlanModel @@ -109,7 +109,12 @@ def second_fake_device() -> FakeDevice: @pytest.fixture def context(fake_device: FakeDevice, second_fake_device: FakeDevice) -> BlueskyContext: ctx = BlueskyContext() - ctx_config = EnvironmentConfig() + ctx_config = EnvironmentConfig( + sources=[ + PlanSource(module="dodal.plans"), + PlanSource(module="dodal.plan_stubs.wrapped"), + ] + ) ctx_config.sources.append(DeviceManagerSource(module="devices")) ctx.register_plan(failing_plan) ctx.register_device(fake_device) @@ -121,7 +126,12 @@ def context(fake_device: FakeDevice, second_fake_device: FakeDevice) -> BlueskyC @pytest.fixture def context_without_devices() -> BlueskyContext: ctx = BlueskyContext() - ctx_config = EnvironmentConfig() + ctx_config = EnvironmentConfig( + sources=[ + PlanSource(module="dodal.plans"), + PlanSource(module="dodal.plan_stubs.wrapped"), + ] + ) ctx_config.sources.append(DeviceManagerSource(module="devices")) ctx.with_config(ctx_config) return ctx