diff --git a/src/daq_queuing_service/plugins/converter.py b/src/daq_queuing_service/plugins/converter.py index 5124080..c0a6545 100644 --- a/src/daq_queuing_service/plugins/converter.py +++ b/src/daq_queuing_service/plugins/converter.py @@ -6,7 +6,10 @@ from daq_queuing_service.task_queue.task import Experiment, Task, TaskWithPosition -class ConverterError(Exception): ... +class ConverterError(Exception): + def __init__(self, original: Exception): + super().__init__(f"{type(original).__name__}: {original}") + self.original = original class ValidateError(Exception): ... diff --git a/src/daq_queuing_service/plugins/i15_1/backgrounds.py b/src/daq_queuing_service/plugins/i15_1/backgrounds.py index 179b272..995f3cc 100644 --- a/src/daq_queuing_service/plugins/i15_1/backgrounds.py +++ b/src/daq_queuing_service/plugins/i15_1/backgrounds.py @@ -12,11 +12,11 @@ class BackgroundInfo(BaseModel): # https://github.com/DiamondLightSource/daq-queuing-service/issues/84 model_config = ConfigDict(frozen=True) bg_type: BACKGROUND_TYPES + time_per_pdf: int def add_tiled_id(self, tiled_id: str) -> "TiledBackground": return TiledBackground( - bg_type=self.bg_type, - tiled_id=tiled_id, + bg_type=self.bg_type, tiled_id=tiled_id, time_per_pdf=self.time_per_pdf ) 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 97db4b3..740c86d 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 @@ -65,6 +65,7 @@ def _construct_blueapi_tasks_from_experiment( self, experiment: Experiment, ) -> list[TaskRequest]: + LOGGER.debug(f"Converting to blueapi calls, experiment = {experiment}") 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("_") @@ -155,6 +156,14 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: # This can be made more robust https://github.com/DiamondLightSource/daq-queuing-service/issues/80 new_tasks: list[Task] = [] + pdf_times = [ + task.experiment.experiment_definition.data["time_per_pdf"] + for task in tasks + if isinstance(task.experiment, Experiment) + and "time_per_pdf" in task.experiment.experiment_definition.data + ] + max_time_per_pdf = max(pdf_times) if pdf_times else 10 + for task in tasks: experiment = task.experiment if ( @@ -162,7 +171,9 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]: and experiment.name != BACKGROUND_SCAN ): instrument_session = experiment.instrument_session - backgrounds = self._get_required_backgrounds(experiment) + backgrounds = self._get_required_backgrounds( + experiment, max_time_per_pdf + ) for background in backgrounds: if tiled_id := get_background_tiled_id( @@ -199,9 +210,11 @@ def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]: LOGGER.debug(f"Removing repeated background scan: {task.experiment}") return new_tasks - def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]: + def _get_required_backgrounds( + self, experiment: Experiment, time_per_pdf: int + ) -> list[BackgroundInfo]: # This should be fleshed out https://github.com/DiamondLightSource/daq-queuing-service/issues/79 - return [BackgroundInfo(bg_type="fq")] + return [BackgroundInfo(bg_type="fq", time_per_pdf=time_per_pdf)] def _add_tiled_background_to_md( self, params: dict[str, Any], tiled_id: str, background: BackgroundInfo @@ -225,6 +238,8 @@ def _construct_background_experiment( # Need to get sample info for test samples (air, empty capillary etc) sample=Sample(name="fq_1_1", id="", data={}), experiment_definition=ExperimentDefinition( - name="background_scan", id="", data={"background": background} + name="background_scan", + id="", + data={"background": background, "time_per_pdf": 10}, ), ) diff --git a/src/daq_queuing_service/task_queue/queue.py b/src/daq_queuing_service/task_queue/queue.py index b9b26e5..362b657 100644 --- a/src/daq_queuing_service/task_queue/queue.py +++ b/src/daq_queuing_service/task_queue/queue.py @@ -136,6 +136,7 @@ def _sync(self): modified, and also right before a call is popped off the front of the queue. """ LOGGER.debug("Syncing") + LOGGER.debug(f"Queue before sync: {self._queue}") for task_id in list(self._queue): task = self._tasks[task_id] if task.status in (Status.COMPLETE, Status.ERROR): @@ -162,7 +163,7 @@ def _sync(self): self._queue_history, ) except Exception as e: - raise ConverterError(*e.args) from e + raise ConverterError(e) from e # Update task_registry to match new tasks # Not needed as long as pre_process modifies in place @@ -200,7 +201,7 @@ def _sync(self): self._queue_history, ) except Exception as e: - raise ConverterError(*e.args) from e + raise ConverterError(e) from e self._call_queue.extend(new_calls) @@ -215,6 +216,7 @@ def _sync(self): self._save_contents() self._broadcast_changes() self._modifying.notify_all() + LOGGER.debug(f"Queue after sync: {self._queue}") def _copy_contents(self) -> QueueContents: return deepcopy( @@ -231,13 +233,18 @@ def _save_contents(self): self._last_good_contents = self._copy_contents() def _restore_from_contents(self, contents: QueueContents): - self._tasks = TaskRegistry(contents["tasks"]) - self._queue = contents["queue"] - self._history = contents["history"] - self._call_queue = contents["call_queue"] - self._call_history = contents["call_history"] + LOGGER.info(f"Restoring to contents: {contents}") + + restored = deepcopy(contents) + + self._tasks = TaskRegistry(restored["tasks"]) + self._queue = restored["queue"] + self._history = restored["history"] + self._call_queue = restored["call_queue"] + self._call_history = restored["call_history"] def _restore_latest_good_contents(self): + LOGGER.info("Restoring to last good contents") self._restore_from_contents(self._last_good_contents) def _broadcast_changes(self): 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 00220c2..f9399fa 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 @@ -41,7 +41,7 @@ def test_get_background_tiled_id_makes_expected_searches( client, search_2, search_3 = mock_tiled_searches get_background_tiled_id( client, - BackgroundInfo(bg_type="air"), + BackgroundInfo(bg_type="air", time_per_pdf=10), instrument_session="cm12345-1", ) client.search.assert_called_once_with( @@ -51,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"}', + value='{"bg_type":"air","time_per_pdf":10}', ) ) @@ -63,7 +63,7 @@ def test_get_background_tiled_returns_most_recent_valid_background( assert ( get_background_tiled_id( client, - BackgroundInfo(bg_type="air"), + BackgroundInfo(bg_type="air", time_per_pdf=10), instrument_session="cm12345-1", ) == "tiled_id_2" @@ -78,7 +78,7 @@ def test_get_background_tiled_id_returns_none_if_no_matching_backgrounds_found( assert ( get_background_tiled_id( client, - BackgroundInfo(bg_type="air"), + BackgroundInfo(bg_type="air", time_per_pdf=10), 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 366eb39..8a31534 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 @@ -6,8 +6,10 @@ from blueapi.service.model import TaskRequest from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall +from daq_queuing_service.broadcaster import Broadcaster from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo from daq_queuing_service.plugins.i15_1.i15_1_converter import I151Converter +from daq_queuing_service.task_queue.queue import TaskQueue from daq_queuing_service.task_queue.task import ( Experiment, ExperimentDefinition, @@ -27,6 +29,34 @@ def assert_tasks_equal(task1: Task | TaskWithPosition, task2: Task | TaskWithPos assert task1 == task2 +@pytest.fixture +async def queue_with_i15_1_plugin(background_not_found_in_tiled: None): + queue = TaskQueue(converter=I151Converter(), broadcaster=Broadcaster()) + tasks = [ + Task( + experiment=Experiment( + name=f"task_{i}", + instrument_session="cm12345-1", + experiment_definition=ExperimentDefinition( + name="", + id="", + data={ + "list_of_temperatures": [100 * i, 100 * i + 20], + "time_per_pdf": i, + "settle_time": 5, + "ramp_rate": 10, + }, + ), + sample=Sample(name=f"sample_{i}_2", id=str(i), data={}), + ) + ) + for i in range(5) + ] + await queue.add_tasks(tasks) + await queue.resume_queue() + return queue + + @pytest.fixture(autouse=True) def background_found_in_tiled(): with patch( @@ -256,7 +286,10 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks( "experiment_definition": { "name": "background_scan", "id": "", - "data": {"background": {"bg_type": "fq"}}, + "data": { + "background": {"bg_type": "fq", "time_per_pdf": 100}, + "time_per_pdf": 10, + }, }, }, "id": "", @@ -270,11 +303,13 @@ 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") - bg_2 = BackgroundInfo(bg_type="bs") - bg_3 = BackgroundInfo(bg_type="fq") + bg_1 = BackgroundInfo(bg_type="air", time_per_pdf=5) + bg_2 = BackgroundInfo(bg_type="bs", time_per_pdf=10) + bg_3 = BackgroundInfo(bg_type="fq", time_per_pdf=15) - def fake_get_required_background(self: I151Converter, experiment: Experiment): + def fake_get_required_background( + self: I151Converter, experiment: Experiment, max_time_per_pdf: int + ): # Get the same background scans every other experiment # Only one of each background should be added if int(experiment.sample.id) % 2 == 0: @@ -337,7 +372,10 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in "experiment_definition": { "name": "background_scan", "id": "", - "data": {"background": {"bg_type": "fq"}}, + "data": { + "background": {"bg_type": "fq", "time_per_pdf": 10}, + "time_per_pdf": 10, + }, }, }, "id": "", @@ -355,7 +393,10 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in "experiment_definition": { "name": "background_scan", "id": "", - "data": {"background": {"bg_type": "fq"}}, + "data": { + "background": {"bg_type": "fq", "time_per_pdf": 10}, + "time_per_pdf": 10, + }, }, }, "id": "", @@ -380,10 +421,12 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde ( {"sample": "my_sample"}, ["tiled_id"], - [BackgroundInfo(bg_type="bs")], + [BackgroundInfo(bg_type="bs", time_per_pdf=5)], { "metadata": { - "tiled_backgrounds": {"tiled_id": BackgroundInfo(bg_type="bs")} + "tiled_backgrounds": { + "tiled_id": BackgroundInfo(bg_type="bs", time_per_pdf=5) + } }, "sample": "my_sample", }, @@ -391,10 +434,12 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde ( {}, ["tiled_id"], - [BackgroundInfo(bg_type="bs")], + [BackgroundInfo(bg_type="bs", time_per_pdf=5)], { "metadata": { - "tiled_backgrounds": {"tiled_id": BackgroundInfo(bg_type="bs")} + "tiled_backgrounds": { + "tiled_id": BackgroundInfo(bg_type="bs", time_per_pdf=5) + } }, }, ), @@ -402,14 +447,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="bs"), - BackgroundInfo(bg_type="air"), + BackgroundInfo(bg_type="bs", time_per_pdf=5), + BackgroundInfo(bg_type="air", time_per_pdf=5), ], { "metadata": { "tiled_backgrounds": { - "tiled_id_1": BackgroundInfo(bg_type="bs"), - "tiled_id_2": BackgroundInfo(bg_type="air"), + "tiled_id_1": BackgroundInfo(bg_type="bs", time_per_pdf=5), + "tiled_id_2": BackgroundInfo(bg_type="air", time_per_pdf=5), } }, "sample": "my_sample", @@ -427,3 +472,9 @@ def test_add_tiled_background_to_md_adds_expected_metadata( I151Converter()._add_tiled_background_to_md(params, tiled_id, background) assert params == expected_params + + +async def test_queue_with_i15_1_converter_can_sync(queue_with_i15_1_plugin: TaskQueue): + first_task = await queue_with_i15_1_plugin.get_task_by_position(0) + assert first_task + await queue_with_i15_1_plugin.move_task(first_task.id, 2) diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index b3296c3..fb1bbf3 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -449,7 +449,7 @@ def fail_conversion( assert response.status_code == 422 assert response.json() == { "error": "converter_error", - "message": "Conversion failed because xyz", + "message": "SomeError: Conversion failed because xyz", } assert task_queue_with_history._queue == ["2", "3", "4"]