From 96e44ed7ddf60662ca351dd1110b55a3ba4892ed Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 8 Jul 2026 13:14:53 +0100 Subject: [PATCH 01/21] Add background scans in i15-1 converter --- pyproject.toml | 1 + .../plugins/i15_1/i15_1_converter.py | 141 ++++++++++++++++++ .../plugins/i15_1/tiled_interaction.py | 26 ++++ .../plugins/i15_1_converter.py | 66 -------- .../plugins/test_i15_1_converter.py | 2 +- uv.lock | 2 + 6 files changed, 171 insertions(+), 67 deletions(-) create mode 100644 src/daq_queuing_service/plugins/i15_1/i15_1_converter.py create mode 100644 src/daq_queuing_service/plugins/i15_1/tiled_interaction.py delete mode 100644 src/daq_queuing_service/plugins/i15_1_converter.py diff --git a/pyproject.toml b/pyproject.toml index 57ace2c..e00e219 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ "blueapi>=1.14.0", "fastapi>=0.136.0", "pydantic>=2.13.2", + "tiled>=0.2.9", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py new file mode 100644 index 0000000..647dcf2 --- /dev/null +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -0,0 +1,141 @@ +from typing import Literal, get_args + +from blueapi.service.model import TaskRequest +from pydantic import BaseModel + +from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall +from daq_queuing_service.plugins.i15_1.tiled_interaction import ( + get_background_tiled_id, +) +from daq_queuing_service.task import Experiment, TaskWithPosition + +BACKGROUND = Literal["air", "capillary_1", "capillary_2"] +SCAN_PLANS = Literal["centre_sample", "static_collection"] + + +class BackgroundInfo(BaseModel): + bg_type: BACKGROUND + cobra: bool + blower: bool + + +def get_required_background(experiment: Experiment) -> BackgroundInfo: + # TODO: Check if this should take a task request instead of experiment + return BackgroundInfo(bg_type="air", cobra=False, blower=False) + + +def construct_background_task_request( + background: BackgroundInfo, instrument_session: str +) -> TaskRequest: + return TaskRequest( + instrument_session=instrument_session, + name="static_collection", + params={"metadata": {"background": background}}, + ) + + +def add_required_background_scans( + tasks: list[TaskWithPosition], calls: list[BlueapiCall] +) -> list[BlueapiCall]: + bg_task_requests: dict[str, TaskRequest] = {} + + for task in tasks: + instrument_session = task.experiment.instrument_session + if not isinstance(task.experiment, Experiment) or not ( + background := get_required_background(task.experiment) + ): + break + + blueapi_calls = [call for call in calls if call.parent_task_id == task.id] + if tiled_id := get_background_tiled_id(background, instrument_session): + for call in filter( + lambda call: call.task_request.name in get_args(SCAN_PLANS), + blueapi_calls, + ): + if metadata := call.task_request.params.get("metadata"): + metadata.update({"background_tiled_id": tiled_id}) + else: + call.task_request.params = { + **call.task_request.params, + "metadata": {"background_tiled_id": tiled_id}, + } + + else: + task_request = construct_background_task_request( + background, instrument_session + ) + if task_request not in bg_task_requests.values(): + bg_task_requests[task.id] = task_request + + new_call_list: list[BlueapiCall] = [] + + for call in calls: + if call.parent_task_id and ( + task_request := bg_task_requests.get(call.parent_task_id) + ): + new_call_list.append(BlueapiCall(task_request=task_request)) + new_call_list.append(call) + + return new_call_list + + +def construct_blueapi_tasks_from_i15_1_experiment( + experiment: Experiment, +) -> list[TaskRequest]: + sample_name = experiment.sample.name + # Assume sample name is of form test_8_1 to load from position 8 on puck 1 + _, position, puck = sample_name.split("_") + + return [ + TaskRequest( + name="robot_load", + params={"puck": puck, "position": position}, + instrument_session=experiment.instrument_session, + ), + TaskRequest( + name="centre_sample", + params={ + "start_z": -20, + "end_z": 0, + "steps": 20, + "exposure_time": 0.01, + "metadata": { + "sample": experiment.sample, + "experiment_definition": experiment.experiment_definition, + }, + }, + instrument_session=experiment.instrument_session, + ), + TaskRequest( + name="robot_unload", + params={}, + instrument_session=experiment.instrument_session, + ), + ] + + +def construct_i15_1_blueapi_call_list( + queue: list[TaskWithPosition], + history: list[TaskWithPosition], + call_history: list[BlueapiCall], +) -> list[BlueapiCall]: + + call_list: list[BlueapiCall] = [] + + for task in queue: + match task.experiment: + case TaskRequest(): + call_list.append( + BlueapiCall(task_request=task.experiment, parent_task_id=task.id) + ) + case Experiment(): + call_list.extend( + [ + BlueapiCall(task_request=b_api_task, parent_task_id=task.id) + for b_api_task in construct_blueapi_tasks_from_i15_1_experiment( + task.experiment + ) + ] + ) + + return add_required_background_scans(queue, call_list) diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py new file mode 100644 index 0000000..14b7b7a --- /dev/null +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -0,0 +1,26 @@ +from tiled.client import from_uri +from tiled.client.container import Container +from tiled.queries import Eq + +from daq_queuing_service.plugins.i15_1.i15_1_converter import BackgroundInfo + + +def get_background_tiled_id(required: BackgroundInfo) -> str | None: + client = from_uri("https://tiled.diamond.ac.uk/api/v1") + + result: Container = ( + client.search(Eq("start.instrument_session", "cm44163-3")) + .search(Eq("start.instrument", "i15-1")) + .search(Eq("start.background", required)) + ) + assert isinstance(result, Container) + + if not len(result): + return + + items = [(key, value) for key, value in result.items()].sort( + key=lambda item: item[1].metadata["start"]["time"] + ) + + # return the tiled ID + return items[-1][0] diff --git a/src/daq_queuing_service/plugins/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1_converter.py deleted file mode 100644 index 7779bd8..0000000 --- a/src/daq_queuing_service/plugins/i15_1_converter.py +++ /dev/null @@ -1,66 +0,0 @@ -from blueapi.service.model import TaskRequest - -from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall -from daq_queuing_service.task import Experiment, TaskWithPosition - - -def construct_blueapi_tasks_from_i15_1_experiment( - experiment: Experiment, -) -> list[TaskRequest]: - sample_name = experiment.sample.name - # Assume sample name is of form test_8_1 to load from position 8 on puck 1 - _, position, puck = sample_name.split("_") - - return [ - TaskRequest( - name="robot_load", - params={"puck": puck, "position": position}, - instrument_session=experiment.instrument_session, - ), - TaskRequest( - name="centre_sample", - params={ - "start_z": -20, - "end_z": 0, - "steps": 20, - "exposure_time": 0.01, - "metadata": { - "sample": experiment.sample, - "experiment_definition": experiment.experiment_definition, - }, - }, - instrument_session=experiment.instrument_session, - ), - TaskRequest( - name="robot_unload", - params={}, - instrument_session=experiment.instrument_session, - ), - ] - - -def construct_i15_1_blueapi_call_list( - queue: list[TaskWithPosition], - history: list[TaskWithPosition], - call_history: list[BlueapiCall], -) -> list[BlueapiCall]: - - call_list: list[BlueapiCall] = [] - - for task in queue: - match task.experiment: - case TaskRequest(): - call_list.append( - BlueapiCall(task_request=task.experiment, parent_task_id=task.id) - ) - case Experiment(): - call_list.extend( - [ - BlueapiCall(task_request=b_api_task, parent_task_id=task.id) - for b_api_task in construct_blueapi_tasks_from_i15_1_experiment( - task.experiment - ) - ] - ) - - return call_list diff --git a/tests/unit_tests/plugins/test_i15_1_converter.py b/tests/unit_tests/plugins/test_i15_1_converter.py index 4bc9efc..4f6beb6 100644 --- a/tests/unit_tests/plugins/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/test_i15_1_converter.py @@ -2,7 +2,7 @@ from blueapi.service.model import TaskRequest -from daq_queuing_service.plugins.i15_1_converter import ( +from daq_queuing_service.plugins.i15_1.i15_1_converter import ( construct_blueapi_tasks_from_i15_1_experiment, construct_i15_1_blueapi_call_list, ) diff --git a/uv.lock b/uv.lock index 2dff840..65aedba 100644 --- a/uv.lock +++ b/uv.lock @@ -1005,6 +1005,7 @@ dependencies = [ { name = "blueapi" }, { name = "fastapi" }, { name = "pydantic" }, + { name = "tiled" }, ] [package.dev-dependencies] @@ -1033,6 +1034,7 @@ requires-dist = [ { name = "blueapi", specifier = ">=1.14.0" }, { name = "fastapi", specifier = ">=0.136.0" }, { name = "pydantic", specifier = ">=2.13.2" }, + { name = "tiled", specifier = ">=0.2.9" }, ] [package.metadata.requires-dev] From d8c44601eb4b841f4d557b459e89ef98e6651dfe Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Thu, 9 Jul 2026 11:56:54 +0100 Subject: [PATCH 02/21] Allow multiple background scans per experiment --- .../plugins/i15_1/i15_1_converter.py | 73 +++++++++++++------ 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 647dcf2..ebaccd1 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -1,4 +1,5 @@ -from typing import Literal, get_args +from collections.abc import Mapping +from typing import Any, Literal, get_args from blueapi.service.model import TaskRequest from pydantic import BaseModel @@ -18,10 +19,37 @@ class BackgroundInfo(BaseModel): cobra: bool blower: bool + def add_tiled_id(self, tiled_id: str) -> "TiledBackground": + return TiledBackground( + bg_type=self.bg_type, + cobra=self.cobra, + blower=self.blower, + tiled_id=tiled_id, + ) -def get_required_background(experiment: Experiment) -> BackgroundInfo: + +class TiledBackground(BackgroundInfo): + tiled_id: str + + +def add_tiled_background_to_md( + params: Mapping[str, Any], tiled_id: str, background: BackgroundInfo +): + if metadata := params.get("metadata"): + if tiled_backgrounds := params.get("tiled_backgrounds"): + tiled_backgrounds.update({tiled_id: background}) + else: + metadata.update({"tiled_backgrounds": {tiled_id: background}}) + else: + params = { + **params, + "metadata": {"background_tiled_id": tiled_id}, + } + + +def get_required_backgrounds(experiment: Experiment) -> list[BackgroundInfo]: # TODO: Check if this should take a task request instead of experiment - return BackgroundInfo(bg_type="air", cobra=False, blower=False) + return [BackgroundInfo(bg_type="air", cobra=False, blower=False)] def construct_background_task_request( @@ -42,30 +70,27 @@ def add_required_background_scans( for task in tasks: instrument_session = task.experiment.instrument_session if not isinstance(task.experiment, Experiment) or not ( - background := get_required_background(task.experiment) + backgrounds := get_required_backgrounds(task.experiment) ): break - blueapi_calls = [call for call in calls if call.parent_task_id == task.id] - if tiled_id := get_background_tiled_id(background, instrument_session): - for call in filter( - lambda call: call.task_request.name in get_args(SCAN_PLANS), - blueapi_calls, - ): - if metadata := call.task_request.params.get("metadata"): - metadata.update({"background_tiled_id": tiled_id}) - else: - call.task_request.params = { - **call.task_request.params, - "metadata": {"background_tiled_id": tiled_id}, - } - - else: - task_request = construct_background_task_request( - background, instrument_session - ) - if task_request not in bg_task_requests.values(): - bg_task_requests[task.id] = task_request + for background in backgrounds: + blueapi_calls = [call for call in calls if call.parent_task_id == task.id] + if tiled_id := get_background_tiled_id(background, instrument_session): + for call in filter( + lambda call: call.task_request.name in get_args(SCAN_PLANS), + blueapi_calls, + ): + add_tiled_background_to_md( + call.task_request.params, tiled_id, background + ) + + else: + task_request = construct_background_task_request( + background, instrument_session + ) + if task_request not in bg_task_requests.values(): + bg_task_requests[task.id] = task_request new_call_list: list[BlueapiCall] = [] From e9ee772e172f35b607cdb6a596f6fc9c1875d263 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 10 Jul 2026 09:06:41 +0100 Subject: [PATCH 03/21] Fixes --- .../plugins/i15_1/i15_1_converter.py | 54 +++++++------------ .../plugins/i15_1/tiled_interaction.py | 6 ++- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index ebaccd1..1265563 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -1,50 +1,27 @@ -from collections.abc import Mapping from typing import Any, Literal, get_args from blueapi.service.model import TaskRequest -from pydantic import BaseModel from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall +from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo from daq_queuing_service.plugins.i15_1.tiled_interaction import ( get_background_tiled_id, ) from daq_queuing_service.task import Experiment, TaskWithPosition -BACKGROUND = Literal["air", "capillary_1", "capillary_2"] SCAN_PLANS = Literal["centre_sample", "static_collection"] -class BackgroundInfo(BaseModel): - bg_type: BACKGROUND - cobra: bool - blower: bool - - def add_tiled_id(self, tiled_id: str) -> "TiledBackground": - return TiledBackground( - bg_type=self.bg_type, - cobra=self.cobra, - blower=self.blower, - tiled_id=tiled_id, - ) - - -class TiledBackground(BackgroundInfo): - tiled_id: str - - def add_tiled_background_to_md( - params: Mapping[str, Any], tiled_id: str, background: BackgroundInfo + params: dict[str, Any], tiled_id: str, background: BackgroundInfo ): if metadata := params.get("metadata"): - if tiled_backgrounds := params.get("tiled_backgrounds"): - tiled_backgrounds.update({tiled_id: background}) + if tiled_backgrounds := metadata.get("tiled_backgrounds"): + tiled_backgrounds[tiled_id] = background else: - metadata.update({"tiled_backgrounds": {tiled_id: background}}) + metadata["tiled_backgrounds"] = {tiled_id: background} else: - params = { - **params, - "metadata": {"background_tiled_id": tiled_id}, - } + params["metadata"] = {"tiled_backgrounds": {tiled_id: background}} def get_required_backgrounds(experiment: Experiment) -> list[BackgroundInfo]: @@ -65,7 +42,7 @@ def construct_background_task_request( def add_required_background_scans( tasks: list[TaskWithPosition], calls: list[BlueapiCall] ) -> list[BlueapiCall]: - bg_task_requests: dict[str, TaskRequest] = {} + bg_task_requests: dict[str, list[TaskRequest]] = {} for task in tasks: instrument_session = task.experiment.instrument_session @@ -81,6 +58,7 @@ def add_required_background_scans( lambda call: call.task_request.name in get_args(SCAN_PLANS), blueapi_calls, ): + call.task_request.params = dict(call.task_request.params) add_tiled_background_to_md( call.task_request.params, tiled_id, background ) @@ -89,16 +67,24 @@ def add_required_background_scans( task_request = construct_background_task_request( background, instrument_session ) - if task_request not in bg_task_requests.values(): - bg_task_requests[task.id] = task_request + if not any( + task_request in task_requests + for task_requests in bg_task_requests.values() + ): + bg_task_requests.setdefault(task.id, []).append(task_request) new_call_list: list[BlueapiCall] = [] for call in calls: if call.parent_task_id and ( - task_request := bg_task_requests.get(call.parent_task_id) + task_requests := bg_task_requests.pop(call.parent_task_id, None) ): - new_call_list.append(BlueapiCall(task_request=task_request)) + new_call_list.extend( + [ + BlueapiCall(task_request=task_request) + for task_request in task_requests + ] + ) new_call_list.append(call) return new_call_list diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 14b7b7a..1ea5c2b 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -5,11 +5,13 @@ from daq_queuing_service.plugins.i15_1.i15_1_converter import BackgroundInfo -def get_background_tiled_id(required: BackgroundInfo) -> str | None: +def get_background_tiled_id( + required: BackgroundInfo, instrument_session: str +) -> str | None: client = from_uri("https://tiled.diamond.ac.uk/api/v1") result: Container = ( - client.search(Eq("start.instrument_session", "cm44163-3")) + client.search(Eq("start.instrument_session", instrument_session)) .search(Eq("start.instrument", "i15-1")) .search(Eq("start.background", required)) ) From a292a2ed81f82e4d7797efce16da383060445a6c Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 10 Jul 2026 12:02:53 +0100 Subject: [PATCH 04/21] Add tests --- .../plugins/i15_1/tiled_interaction.py | 4 +- .../plugins/test_i15_1_converter.py | 187 +++++++++++++++++- 2 files changed, 188 insertions(+), 3 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 1ea5c2b..0b84098 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -2,7 +2,7 @@ from tiled.client.container import Container from tiled.queries import Eq -from daq_queuing_service.plugins.i15_1.i15_1_converter import BackgroundInfo +from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo def get_background_tiled_id( @@ -13,7 +13,7 @@ def get_background_tiled_id( result: Container = ( client.search(Eq("start.instrument_session", instrument_session)) .search(Eq("start.instrument", "i15-1")) - .search(Eq("start.background", required)) + .search(Eq("start.background", required.model_dump_json())) ) assert isinstance(result, Container) diff --git a/tests/unit_tests/plugins/test_i15_1_converter.py b/tests/unit_tests/plugins/test_i15_1_converter.py index 4f6beb6..a6394af 100644 --- a/tests/unit_tests/plugins/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/test_i15_1_converter.py @@ -1,8 +1,14 @@ from copy import deepcopy +from unittest.mock import MagicMock, patch +import pytest from blueapi.service.model import TaskRequest +from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall +from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo from daq_queuing_service.plugins.i15_1.i15_1_converter import ( + add_required_background_scans, + construct_background_task_request, construct_blueapi_tasks_from_i15_1_experiment, construct_i15_1_blueapi_call_list, ) @@ -11,11 +17,52 @@ ExperimentDefinition, Sample, Status, + Task, TaskKind, TaskWithPosition, ) +@pytest.fixture(autouse=True) +def background_found_in_tiled(): + with patch( + "daq_queuing_service.plugins.i15_1.i15_1_converter.get_background_tiled_id", + MagicMock(return_value="fake_tiled_id"), + ) as mock_get_background_tiled_id: + yield mock_get_background_tiled_id + + +@pytest.fixture() +def background_not_found_in_tiled(): + with patch( + "daq_queuing_service.plugins.i15_1.i15_1_converter.get_background_tiled_id", + MagicMock(return_value=None), + ) as mock_get_background_tiled_id: + yield mock_get_background_tiled_id + + +@pytest.fixture() +def tasks_and_calls( + tasks: list[Task], +) -> tuple[list[TaskWithPosition], list[BlueapiCall]]: + tasks_with_positions = [TaskWithPosition.from_task(task) for task in tasks] + calls: list[BlueapiCall] = [] + for task in tasks_with_positions: + assert isinstance(task.experiment, Experiment) + calls.extend( + [ + BlueapiCall( + task_request=task_request, + parent_task_id=task.id, + ) + for task_request in construct_blueapi_tasks_from_i15_1_experiment( + task.experiment + ) + ] + ) + return tasks_with_positions, calls + + def test_given_sample_name_in_correct_format_then_correct_sample_loaded(): experiment = Experiment( name="test_experiment", @@ -102,7 +149,8 @@ def test_mix_of_experiments_with_correct_experiment_type_are_converted(): kind=TaskKind.EXPERIMENT, ) - class BadExperiment: ... + class BadExperiment: + instrument_session = "cm12345-1" bad_task = deepcopy(good_task) bad_task.experiment = BadExperiment() # type: ignore @@ -112,3 +160,140 @@ class BadExperiment: ... tasks = [good_task, bad_task, plan_task, good_task] call_list = construct_i15_1_blueapi_call_list(tasks, [], []) assert len(call_list) == 7 + + +def test_if_no_background_found_in_tiled_then_background_scan_added_to_call_list( + background_not_found_in_tiled: None, +): + experiment = Experiment( + name="test_experiment", + experiment_definition=ExperimentDefinition( + name="run_full_collection", id="", data={} + ), + sample=Sample(name="test_8_1", id="", data={}), + instrument_session="cm12345-1", + ) + task = TaskWithPosition( + experiment=experiment, + id="1", + status=Status.QUEUED, + blueapi_calls=[], + position=None, + kind=TaskKind.EXPERIMENT, + ) + tasks = construct_i15_1_blueapi_call_list([task], [], []) + assert len(tasks) == 4 + assert tasks[0] == BlueapiCall( + task_request=TaskRequest( + name="static_collection", + params={ + "metadata": { + "background": BackgroundInfo( + bg_type="air", cobra=False, blower=False + ) + } + }, + instrument_session="cm12345-1", + ) + ) + + +def test_add_required_background_scans_does_not_add_the_same_background_twice( + tasks_and_calls: tuple[list[TaskWithPosition], list[BlueapiCall]], + background_not_found_in_tiled: None, +): + tasks, calls = tasks_and_calls + bg_1 = BackgroundInfo(bg_type="air", cobra=False, blower=False) + bg_2 = BackgroundInfo(bg_type="capillary_1", cobra=True, blower=False) + bg_3 = BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True) + + def fake_get_required_background(experiment: Experiment): + # Get the same background scans every other experiment + # Only one of each background should be added + if int(experiment.sample.id) % 2 == 0: + return [bg_1, bg_2] + else: + return [bg_3] + + assert len(calls) == 15 + with patch( + "daq_queuing_service.plugins.i15_1.i15_1_converter.get_required_backgrounds", + fake_get_required_background, + ): + new_calls = add_required_background_scans(tasks, calls) + + assert len(new_calls) == 18 + + assert new_calls[0] == BlueapiCall( + task_request=construct_background_task_request(bg_1, instrument_session="") + ) + assert new_calls[1] == BlueapiCall( + task_request=construct_background_task_request(bg_2, instrument_session=""), + ) + # This one placed before the task that requires it + assert new_calls[5] == BlueapiCall( + task_request=construct_background_task_request(bg_3, instrument_session=""), + ) + + +def test_same_experiment_in_different_instrument_sessions_will_add_background_in_each( + tasks_and_calls: tuple[list[TaskWithPosition], list[BlueapiCall]], + background_not_found_in_tiled: None, +): + tasks, _ = tasks_and_calls + tasks[1].experiment.instrument_session = "different" + calls: list[BlueapiCall] = [] + for task in tasks: + assert isinstance(task.experiment, Experiment) + calls.extend( + [ + BlueapiCall( + task_request=task_request, + parent_task_id=task.id, + ) + for task_request in construct_blueapi_tasks_from_i15_1_experiment( + task.experiment + ) + ] + ) + + assert len(calls) == 15 + + new_calls = add_required_background_scans(tasks, calls) + + assert len(new_calls) == 17 + assert new_calls[0] == BlueapiCall( + task_request=TaskRequest( + name="static_collection", + params={ + "metadata": { + "background": BackgroundInfo( + bg_type="air", cobra=False, blower=False + ) + } + }, + instrument_session="", + ) + ) + assert new_calls[4] == BlueapiCall( + task_request=TaskRequest( + name="static_collection", + params={ + "metadata": { + "background": BackgroundInfo( + bg_type="air", cobra=False, blower=False + ) + } + }, + instrument_session="different", + ) + ) + + +def test_add_required_background_scans_if_found_in_tiled_then_no_background_added( + tasks_and_calls: tuple[list[TaskWithPosition], list[BlueapiCall]], + background_found_in_tiled: None, +): + tasks, calls_before = tasks_and_calls + calls_after = add_required_background_scans(tasks, calls_before) + assert calls_after == calls_before From b400a3197fd7e2a8f80597bc4b896e06c00a7bfa Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 10 Jul 2026 12:59:53 +0100 Subject: [PATCH 05/21] Add missing file --- .../plugins/i15_1/backgrounds.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/daq_queuing_service/plugins/i15_1/backgrounds.py diff --git a/src/daq_queuing_service/plugins/i15_1/backgrounds.py b/src/daq_queuing_service/plugins/i15_1/backgrounds.py new file mode 100644 index 0000000..4eb6813 --- /dev/null +++ b/src/daq_queuing_service/plugins/i15_1/backgrounds.py @@ -0,0 +1,23 @@ +from typing import Literal + +from pydantic import BaseModel + +BACKGROUND = Literal["air", "capillary_1", "capillary_2"] + + +class BackgroundInfo(BaseModel): + bg_type: BACKGROUND + cobra: bool + blower: bool + + def add_tiled_id(self, tiled_id: str) -> "TiledBackground": + return TiledBackground( + bg_type=self.bg_type, + cobra=self.cobra, + blower=self.blower, + tiled_id=tiled_id, + ) + + +class TiledBackground(BackgroundInfo): + tiled_id: str From c9c946ca7a2e67875d8a8aefd33980ef12fbf5fe Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Mon, 13 Jul 2026 16:08:29 +0100 Subject: [PATCH 06/21] Tests and fixes --- .../plugins/i15_1/tiled_interaction.py | 10 +-- .../plugins/test_get_background_tiled_id.py | 88 +++++++++++++++++++ .../plugins/test_i15_1_converter.py | 69 +++++++++++++++ 3 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 tests/unit_tests/plugins/test_get_background_tiled_id.py diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 0b84098..e995a4e 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -6,22 +6,22 @@ def get_background_tiled_id( - required: BackgroundInfo, instrument_session: str + required_background: BackgroundInfo, instrument_session: str ) -> str | None: client = from_uri("https://tiled.diamond.ac.uk/api/v1") result: Container = ( client.search(Eq("start.instrument_session", instrument_session)) .search(Eq("start.instrument", "i15-1")) - .search(Eq("start.background", required.model_dump_json())) + .search(Eq("start.background", required_background.model_dump_json())) ) - assert isinstance(result, Container) if not len(result): return - items = [(key, value) for key, value in result.items()].sort( - key=lambda item: item[1].metadata["start"]["time"] + items = sorted( + ((key, value) for key, value in result.items()), + key=lambda item: item[1].metadata["start"]["time"], ) # return the tiled ID diff --git a/tests/unit_tests/plugins/test_get_background_tiled_id.py b/tests/unit_tests/plugins/test_get_background_tiled_id.py new file mode 100644 index 0000000..898954f --- /dev/null +++ b/tests/unit_tests/plugins/test_get_background_tiled_id.py @@ -0,0 +1,88 @@ +from collections.abc import Generator +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest +from tiled.queries import Eq + +from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo +from daq_queuing_service.plugins.i15_1.tiled_interaction import get_background_tiled_id + + +@pytest.fixture() +def mock_tiled_searches() -> Generator[ + tuple[MagicMock, MagicMock, MagicMock], Any, None +]: + result_1 = MagicMock() + result_1.metadata = {"start": {"time": 1}} + result_2 = MagicMock() + result_2.metadata = {"start": {"time": 10}} + result_3 = MagicMock() + result_3.metadata = {"start": {"time": 2}} + + search_result_3 = MagicMock() + search_result_3.search = MagicMock( + return_value={ + "tiled_id_1": result_1, + "tiled_id_2": result_2, + "tiled_id_3": result_3, + } + ) + + search_result_2 = MagicMock() + search_result_2.search = MagicMock(return_value=search_result_3) + + client = MagicMock() + client.search = MagicMock(return_value=search_result_2) + + with patch( + "daq_queuing_service.plugins.i15_1.tiled_interaction.from_uri", + MagicMock(return_value=client), + ): + yield client, search_result_2, search_result_3 + + +def test_get_background_tiled_id_makes_expected_searches( + mock_tiled_searches: tuple[MagicMock, MagicMock, MagicMock], +): + client, search_2, search_3 = mock_tiled_searches + get_background_tiled_id( + BackgroundInfo(bg_type="air", cobra=False, blower=False), + instrument_session="cm12345-1", + ) + client.search.assert_called_once_with( + Eq(key="start.instrument_session", value="cm12345-1") + ) + search_2.search.assert_called_once_with(Eq(key="start.instrument", value="i15-1")) + search_3.search.assert_called_once_with( + Eq( + key="start.background", + value='{"bg_type":"air","cobra":false,"blower":false}', + ) + ) + + +def test_get_background_tiled_returns_most_recent_valid_background( + mock_tiled_searches: tuple[MagicMock, MagicMock, MagicMock], +): + assert ( + get_background_tiled_id( + BackgroundInfo(bg_type="air", cobra=False, blower=False), + instrument_session="cm12345-1", + ) + == "tiled_id_2" + ) + + +def test_get_background_tiled_id_returns_none_if_no_matching_backgrounds_found( + mock_tiled_searches: tuple[MagicMock, MagicMock, MagicMock], +): + _, _, final_search = mock_tiled_searches + final_search.search.return_value = {} + assert ( + get_background_tiled_id( + BackgroundInfo(bg_type="air", cobra=False, blower=False), + instrument_session="cm12345-1", + ) + is None + ) diff --git a/tests/unit_tests/plugins/test_i15_1_converter.py b/tests/unit_tests/plugins/test_i15_1_converter.py index a6394af..08d142c 100644 --- a/tests/unit_tests/plugins/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/test_i15_1_converter.py @@ -1,4 +1,5 @@ from copy import deepcopy +from typing import Any from unittest.mock import MagicMock, patch import pytest @@ -8,6 +9,7 @@ from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo from daq_queuing_service.plugins.i15_1.i15_1_converter import ( add_required_background_scans, + add_tiled_background_to_md, construct_background_task_request, construct_blueapi_tasks_from_i15_1_experiment, construct_i15_1_blueapi_call_list, @@ -297,3 +299,70 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde tasks, calls_before = tasks_and_calls calls_after = add_required_background_scans(tasks, calls_before) assert calls_after == calls_before + + +@pytest.mark.parametrize( + "params, tiled_ids, backgrounds, expected_params", + [ + ( + {"sample": "my_sample"}, + ["tiled_id"], + [BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True)], + { + "metadata": { + "tiled_backgrounds": { + "tiled_id": BackgroundInfo( + bg_type="capillary_1", cobra=False, blower=True + ) + } + }, + "sample": "my_sample", + }, + ), + ( + {}, + ["tiled_id"], + [BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True)], + { + "metadata": { + "tiled_backgrounds": { + "tiled_id": BackgroundInfo( + bg_type="capillary_1", cobra=False, blower=True + ) + } + }, + }, + ), + ( + {"sample": "my_sample"}, + ["tiled_id_1", "tiled_id_2"], + [ + BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True), + BackgroundInfo(bg_type="air", cobra=True, blower=False), + ], + { + "metadata": { + "tiled_backgrounds": { + "tiled_id_1": BackgroundInfo( + bg_type="capillary_1", cobra=False, blower=True + ), + "tiled_id_2": BackgroundInfo( + bg_type="air", cobra=True, blower=False + ), + } + }, + "sample": "my_sample", + }, + ), + ], +) +def test_add_tiled_background_to_md_adds_expected_metadata( + params: dict[str, Any], + tiled_ids: list[str], + backgrounds: list[BackgroundInfo], + expected_params: dict[str, Any], +): + for tiled_id, background in zip(tiled_ids, backgrounds, strict=True): + add_tiled_background_to_md(params, tiled_id, background) + + assert params == expected_params From 2b5a7c8732bf32aaead232205e447925bd2d6a4e Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 14 Jul 2026 10:05:13 +0100 Subject: [PATCH 07/21] Fix config --- tests/test_data/i15_1/test_daq_queue_config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_data/i15_1/test_daq_queue_config.yaml b/tests/test_data/i15_1/test_daq_queue_config.yaml index c022ab4..d677283 100644 --- a/tests/test_data/i15_1/test_daq_queue_config.yaml +++ b/tests/test_data/i15_1/test_daq_queue_config.yaml @@ -1,5 +1,5 @@ converter: - path: "daq_queuing_service.plugins.i15_1_converter" + path: "daq_queuing_service.plugins.i15_1.i15_1_converter" name: "construct_i15_1_blueapi_call_list" blueapi: api: From 38e44e3da99cc2d635e8efa734576ff47d78c5b2 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 4 Aug 2026 17:10:07 +0100 Subject: [PATCH 08/21] Add background scans as experiments instead of single plans --- .../plugins/i15_1/i15_1_converter.py | 191 +++++++++--------- src/daq_queuing_service/task.py | 5 +- .../plugins/test_i15_1_converter.py | 190 ++++++++--------- 3 files changed, 190 insertions(+), 196 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 97c455c..164c124 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -1,4 +1,4 @@ -from typing import Any, Literal, get_args +from typing import Any, Literal from blueapi.service.model import TaskRequest @@ -8,90 +8,55 @@ from daq_queuing_service.plugins.i15_1.tiled_interaction import ( get_background_tiled_id, ) -from daq_queuing_service.task import Experiment, TaskWithPosition +from daq_queuing_service.task import ( + Experiment, + ExperimentDefinition, + Sample, + Task, + TaskWithPosition, +) SCAN_PLANS = Literal["centre_sample", "static_collection"] class I151Converter(Converter): - def _add_tiled_background_to_md( - self, params: dict[str, Any], tiled_id: str, background: BackgroundInfo - ): - if metadata := params.get("metadata"): - if tiled_backgrounds := metadata.get("tiled_backgrounds"): - tiled_backgrounds[tiled_id] = background - else: - metadata["tiled_backgrounds"] = {tiled_id: background} - else: - params["metadata"] = {"tiled_backgrounds": {tiled_id: background}} - - def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: - # TODO: Check if this should take a task request instead of experiment - return [BackgroundInfo(bg_type="air", cobra=False, blower=False)] - - def _construct_background_task_request( - self, background: BackgroundInfo, instrument_session: str - ) -> TaskRequest: - return TaskRequest( - instrument_session=instrument_session, - name="static_collection", - params={"metadata": {"background": background}}, - ) + def pre_process( + self, + queue: list[Task], + history: list[TaskWithPosition], + call_history: list[BlueapiCall], + ) -> list[Task]: + return self._add_required_background_scans(queue) - def _add_required_background_scans( - self, tasks: list[TaskWithPosition], calls: list[BlueapiCall] + def construct_blueapi_calls( + self, + queue: list[TaskWithPosition], + history: list[TaskWithPosition], + call_history: list[BlueapiCall], ) -> list[BlueapiCall]: - bg_task_requests: dict[str, list[TaskRequest]] = {} - for task in tasks: - instrument_session = task.experiment.instrument_session - if not isinstance(task.experiment, Experiment) or not ( - backgrounds := self._get_required_backgrounds(task.experiment) - ): - break - - for background in backgrounds: - blueapi_calls = [ - call for call in calls if call.parent_task_id == task.id - ] - if tiled_id := get_background_tiled_id(background, instrument_session): - for call in filter( - lambda call: call.task_request.name in get_args(SCAN_PLANS), - blueapi_calls, - ): - call.task_request.params = dict(call.task_request.params) - self._add_tiled_background_to_md( - call.task_request.params, tiled_id, background - ) - - else: - task_request = self._construct_background_task_request( - background, instrument_session - ) - if not any( - task_request in task_requests - for task_requests in bg_task_requests.values() - ): - bg_task_requests.setdefault(task.id, []).append(task_request) - - new_call_list: list[BlueapiCall] = [] + call_list: list[BlueapiCall] = [] - for call in calls: - if call.parent_task_id and ( - task_requests := bg_task_requests.pop(call.parent_task_id, None) - ): - new_call_list.extend( - [ + for task in queue: + match task.experiment: + case TaskRequest(): + call_list.append( BlueapiCall( - task_request=task_request, - parent_task_id=call.parent_task_id, + task_request=task.experiment, parent_task_id=task.id ) - for task_request in task_requests - ] - ) - new_call_list.append(call) - - return new_call_list + ) + case Experiment(): + call_list.extend( + [ + BlueapiCall(task_request=b_api_task, parent_task_id=task.id) + for b_api_task in ( + self._construct_blueapi_tasks_from_experiment( + task.experiment + ) + ) + ] + ) + return call_list def _construct_blueapi_tasks_from_experiment( self, @@ -116,6 +81,7 @@ def _construct_blueapi_tasks_from_experiment( "exposure_time": 0.01, "metadata": { "sample": experiment.sample, + # This will include tiled background scan info "experiment_definition": experiment.experiment_definition, }, }, @@ -128,33 +94,58 @@ def _construct_blueapi_tasks_from_experiment( ), ] - def construct_blueapi_calls( - self, - queue: list[TaskWithPosition], - history: list[TaskWithPosition], - call_history: list[BlueapiCall], - ) -> list[BlueapiCall]: + def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: + new_tasks: list[Task] = [] - call_list: list[BlueapiCall] = [] + for task in tasks: + experiment = task.experiment + if isinstance(experiment, Experiment): + instrument_session = experiment.instrument_session + backgrounds = self._get_required_backgrounds(experiment) - for task in queue: - match task.experiment: - case TaskRequest(): - call_list.append( - BlueapiCall( - task_request=task.experiment, parent_task_id=task.id + for background in backgrounds: + if tiled_id := get_background_tiled_id( + background, instrument_session + ): + self._add_tiled_background_to_md( + experiment.experiment_definition.data, tiled_id, background ) - ) - case Experiment(): - call_list.extend( - [ - BlueapiCall(task_request=b_api_task, parent_task_id=task.id) - for b_api_task in ( - self._construct_blueapi_tasks_from_experiment( - task.experiment - ) - ) - ] - ) - return self._add_required_background_scans(queue, call_list) + else: + bg_experiment = self._construct_background_experiment( + background, instrument_session + ) + if bg_experiment not in [ + t.experiment for t in tasks + new_tasks + ]: + new_tasks.append(Task(experiment=bg_experiment)) + + new_tasks.append(task) + return new_tasks + + def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: + return [BackgroundInfo(bg_type="air", cobra=False, blower=False)] + + def _add_tiled_background_to_md( + self, params: dict[str, Any], tiled_id: str, background: BackgroundInfo + ): + if metadata := params.get("metadata"): + if tiled_backgrounds := metadata.get("tiled_backgrounds"): + tiled_backgrounds[tiled_id] = background + else: + metadata["tiled_backgrounds"] = {tiled_id: background} + else: + params["metadata"] = {"tiled_backgrounds": {tiled_id: background}} + + def _construct_background_experiment( + self, background: BackgroundInfo, instrument_session: str + ) -> Experiment: + return Experiment( + name="background", + instrument_session=instrument_session, + # Need to get sample info for test samples (air, empty capillary etc) + sample=Sample(name="air", id="", data={}), + experiment_definition=ExperimentDefinition( + name="background_scan", id="", data={"background": background} + ), + ) diff --git a/src/daq_queuing_service/task.py b/src/daq_queuing_service/task.py index 6303fa0..016a187 100644 --- a/src/daq_queuing_service/task.py +++ b/src/daq_queuing_service/task.py @@ -1,4 +1,3 @@ -from collections.abc import Mapping from enum import StrEnum from typing import Any, Self from uuid import uuid4 @@ -16,13 +15,13 @@ class Sample(BaseModel): name: str id: str - data: Mapping[str, Any] + data: dict[str, Any] class ExperimentDefinition(BaseModel): name: str id: str - data: Mapping[str, Any] + data: dict[str, Any] class Experiment(BaseModel): diff --git a/tests/unit_tests/plugins/test_i15_1_converter.py b/tests/unit_tests/plugins/test_i15_1_converter.py index dc96a59..0bfda6f 100644 --- a/tests/unit_tests/plugins/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/test_i15_1_converter.py @@ -19,6 +19,14 @@ ) +def assert_tasks_equal(task1: Task | TaskWithPosition, task2: Task | TaskWithPosition): + # Check two tasks are equal other than the generated UUID + copy1 = type(task1).model_validate(task1) + copy2 = type(task2).model_validate(task2) + copy1.id = copy2.id = "" + assert task1 == task2 + + @pytest.fixture(autouse=True) def background_found_in_tiled(): with patch( @@ -160,7 +168,7 @@ class BadExperiment: assert len(call_list) == 7 -def test_if_no_background_found_in_tiled_then_background_scan_added_to_call_list( +def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks( background_not_found_in_tiled: None, ): experiment = Experiment( @@ -171,37 +179,36 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_call_list sample=Sample(name="test_8_1", id="", data={}), instrument_session="cm12345-1", ) - task = TaskWithPosition( + task = Task( experiment=experiment, id="1", - status=Status.QUEUED, - blueapi_calls=[], - position=None, - kind=TaskKind.EXPERIMENT, ) - tasks = I151Converter().construct_blueapi_calls([task], [], []) - assert len(tasks) == 4 - assert tasks[0] == BlueapiCall( - task_request=TaskRequest( - name="static_collection", - params={ - "metadata": { - "background": BackgroundInfo( - bg_type="air", cobra=False, blower=False - ) - } + tasks = I151Converter().pre_process([task], [], []) + assert len(tasks) == 2 + tasks[0].id = "" + assert tasks[0].model_dump() == { + "experiment": { + "name": "background", + "instrument_session": "cm12345-1", + "sample": {"name": "air", "id": "", "data": {}}, + "experiment_definition": { + "name": "background_scan", + "id": "", + "data": { + "background": {"bg_type": "air", "cobra": False, "blower": False} + }, }, - instrument_session="cm12345-1", - ), - parent_task_id="1", - ) + }, + "id": "", + "blueapi_calls": [], + "status": Status.QUEUED, + "kind": TaskKind.EXPERIMENT, + } def test_add_required_background_scans_does_not_add_the_same_background_twice( - tasks_and_calls: tuple[list[TaskWithPosition], list[BlueapiCall]], - background_not_found_in_tiled: None, + tasks: list[Task], background_not_found_in_tiled: None ): - tasks, calls = tasks_and_calls bg_1 = BackgroundInfo(bg_type="air", cobra=False, blower=False) bg_2 = BackgroundInfo(bg_type="capillary_1", cobra=True, blower=False) bg_3 = BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True) @@ -214,101 +221,98 @@ def fake_get_required_background(self: I151Converter, experiment: Experiment): else: return [bg_3] - assert len(calls) == 15 + assert len(tasks) == 5 with patch( "daq_queuing_service.plugins.i15_1.i15_1_converter.I151Converter._get_required_backgrounds", fake_get_required_background, ): - new_calls = I151Converter()._add_required_background_scans(tasks, calls) + new_tasks = I151Converter()._add_required_background_scans(tasks) - assert len(new_calls) == 18 + assert len(new_tasks) == 8 - assert new_calls[0] == BlueapiCall( - task_request=I151Converter()._construct_background_task_request( - bg_1, instrument_session="" + assert_tasks_equal( + new_tasks[0], + Task( + experiment=I151Converter()._construct_background_experiment( + bg_1, instrument_session="" + ), ), - parent_task_id="0", ) - assert new_calls[1] == BlueapiCall( - task_request=I151Converter()._construct_background_task_request( - bg_2, instrument_session="" + assert_tasks_equal( + new_tasks[1], + Task( + experiment=I151Converter()._construct_background_experiment( + bg_2, instrument_session="" + ), ), - parent_task_id="0", ) # This one placed before the task that requires it - assert new_calls[5] == BlueapiCall( - task_request=I151Converter()._construct_background_task_request( - bg_3, instrument_session="" + assert_tasks_equal( + new_tasks[3], + Task( + experiment=I151Converter()._construct_background_experiment( + bg_3, instrument_session="" + ), ), - parent_task_id="1", ) def test_same_experiment_in_different_instrument_sessions_will_add_background_in_each( - tasks_and_calls: tuple[list[TaskWithPosition], list[BlueapiCall]], - background_not_found_in_tiled: None, + tasks: list[Task], background_not_found_in_tiled: None ): - tasks, _ = tasks_and_calls tasks[1].experiment.instrument_session = "different" - calls: list[BlueapiCall] = [] - for task in tasks: - assert isinstance(task.experiment, Experiment) - calls.extend( - [ - BlueapiCall( - task_request=task_request, - parent_task_id=task.id, - ) - for task_request in ( - I151Converter()._construct_blueapi_tasks_from_experiment( - task.experiment - ) - ) - ] - ) - - assert len(calls) == 15 - - new_calls = I151Converter()._add_required_background_scans(tasks, calls) - assert len(new_calls) == 17 - assert new_calls[0] == BlueapiCall( - task_request=TaskRequest( - name="static_collection", - params={ - "metadata": { - "background": BackgroundInfo( - bg_type="air", cobra=False, blower=False - ) - } + assert len(tasks) == 5 + + new_tasks = I151Converter()._add_required_background_scans(tasks) + + assert len(new_tasks) == 7 + new_tasks[0].id = "" + assert new_tasks[0].model_dump() == { + "experiment": { + "name": "background", + "instrument_session": "", + "sample": {"name": "air", "id": "", "data": {}}, + "experiment_definition": { + "name": "background_scan", + "id": "", + "data": { + "background": {"bg_type": "air", "cobra": False, "blower": False} + }, }, - instrument_session="", - ), - parent_task_id="0", - ) - assert new_calls[4] == BlueapiCall( - task_request=TaskRequest( - name="static_collection", - params={ - "metadata": { - "background": BackgroundInfo( - bg_type="air", cobra=False, blower=False - ) - } + }, + "id": "", + "blueapi_calls": [], + "status": Status.QUEUED, + "kind": TaskKind.EXPERIMENT, + } + new_tasks[2].id = "" + assert new_tasks[2].model_dump() == { + "experiment": { + "name": "background", + "instrument_session": "different", + "sample": {"name": "air", "id": "", "data": {}}, + "experiment_definition": { + "name": "background_scan", + "id": "", + "data": { + "background": {"bg_type": "air", "cobra": False, "blower": False} + }, }, - instrument_session="different", - ), - parent_task_id="1", - ) + }, + "id": "", + "blueapi_calls": [], + "status": Status.QUEUED, + "kind": TaskKind.EXPERIMENT, + } def test_add_required_background_scans_if_found_in_tiled_then_no_background_added( - tasks_and_calls: tuple[list[TaskWithPosition], list[BlueapiCall]], + tasks: list[Task], background_found_in_tiled: None, ): - tasks, calls_before = tasks_and_calls - calls_after = I151Converter()._add_required_background_scans(tasks, calls_before) - assert calls_after == calls_before + tasks_after = I151Converter()._add_required_background_scans(tasks) + assert tasks_after == tasks @pytest.mark.parametrize( From b8f38a5bf29d8dc56589177b4432c9974c4ceea5 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 4 Aug 2026 17:51:56 +0100 Subject: [PATCH 09/21] Fix bugs --- src/daq_queuing_service/api/errors.py | 2 ++ .../plugins/i15_1/i15_1_converter.py | 27 +++++++++++++------ .../i15_1/test_daq_queue_config.yaml | 2 +- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/daq_queuing_service/api/errors.py b/src/daq_queuing_service/api/errors.py index c8ef493..741f6d9 100644 --- a/src/daq_queuing_service/api/errors.py +++ b/src/daq_queuing_service/api/errors.py @@ -1,6 +1,7 @@ from fastapi import FastAPI, Request from fastapi.responses import JSONResponse +from daq_queuing_service.log import LOGGER from daq_queuing_service.plugins.converter import ConverterError, ValidateError from daq_queuing_service.task_queue.queue_utils import ( NegativePositionError, @@ -64,6 +65,7 @@ async def validation_error_handler(request: Request, exception: ValidateError): @app.exception_handler(ConverterError) async def converter_error_handler(request: Request, exception: ConverterError): + LOGGER.exception("Queue error occurred") return JSONResponse( status_code=422, content={"error": "converter_error", "message": str(exception)}, diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 164c124..bddc7b9 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -26,7 +26,8 @@ def pre_process( history: list[TaskWithPosition], call_history: list[BlueapiCall], ) -> list[Task]: - return self._add_required_background_scans(queue) + queue_with_backgrounds = self._add_required_background_scans(queue) + return self._remove_repeated_backgrounds(queue_with_backgrounds) def construct_blueapi_calls( self, @@ -99,7 +100,7 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: for task in tasks: experiment = task.experiment - if isinstance(experiment, Experiment): + if isinstance(experiment, Experiment) and experiment.name != "Background": instrument_session = experiment.instrument_session backgrounds = self._get_required_backgrounds(experiment) @@ -115,14 +116,24 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: bg_experiment = self._construct_background_experiment( background, instrument_session ) - if bg_experiment not in [ - t.experiment for t in tasks + new_tasks - ]: - new_tasks.append(Task(experiment=bg_experiment)) + new_tasks.append(Task(experiment=bg_experiment)) new_tasks.append(task) return new_tasks + def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: + new_tasks: list[Task] = [] + queued_background_experiments: list[Experiment] = [] + + for task in tasks: + if task.experiment.name != "Background": + new_tasks.append(task) + elif task.experiment not in queued_background_experiments: + assert isinstance(task.experiment, Experiment) + queued_background_experiments.append(task.experiment) + new_tasks.append(task) + return new_tasks + def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: return [BackgroundInfo(bg_type="air", cobra=False, blower=False)] @@ -141,10 +152,10 @@ def _construct_background_experiment( self, background: BackgroundInfo, instrument_session: str ) -> Experiment: return Experiment( - name="background", + name="Background", instrument_session=instrument_session, # Need to get sample info for test samples (air, empty capillary etc) - sample=Sample(name="air", id="", data={}), + sample=Sample(name="air_1_1", id="", data={}), experiment_definition=ExperimentDefinition( name="background_scan", id="", data={"background": background} ), diff --git a/tests/test_data/i15_1/test_daq_queue_config.yaml b/tests/test_data/i15_1/test_daq_queue_config.yaml index d677283..80937ec 100644 --- a/tests/test_data/i15_1/test_daq_queue_config.yaml +++ b/tests/test_data/i15_1/test_daq_queue_config.yaml @@ -1,6 +1,6 @@ converter: path: "daq_queuing_service.plugins.i15_1.i15_1_converter" - name: "construct_i15_1_blueapi_call_list" + name: "I151Converter" blueapi: api: url: "http://localhost:8000" From 2f5863c3f1fce3fa45eee1230a32baf906fb7258 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 4 Aug 2026 17:55:55 +0100 Subject: [PATCH 10/21] Update tiled search --- src/daq_queuing_service/plugins/i15_1/tiled_interaction.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index e995a4e..4ebddfd 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -13,7 +13,12 @@ def get_background_tiled_id( result: Container = ( client.search(Eq("start.instrument_session", instrument_session)) .search(Eq("start.instrument", "i15-1")) - .search(Eq("start.background", required_background.model_dump_json())) + .search( + Eq( + "start.experiment_definition.metadata.background", + required_background.model_dump_json(), + ) + ) ) if not len(result): From 5e131b33aa78dfd956b6729ac05d449649554364 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 4 Aug 2026 18:02:16 +0100 Subject: [PATCH 11/21] Fix tests --- .../plugins/i15_1/i15_1_converter.py | 5 ++--- .../plugins/test_get_background_tiled_id.py | 2 +- .../unit_tests/plugins/test_i15_1_converter.py | 18 +++++++++++------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index bddc7b9..846acde 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -26,8 +26,7 @@ def pre_process( history: list[TaskWithPosition], call_history: list[BlueapiCall], ) -> list[Task]: - queue_with_backgrounds = self._add_required_background_scans(queue) - return self._remove_repeated_backgrounds(queue_with_backgrounds) + return self._add_required_background_scans(queue) def construct_blueapi_calls( self, @@ -119,7 +118,7 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: new_tasks.append(Task(experiment=bg_experiment)) new_tasks.append(task) - return new_tasks + return self._remove_repeated_backgrounds(new_tasks) def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: new_tasks: list[Task] = [] diff --git a/tests/unit_tests/plugins/test_get_background_tiled_id.py b/tests/unit_tests/plugins/test_get_background_tiled_id.py index 898954f..488855e 100644 --- a/tests/unit_tests/plugins/test_get_background_tiled_id.py +++ b/tests/unit_tests/plugins/test_get_background_tiled_id.py @@ -56,7 +56,7 @@ def test_get_background_tiled_id_makes_expected_searches( search_2.search.assert_called_once_with(Eq(key="start.instrument", value="i15-1")) search_3.search.assert_called_once_with( Eq( - key="start.background", + key="start.experiment_definition.metadata.background", value='{"bg_type":"air","cobra":false,"blower":false}', ) ) diff --git a/tests/unit_tests/plugins/test_i15_1_converter.py b/tests/unit_tests/plugins/test_i15_1_converter.py index 0bfda6f..7d44811 100644 --- a/tests/unit_tests/plugins/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/test_i15_1_converter.py @@ -188,9 +188,9 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks( tasks[0].id = "" assert tasks[0].model_dump() == { "experiment": { - "name": "background", + "name": "Background", "instrument_session": "cm12345-1", - "sample": {"name": "air", "id": "", "data": {}}, + "sample": {"name": "air_1_1", "id": "", "data": {}}, "experiment_definition": { "name": "background_scan", "id": "", @@ -270,14 +270,18 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in new_tasks[0].id = "" assert new_tasks[0].model_dump() == { "experiment": { - "name": "background", + "name": "Background", "instrument_session": "", - "sample": {"name": "air", "id": "", "data": {}}, + "sample": {"name": "air_1_1", "id": "", "data": {}}, "experiment_definition": { "name": "background_scan", "id": "", "data": { - "background": {"bg_type": "air", "cobra": False, "blower": False} + "background": { + "bg_type": "air", + "cobra": False, + "blower": False, + } }, }, }, @@ -289,9 +293,9 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in new_tasks[2].id = "" assert new_tasks[2].model_dump() == { "experiment": { - "name": "background", + "name": "Background", "instrument_session": "different", - "sample": {"name": "air", "id": "", "data": {}}, + "sample": {"name": "air_1_1", "id": "", "data": {}}, "experiment_definition": { "name": "background_scan", "id": "", From 9960144ecf7418934eca7237c2529869f1e599d3 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 4 Aug 2026 18:09:43 +0100 Subject: [PATCH 12/21] Add caching to tiled query --- src/daq_queuing_service/plugins/i15_1/tiled_interaction.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 4ebddfd..5d7b3fa 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -1,10 +1,14 @@ +from cachetools import TTLCache, cached from tiled.client import from_uri from tiled.client.container import Container from tiled.queries import Eq from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo +cache: TTLCache[tuple[BackgroundInfo, str], str | None] = TTLCache(maxsize=100, ttl=1) + +@cached(cache) def get_background_tiled_id( required_background: BackgroundInfo, instrument_session: str ) -> str | None: From ee6fa4b77fd292b1f877b8837b1057474bad4535 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 4 Aug 2026 18:16:21 +0100 Subject: [PATCH 13/21] Fix tests --- src/daq_queuing_service/plugins/i15_1/backgrounds.py | 3 ++- tests/unit_tests/plugins/i15-1/conftest.py | 9 +++++++++ .../plugins/{ => i15-1}/test_get_background_tiled_id.py | 0 .../plugins/{ => i15-1}/test_i15_1_converter.py | 0 4 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/plugins/i15-1/conftest.py rename tests/unit_tests/plugins/{ => i15-1}/test_get_background_tiled_id.py (100%) rename tests/unit_tests/plugins/{ => i15-1}/test_i15_1_converter.py (100%) diff --git a/src/daq_queuing_service/plugins/i15_1/backgrounds.py b/src/daq_queuing_service/plugins/i15_1/backgrounds.py index 4eb6813..ae11a56 100644 --- a/src/daq_queuing_service/plugins/i15_1/backgrounds.py +++ b/src/daq_queuing_service/plugins/i15_1/backgrounds.py @@ -1,11 +1,12 @@ from typing import Literal -from pydantic import BaseModel +from pydantic import BaseModel, ConfigDict BACKGROUND = Literal["air", "capillary_1", "capillary_2"] class BackgroundInfo(BaseModel): + model_config = ConfigDict(frozen=True) bg_type: BACKGROUND cobra: bool blower: bool diff --git a/tests/unit_tests/plugins/i15-1/conftest.py b/tests/unit_tests/plugins/i15-1/conftest.py new file mode 100644 index 0000000..dda99cc --- /dev/null +++ b/tests/unit_tests/plugins/i15-1/conftest.py @@ -0,0 +1,9 @@ +import pytest + +from daq_queuing_service.plugins.i15_1.tiled_interaction import cache + + +@pytest.fixture(autouse=True) +def clear_cache(): + yield + cache.clear() diff --git a/tests/unit_tests/plugins/test_get_background_tiled_id.py b/tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py similarity index 100% rename from tests/unit_tests/plugins/test_get_background_tiled_id.py rename to tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py diff --git a/tests/unit_tests/plugins/test_i15_1_converter.py b/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py similarity index 100% rename from tests/unit_tests/plugins/test_i15_1_converter.py rename to tests/unit_tests/plugins/i15-1/test_i15_1_converter.py From 2864f837dfb21429470757b2be5347e531e35c11 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 7 Aug 2026 09:34:27 +0100 Subject: [PATCH 14/21] Improve capillary names --- src/daq_queuing_service/plugins/i15_1/backgrounds.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/backgrounds.py b/src/daq_queuing_service/plugins/i15_1/backgrounds.py index ae11a56..9432193 100644 --- a/src/daq_queuing_service/plugins/i15_1/backgrounds.py +++ b/src/daq_queuing_service/plugins/i15_1/backgrounds.py @@ -2,12 +2,14 @@ from pydantic import BaseModel, ConfigDict -BACKGROUND = Literal["air", "capillary_1", "capillary_2"] +# This should be generated from the json schema +# https://github.com/DiamondLightSource/daq-queuing-service/issues/78 +CAPILLARY = Literal["air", "bs", "fq", "pi"] class BackgroundInfo(BaseModel): model_config = ConfigDict(frozen=True) - bg_type: BACKGROUND + bg_type: CAPILLARY cobra: bool blower: bool From d3f944497cfb6f0209310360668dd294384fde48 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 7 Aug 2026 10:33:32 +0100 Subject: [PATCH 15/21] WIP --- .../plugins/i15_1/i15_1_converter.py | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 846acde..19c65fa 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -1,4 +1,4 @@ -from typing import Any, Literal +from typing import Any from blueapi.service.model import TaskRequest @@ -16,7 +16,7 @@ TaskWithPosition, ) -SCAN_PLANS = Literal["centre_sample", "static_collection"] +BACKGROUND_SCAN = "Background" class I151Converter(Converter): @@ -95,11 +95,23 @@ def _construct_blueapi_tasks_from_experiment( ] def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: + """Adds background scan tasks to the queue. Backgrounds will be added directly + in front of the first task in the queue that requires them. + + Args: + tasks (list[Task]): Current list of tasks + + Returns: + list[Task]: New list of tasks including backgrounds + """ new_tasks: list[Task] = [] for task in tasks: experiment = task.experiment - if isinstance(experiment, Experiment) and experiment.name != "Background": + if ( + isinstance(experiment, Experiment) + and experiment.name != BACKGROUND_SCAN + ): instrument_session = experiment.instrument_session backgrounds = self._get_required_backgrounds(experiment) @@ -125,7 +137,7 @@ def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: queued_background_experiments: list[Experiment] = [] for task in tasks: - if task.experiment.name != "Background": + if task.experiment.name != BACKGROUND_SCAN: new_tasks.append(task) elif task.experiment not in queued_background_experiments: assert isinstance(task.experiment, Experiment) @@ -134,6 +146,7 @@ def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: return new_tasks def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: + # This should be fleshed out https://github.com/DiamondLightSource/daq-queuing-service/issues/79 return [BackgroundInfo(bg_type="air", cobra=False, blower=False)] def _add_tiled_background_to_md( @@ -151,10 +164,10 @@ def _construct_background_experiment( self, background: BackgroundInfo, instrument_session: str ) -> Experiment: return Experiment( - name="Background", + name=BACKGROUND_SCAN, instrument_session=instrument_session, # Need to get sample info for test samples (air, empty capillary etc) - sample=Sample(name="air_1_1", id="", data={}), + sample=Sample(name="fq_1_1", id="", data={}), experiment_definition=ExperimentDefinition( name="background_scan", id="", data={"background": background} ), From 733f0e0da0b70392682df1ef8a1d9e42a9f209c1 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 11:38:01 +0100 Subject: [PATCH 16/21] Speed up plugin and PR comments --- .../plugins/i15_1/backgrounds.py | 10 ++-- .../plugins/i15_1/i15_1_converter.py | 15 ++++- .../plugins/i15_1/tiled_interaction.py | 53 +++++++++++------- tests/unit_tests/plugins/i15-1/conftest.py | 10 ++++ .../i15-1/test_get_background_tiled_id.py | 33 +++++------ .../plugins/i15-1/test_i15_1_converter.py | 56 ++++++------------- 6 files changed, 90 insertions(+), 87 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/backgrounds.py b/src/daq_queuing_service/plugins/i15_1/backgrounds.py index 9432193..179b272 100644 --- a/src/daq_queuing_service/plugins/i15_1/backgrounds.py +++ b/src/daq_queuing_service/plugins/i15_1/backgrounds.py @@ -4,20 +4,18 @@ # This should be generated from the json schema # https://github.com/DiamondLightSource/daq-queuing-service/issues/78 -CAPILLARY = Literal["air", "bs", "fq", "pi"] +BACKGROUND_TYPES = Literal["air", "bs", "fq", "pi"] class BackgroundInfo(BaseModel): + # Currently only room temperatures scans are supported + # https://github.com/DiamondLightSource/daq-queuing-service/issues/84 model_config = ConfigDict(frozen=True) - bg_type: CAPILLARY - cobra: bool - blower: bool + bg_type: BACKGROUND_TYPES def add_tiled_id(self, tiled_id: str) -> "TiledBackground": return TiledBackground( bg_type=self.bg_type, - cobra=self.cobra, - blower=self.blower, tiled_id=tiled_id, ) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 19c65fa..0d73ef3 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -1,12 +1,13 @@ from typing import Any from blueapi.service.model import TaskRequest +from tiled.client import from_uri # type: ignore from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall from daq_queuing_service.plugins.converter import Converter from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo from daq_queuing_service.plugins.i15_1.tiled_interaction import ( - get_background_tiled_id, + get_background_tiled_id, # type: ignore ) from daq_queuing_service.task import ( Experiment, @@ -20,6 +21,9 @@ class I151Converter(Converter): + def __init__(self): + self.tiled_client = from_uri("https://tiled.diamond.ac.uk/api/v1") # type: ignore + def pre_process( self, queue: list[Task], @@ -66,6 +70,8 @@ def _construct_blueapi_tasks_from_experiment( # Assume sample name is of form test_8_1 to load from position 8 on puck 1 _, position, puck = sample_name.split("_") + # For air calibration scans, we need to not to robot load/unload. + # https://github.com/DiamondLightSource/daq-queuing-service/issues/83 return [ TaskRequest( name="robot_load", @@ -104,6 +110,7 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: Returns: list[Task]: New list of tasks including backgrounds """ + # This can be made more robust https://github.com/DiamondLightSource/daq-queuing-service/issues/80 new_tasks: list[Task] = [] for task in tasks: @@ -117,7 +124,9 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: for background in backgrounds: if tiled_id := get_background_tiled_id( - background, instrument_session + self.tiled_client, # type: ignore + background, + instrument_session, ): self._add_tiled_background_to_md( experiment.experiment_definition.data, tiled_id, background @@ -147,7 +156,7 @@ def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: # This should be fleshed out https://github.com/DiamondLightSource/daq-queuing-service/issues/79 - return [BackgroundInfo(bg_type="air", cobra=False, blower=False)] + return [BackgroundInfo(bg_type="fq")] def _add_tiled_background_to_md( self, params: dict[str, Any], tiled_id: str, background: BackgroundInfo diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 5d7b3fa..5e1d777 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -1,37 +1,48 @@ from cachetools import TTLCache, cached -from tiled.client import from_uri from tiled.client.container import Container from tiled.queries import Eq from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo +# pyright: reportUnknownMemberType=false +# pyright: reportUnknownVariableType=false +# pyright: reportUnknownArgumentType=false +# pyright: reportUnknownLambdaType=false + cache: TTLCache[tuple[BackgroundInfo, str], str | None] = TTLCache(maxsize=100, ttl=1) -@cached(cache) def get_background_tiled_id( - required_background: BackgroundInfo, instrument_session: str + tiled_client: Container, + required_background: BackgroundInfo, + instrument_session: str, ) -> str | None: - client = from_uri("https://tiled.diamond.ac.uk/api/v1") - - result: Container = ( - client.search(Eq("start.instrument_session", instrument_session)) - .search(Eq("start.instrument", "i15-1")) - .search( - Eq( - "start.experiment_definition.metadata.background", - required_background.model_dump_json(), + + @cached(cache) + def _get_background_tiled_id( + required_background: BackgroundInfo, instrument_session: str + ) -> str | None: + + result: Container = ( + tiled_client.search(Eq("start.instrument_session", instrument_session)) + .search(Eq("start.instrument", "i15-1")) + .search( + Eq( + "start.experiment_definition.metadata.background", + required_background.model_dump_json(), + ) ) ) - ) - if not len(result): - return + if not len(result): + return + + items = sorted( + ((key, value) for key, value in result.items()), + key=lambda item: item[1].metadata["start"]["time"], + ) - items = sorted( - ((key, value) for key, value in result.items()), - key=lambda item: item[1].metadata["start"]["time"], - ) + # return the tiled ID + return items[-1][0] - # return the tiled ID - return items[-1][0] + return _get_background_tiled_id(required_background, instrument_session) diff --git a/tests/unit_tests/plugins/i15-1/conftest.py b/tests/unit_tests/plugins/i15-1/conftest.py index dda99cc..4ab8eb5 100644 --- a/tests/unit_tests/plugins/i15-1/conftest.py +++ b/tests/unit_tests/plugins/i15-1/conftest.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + import pytest from daq_queuing_service.plugins.i15_1.tiled_interaction import cache @@ -7,3 +9,11 @@ def clear_cache(): yield cache.clear() + + +@pytest.fixture(autouse=True) +def tiled_client(): + with patch( + "daq_queuing_service.plugins.i15_1.i15_1_converter.from_uri" + ) as mock_from_uri: + yield mock_from_uri.return_value diff --git a/tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py b/tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py index 488855e..00220c2 100644 --- a/tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py +++ b/tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py @@ -1,6 +1,4 @@ -from collections.abc import Generator -from typing import Any -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock import pytest from tiled.queries import Eq @@ -10,9 +8,9 @@ @pytest.fixture() -def mock_tiled_searches() -> Generator[ - tuple[MagicMock, MagicMock, MagicMock], Any, None -]: +def mock_tiled_searches( + tiled_client: MagicMock, +) -> tuple[MagicMock, MagicMock, MagicMock]: result_1 = MagicMock() result_1.metadata = {"start": {"time": 1}} result_2 = MagicMock() @@ -32,14 +30,9 @@ def mock_tiled_searches() -> Generator[ search_result_2 = MagicMock() search_result_2.search = MagicMock(return_value=search_result_3) - client = MagicMock() - client.search = MagicMock(return_value=search_result_2) + tiled_client.search = MagicMock(return_value=search_result_2) - with patch( - "daq_queuing_service.plugins.i15_1.tiled_interaction.from_uri", - MagicMock(return_value=client), - ): - yield client, search_result_2, search_result_3 + return tiled_client, search_result_2, search_result_3 def test_get_background_tiled_id_makes_expected_searches( @@ -47,7 +40,8 @@ def test_get_background_tiled_id_makes_expected_searches( ): client, search_2, search_3 = mock_tiled_searches get_background_tiled_id( - BackgroundInfo(bg_type="air", cobra=False, blower=False), + client, + BackgroundInfo(bg_type="air"), instrument_session="cm12345-1", ) client.search.assert_called_once_with( @@ -57,7 +51,7 @@ def test_get_background_tiled_id_makes_expected_searches( search_3.search.assert_called_once_with( Eq( key="start.experiment_definition.metadata.background", - value='{"bg_type":"air","cobra":false,"blower":false}', + value='{"bg_type":"air"}', ) ) @@ -65,9 +59,11 @@ def test_get_background_tiled_id_makes_expected_searches( def test_get_background_tiled_returns_most_recent_valid_background( mock_tiled_searches: tuple[MagicMock, MagicMock, MagicMock], ): + client, _, _ = mock_tiled_searches assert ( get_background_tiled_id( - BackgroundInfo(bg_type="air", cobra=False, blower=False), + client, + BackgroundInfo(bg_type="air"), instrument_session="cm12345-1", ) == "tiled_id_2" @@ -77,11 +73,12 @@ def test_get_background_tiled_returns_most_recent_valid_background( def test_get_background_tiled_id_returns_none_if_no_matching_backgrounds_found( mock_tiled_searches: tuple[MagicMock, MagicMock, MagicMock], ): - _, _, final_search = mock_tiled_searches + client, _, final_search = mock_tiled_searches final_search.search.return_value = {} assert ( get_background_tiled_id( - BackgroundInfo(bg_type="air", cobra=False, blower=False), + client, + BackgroundInfo(bg_type="air"), instrument_session="cm12345-1", ) is None diff --git a/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py b/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py index 7d44811..430555a 100644 --- a/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py @@ -190,13 +190,11 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks( "experiment": { "name": "Background", "instrument_session": "cm12345-1", - "sample": {"name": "air_1_1", "id": "", "data": {}}, + "sample": {"name": "fq_1_1", "id": "", "data": {}}, "experiment_definition": { "name": "background_scan", "id": "", - "data": { - "background": {"bg_type": "air", "cobra": False, "blower": False} - }, + "data": {"background": {"bg_type": "fq"}}, }, }, "id": "", @@ -209,9 +207,9 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks( def test_add_required_background_scans_does_not_add_the_same_background_twice( tasks: list[Task], background_not_found_in_tiled: None ): - bg_1 = BackgroundInfo(bg_type="air", cobra=False, blower=False) - bg_2 = BackgroundInfo(bg_type="capillary_1", cobra=True, blower=False) - bg_3 = BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True) + bg_1 = BackgroundInfo(bg_type="air") + bg_2 = BackgroundInfo(bg_type="bs") + bg_3 = BackgroundInfo(bg_type="fq") def fake_get_required_background(self: I151Converter, experiment: Experiment): # Get the same background scans every other experiment @@ -272,17 +270,11 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in "experiment": { "name": "Background", "instrument_session": "", - "sample": {"name": "air_1_1", "id": "", "data": {}}, + "sample": {"name": "fq_1_1", "id": "", "data": {}}, "experiment_definition": { "name": "background_scan", "id": "", - "data": { - "background": { - "bg_type": "air", - "cobra": False, - "blower": False, - } - }, + "data": {"background": {"bg_type": "fq"}}, }, }, "id": "", @@ -295,13 +287,11 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in "experiment": { "name": "Background", "instrument_session": "different", - "sample": {"name": "air_1_1", "id": "", "data": {}}, + "sample": {"name": "fq_1_1", "id": "", "data": {}}, "experiment_definition": { "name": "background_scan", "id": "", - "data": { - "background": {"bg_type": "air", "cobra": False, "blower": False} - }, + "data": {"background": {"bg_type": "fq"}}, }, }, "id": "", @@ -325,14 +315,10 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde ( {"sample": "my_sample"}, ["tiled_id"], - [BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True)], + [BackgroundInfo(bg_type="bs")], { "metadata": { - "tiled_backgrounds": { - "tiled_id": BackgroundInfo( - bg_type="capillary_1", cobra=False, blower=True - ) - } + "tiled_backgrounds": {"tiled_id": BackgroundInfo(bg_type="bs")} }, "sample": "my_sample", }, @@ -340,14 +326,10 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde ( {}, ["tiled_id"], - [BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True)], + [BackgroundInfo(bg_type="bs")], { "metadata": { - "tiled_backgrounds": { - "tiled_id": BackgroundInfo( - bg_type="capillary_1", cobra=False, blower=True - ) - } + "tiled_backgrounds": {"tiled_id": BackgroundInfo(bg_type="bs")} }, }, ), @@ -355,18 +337,14 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde {"sample": "my_sample"}, ["tiled_id_1", "tiled_id_2"], [ - BackgroundInfo(bg_type="capillary_1", cobra=False, blower=True), - BackgroundInfo(bg_type="air", cobra=True, blower=False), + BackgroundInfo(bg_type="bs"), + BackgroundInfo(bg_type="air"), ], { "metadata": { "tiled_backgrounds": { - "tiled_id_1": BackgroundInfo( - bg_type="capillary_1", cobra=False, blower=True - ), - "tiled_id_2": BackgroundInfo( - bg_type="air", cobra=True, blower=False - ), + "tiled_id_1": BackgroundInfo(bg_type="bs"), + "tiled_id_2": BackgroundInfo(bg_type="air"), } }, "sample": "my_sample", From 566687b44a4e0be4f8b5d0e364dc9fc0ca536264 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 11:59:43 +0100 Subject: [PATCH 17/21] Improve queue logging --- src/daq_queuing_service/api/errors.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/daq_queuing_service/api/errors.py b/src/daq_queuing_service/api/errors.py index 741f6d9..815f9c2 100644 --- a/src/daq_queuing_service/api/errors.py +++ b/src/daq_queuing_service/api/errors.py @@ -1,3 +1,7 @@ +from collections.abc import Awaitable, Callable +from functools import wraps +from typing import TypeVar + from fastapi import FastAPI, Request from fastapi.responses import JSONResponse @@ -13,9 +17,23 @@ # pyright: reportUnusedFunction=false +E = TypeVar("E", bound=Exception) + +Handler = Callable[[Request, E], Awaitable[JSONResponse]] + + +def log_exception(handler: Handler[E]) -> Handler[E]: + @wraps(handler) + async def wrapper(request: Request, exception: E) -> JSONResponse: + LOGGER.exception("Error while handling request: %s", request) + return await handler(request, exception) + + return wrapper + def register_exception_handlers(app: FastAPI): @app.exception_handler(TaskInProgressError) + @log_exception async def task_in_progress_handler( request: Request, exception: TaskInProgressError ): @@ -25,6 +43,7 @@ async def task_in_progress_handler( ) @app.exception_handler(TaskNotFoundError) + @log_exception async def task_not_found_handler(request: Request, exception: TaskNotFoundError): return JSONResponse( status_code=404, @@ -32,6 +51,7 @@ async def task_not_found_handler(request: Request, exception: TaskNotFoundError) ) @app.exception_handler(TaskNotInQueueError) + @log_exception async def task_not_in_queue_handler( request: Request, exception: TaskNotInQueueError ): @@ -41,6 +61,7 @@ async def task_not_in_queue_handler( ) @app.exception_handler(NegativePositionError) + @log_exception async def negative_position_handler( request: Request, exception: NegativePositionError ): @@ -50,6 +71,7 @@ async def negative_position_handler( ) @app.exception_handler(QueueError) + @log_exception async def queue_error_handler(request: Request, exception: QueueError): return JSONResponse( status_code=409, @@ -57,6 +79,7 @@ async def queue_error_handler(request: Request, exception: QueueError): ) @app.exception_handler(ValidateError) + @log_exception async def validation_error_handler(request: Request, exception: ValidateError): return JSONResponse( status_code=422, @@ -64,8 +87,9 @@ async def validation_error_handler(request: Request, exception: ValidateError): ) @app.exception_handler(ConverterError) + @log_exception async def converter_error_handler(request: Request, exception: ConverterError): - LOGGER.exception("Queue error occurred") + LOGGER.exception("Converter error") return JSONResponse( status_code=422, content={"error": "converter_error", "message": str(exception)}, From 4250e618e118af87a796f358b647e3571b417d51 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 12:09:29 +0100 Subject: [PATCH 18/21] Improve error handling and logging --- src/daq_queuing_service/api/errors.py | 109 ++++++++++---------------- 1 file changed, 42 insertions(+), 67 deletions(-) diff --git a/src/daq_queuing_service/api/errors.py b/src/daq_queuing_service/api/errors.py index 815f9c2..6f9b2cd 100644 --- a/src/daq_queuing_service/api/errors.py +++ b/src/daq_queuing_service/api/errors.py @@ -1,5 +1,4 @@ from collections.abc import Awaitable, Callable -from functools import wraps from typing import TypeVar from fastapi import FastAPI, Request @@ -22,75 +21,51 @@ Handler = Callable[[Request, E], Awaitable[JSONResponse]] -def log_exception(handler: Handler[E]) -> Handler[E]: - @wraps(handler) - async def wrapper(request: Request, exception: E) -> JSONResponse: +def make_exception_handler( + status_code: int, error_code: str +) -> Callable[[Request, Exception], Awaitable[JSONResponse]]: + async def handler(request: Request, exception: Exception): LOGGER.exception("Error while handling request: %s", request) - return await handler(request, exception) - - return wrapper - - -def register_exception_handlers(app: FastAPI): - @app.exception_handler(TaskInProgressError) - @log_exception - async def task_in_progress_handler( - request: Request, exception: TaskInProgressError - ): - return JSONResponse( - status_code=409, - content={"error": "task_in_progress", "message": str(exception)}, - ) - - @app.exception_handler(TaskNotFoundError) - @log_exception - async def task_not_found_handler(request: Request, exception: TaskNotFoundError): - return JSONResponse( - status_code=404, - content={"error": "task_not_found", "message": str(exception)}, - ) - - @app.exception_handler(TaskNotInQueueError) - @log_exception - async def task_not_in_queue_handler( - request: Request, exception: TaskNotInQueueError - ): return JSONResponse( - status_code=409, - content={"error": "task_not_in_queue", "message": str(exception)}, + status_code=status_code, + content={"error": error_code, "message": str(exception)}, ) - @app.exception_handler(NegativePositionError) - @log_exception - async def negative_position_handler( - request: Request, exception: NegativePositionError - ): - return JSONResponse( - status_code=400, - content={"error": "negative_position", "message": str(exception)}, - ) - - @app.exception_handler(QueueError) - @log_exception - async def queue_error_handler(request: Request, exception: QueueError): - return JSONResponse( - status_code=409, - content={"error": "queue_error", "message": str(exception)}, - ) + return handler - @app.exception_handler(ValidateError) - @log_exception - async def validation_error_handler(request: Request, exception: ValidateError): - return JSONResponse( - status_code=422, - content={"error": "validation_error", "message": str(exception)}, - ) - @app.exception_handler(ConverterError) - @log_exception - async def converter_error_handler(request: Request, exception: ConverterError): - LOGGER.exception("Converter error") - return JSONResponse( - status_code=422, - content={"error": "converter_error", "message": str(exception)}, - ) +def register_exception_handlers(app: FastAPI): + app.add_exception_handler( + TaskInProgressError, + make_exception_handler(409, "task_in_progress"), + ) + + app.add_exception_handler( + TaskNotFoundError, + make_exception_handler(404, "task_not_found"), + ) + + app.add_exception_handler( + TaskNotInQueueError, + make_exception_handler(409, "task_not_in_queue"), + ) + + app.add_exception_handler( + NegativePositionError, + make_exception_handler(400, "negative_position"), + ) + + app.add_exception_handler( + QueueError, + make_exception_handler(409, "queue_error"), + ) + + app.add_exception_handler( + ValidateError, + make_exception_handler(422, "validation_error"), + ) + + app.add_exception_handler( + ConverterError, + make_exception_handler(422, "converter_error"), + ) From bf60535a4ff0525aed4d49c148f48a0b41773ff1 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 13:29:14 +0100 Subject: [PATCH 19/21] Add more logging --- .../plugins/i15_1/i15_1_converter.py | 8 ++++++++ .../plugins/i15_1/tiled_interaction.py | 11 ++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 0d73ef3..e607f4e 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -4,6 +4,7 @@ from tiled.client import from_uri # type: ignore from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall +from daq_queuing_service.log import LOGGER from daq_queuing_service.plugins.converter import Converter from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo from daq_queuing_service.plugins.i15_1.tiled_interaction import ( @@ -110,6 +111,8 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: Returns: list[Task]: New list of tasks including backgrounds """ + LOGGER.info("Adding required background scans") + # This can be made more robust https://github.com/DiamondLightSource/daq-queuing-service/issues/80 new_tasks: list[Task] = [] @@ -142,6 +145,7 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: return self._remove_repeated_backgrounds(new_tasks) def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: + LOGGER.info("Removing repeated background scans") new_tasks: list[Task] = [] queued_background_experiments: list[Experiment] = [] @@ -152,6 +156,8 @@ def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: assert isinstance(task.experiment, Experiment) queued_background_experiments.append(task.experiment) new_tasks.append(task) + else: + LOGGER.debug(f"Removing repeated background scan: {task.experiment}") return new_tasks def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: @@ -161,6 +167,7 @@ def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundIn def _add_tiled_background_to_md( self, params: dict[str, Any], tiled_id: str, background: BackgroundInfo ): + LOGGER.debug("Adding background scan tiled info to metadata") if metadata := params.get("metadata"): if tiled_backgrounds := metadata.get("tiled_backgrounds"): tiled_backgrounds[tiled_id] = background @@ -172,6 +179,7 @@ def _add_tiled_background_to_md( def _construct_background_experiment( self, background: BackgroundInfo, instrument_session: str ) -> Experiment: + LOGGER.debug(f"Constructing experiment for background: {background}") return Experiment( name=BACKGROUND_SCAN, instrument_session=instrument_session, diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 5e1d777..6e265b7 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -2,6 +2,7 @@ from tiled.client.container import Container from tiled.queries import Eq +from daq_queuing_service.log import LOGGER from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo # pyright: reportUnknownMemberType=false @@ -35,6 +36,9 @@ def _get_background_tiled_id( ) if not len(result): + LOGGER.debug( + f"Found no scans in tiled matching background: {required_background}" + ) return items = sorted( @@ -43,6 +47,11 @@ def _get_background_tiled_id( ) # return the tiled ID - return items[-1][0] + tiled_id = items[-1][0] + LOGGER.debug( + f"Found {len(items)} scans in tiled matching background: " + + f"{required_background}. Returning the first: {tiled_id}" + ) + return tiled_id return _get_background_tiled_id(required_background, instrument_session) From 29f8bbcceb39bb4d86d4d8c45eb57bbdc53533a3 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 14:27:24 +0100 Subject: [PATCH 20/21] Remove uneeded type: ignore --- src/daq_queuing_service/plugins/i15_1/i15_1_converter.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py index 8a28808..bd82c4c 100644 --- a/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py +++ b/src/daq_queuing_service/plugins/i15_1/i15_1_converter.py @@ -2,14 +2,13 @@ from blueapi.service.model import TaskRequest from tiled.client import from_uri # type: ignore +from tiled.client.container import Container from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall from daq_queuing_service.log import LOGGER from daq_queuing_service.plugins.converter import Converter from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo -from daq_queuing_service.plugins.i15_1.tiled_interaction import ( - get_background_tiled_id, # type: ignore -) +from daq_queuing_service.plugins.i15_1.tiled_interaction import get_background_tiled_id from daq_queuing_service.task_queue.task import ( Experiment, ExperimentDefinition, @@ -23,7 +22,7 @@ class I151Converter(Converter): def __init__(self): - self.tiled_client = from_uri("https://tiled.diamond.ac.uk/api/v1") # type: ignore + self.tiled_client: Container = from_uri("https://tiled.diamond.ac.uk/api/v1") def pre_process( self, @@ -127,7 +126,7 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: for background in backgrounds: if tiled_id := get_background_tiled_id( - self.tiled_client, # type: ignore + self.tiled_client, background, instrument_session, ): From d91a1a1d73393917cf7e04a33ac511ec5a079f8d Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 13:41:14 +0100 Subject: [PATCH 21/21] Add comment --- src/daq_queuing_service/plugins/i15_1/tiled_interaction.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py index 6e265b7..5216857 100644 --- a/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py +++ b/src/daq_queuing_service/plugins/i15_1/tiled_interaction.py @@ -5,6 +5,7 @@ from daq_queuing_service.log import LOGGER from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo +# Ignoring the following rules as the tiled client is poorly typed as scares the linter # pyright: reportUnknownMemberType=false # pyright: reportUnknownVariableType=false # pyright: reportUnknownArgumentType=false