From 4cfe0cc0db1a2f855b04b381ff2cd1c35e97cb84 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Mon, 10 Nov 2025 14:54:34 +0000 Subject: [PATCH 01/14] Check redis connection and thaw if connection fails --- src/mx_bluesky/beamlines/i04/thawing_plan.py | 37 ++++++++++++ .../unit_tests/beamlines/i04/test_thawing.py | 56 +++++++++++++++++-- 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/thawing_plan.py b/src/mx_bluesky/beamlines/i04/thawing_plan.py index b0c9973af..2992bc4af 100644 --- a/src/mx_bluesky/beamlines/i04/thawing_plan.py +++ b/src/mx_bluesky/beamlines/i04/thawing_plan.py @@ -10,6 +10,8 @@ from dodal.devices.smargon import Smargon from dodal.devices.thawer import OnOff, Thawer from dodal.log import LOGGER +from redis import StrictRedis +from redis.exceptions import ConnectionError from mx_bluesky.beamlines.i04.callbacks.murko_callback import MurkoCallback @@ -49,6 +51,18 @@ def cleanup(): ) +def check_redis_connection(redis_host, redis_password, redis_db) -> bool: + redis_client = StrictRedis(host=redis_host, password=redis_password, db=redis_db) + try: + redis_client.ping() + return True + except ConnectionError: + LOGGER.warning( + f"Failed to connect to redis with \nhost: {redis_host}, password: {redis_password}, db: {redis_db}" + ) + return False + + def thaw_and_murko_centre( time_to_thaw: float, rotation: float = 360, @@ -78,6 +92,17 @@ def thaw_and_murko_centre( ... devices: These are the specific ophyd-devices used for the plan, the defaults are always correct """ + + if not check_redis_connection( + RedisConstants.REDIS_HOST, + RedisConstants.REDIS_PASSWORD, + RedisConstants.MURKO_REDIS_DB, + ): + yield from thaw( + time_to_thaw=time_to_thaw, rotation=rotation, thawer=thawer, smargon=smargon + ) + return + murko_results_group = "get_results" sample_id = yield from bps.rd(robot.sample_id) @@ -88,6 +113,7 @@ def thaw_and_murko_centre( initial_zoom_level = yield from bps.rd(oav_fs.zoom_controller.level) initial_velocity = yield from bps.rd(smargon.omega.velocity) new_velocity = abs(rotation / time_to_thaw) * 2.0 + murko_callback = MurkoCallback( RedisConstants.REDIS_HOST, RedisConstants.REDIS_PASSWORD, @@ -174,6 +200,17 @@ def thaw_and_stream_to_redis( ... devices: These are the specific ophyd-devices used for the plan, the defaults are always correct """ + + if not check_redis_connection( + RedisConstants.REDIS_HOST, + RedisConstants.REDIS_PASSWORD, + RedisConstants.MURKO_REDIS_DB, + ): + yield from thaw( + time_to_thaw=time_to_thaw, rotation=rotation, thawer=thawer, smargon=smargon + ) + return + sample_id = yield from bps.rd(robot.sample_id) sample_id = int(sample_id) diff --git a/tests/unit_tests/beamlines/i04/test_thawing.py b/tests/unit_tests/beamlines/i04/test_thawing.py index 9b1fb7eb1..928f1c619 100644 --- a/tests/unit_tests/beamlines/i04/test_thawing.py +++ b/tests/unit_tests/beamlines/i04/test_thawing.py @@ -116,6 +116,15 @@ async def oav_forwarder(oav_full_screen: OAV, oav_roi: OAV) -> OAVToRedisForward return oav_forwarder +@pytest.fixture +def thaw_and_murko_centre_with_mock_check_redis_connection(): + with patch( + "mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection" + ) as mock_check_redis_connection: + mock_check_redis_connection.return_value = True + yield thaw_and_murko_centre + + @pytest.fixture def robot() -> BartRobot: return i04.robot(connect_immediately=True, mock=True) @@ -405,6 +414,7 @@ def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( mock_rotate_and_stream, patch_murko_callback, + thaw_and_murko_centre_with_mock_check_redis_connection, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -413,7 +423,7 @@ def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( run_engine: RunEngine, ): run_engine( - thaw_and_murko_centre( + thaw_and_murko_centre_with_mock_check_redis_connection( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -426,6 +436,7 @@ def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( mock__thaw, patch_murko_callback, + thaw_and_murko_centre_with_mock_check_redis_connection, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -437,7 +448,7 @@ def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( with pytest.raises(ValueError): run_engine( - thaw_and_murko_centre( + thaw_and_murko_centre_with_mock_check_redis_connection( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -447,6 +458,7 @@ def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( + thaw_and_murko_centre_with_mock_check_redis_connection, sim_run_engine, smargon: Smargon, thawer: Thawer, @@ -456,7 +468,7 @@ def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( ): _test_plan_will_switch_murko_source_half_way_through_thaw( sim_run_engine, - thaw_and_murko_centre( + thaw_and_murko_centre_with_mock_check_redis_connection( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -465,6 +477,7 @@ def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_thaw_and_murko_centre_will_centre_based_on_murko_results_after_both_rotations( patch_murko_callback, + thaw_and_murko_centre_with_mock_check_redis_connection, run_engine, smargon: Smargon, thawer: Thawer, @@ -494,7 +507,7 @@ def side_effect(): mock_trigger.side_effect = side_effect run_engine( - thaw_and_murko_centre( + thaw_and_murko_centre_with_mock_check_redis_connection( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -508,6 +521,7 @@ def side_effect(): def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( + thaw_and_murko_centre_with_mock_check_redis_connection, sim_run_engine, smargon: Smargon, thawer: Thawer, @@ -518,7 +532,7 @@ def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( sim_run_engine.add_read_handler_for(robot.sample_id, "1234") msgs = sim_run_engine.simulate_plan( - thaw_and_murko_centre( + thaw_and_murko_centre_with_mock_check_redis_connection( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ) ) @@ -535,6 +549,7 @@ def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( + thaw_and_murko_centre_with_mock_check_redis_connection, run_engine, smargon: Smargon, thawer: Thawer, @@ -548,7 +563,7 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( ) as mock_murko_callback: mock_murko_callback.return_value = murko_callback run_engine( - thaw_and_murko_centre( + thaw_and_murko_centre_with_mock_check_redis_connection( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -583,3 +598,32 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( assert publish_call_args_list[1].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) assert publish_call_args_list[2].args[1] == json.dumps(expected_roi_md) assert publish_call_args_list[3].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) + + +@patch("mx_bluesky.beamlines.i04.thawing_plan.RedisConstants") +@patch("mx_bluesky.beamlines.i04.thawing_plan.thaw") +def test_plans_do_thaw_if_redis_connection_check_fails( + patch_thaw: MagicMock, + patch_redis_constants: MagicMock, + smargon: Smargon, + thawer: Thawer, + robot: BartRobot, + oav_forwarder: OAVToRedisForwarder, + run_engine: RunEngine, +): + for plan in (thaw_and_murko_centre, thaw_and_stream_to_redis): + run_engine( + plan( + 10, + 360, + thawer=thawer, + smargon=smargon, + robot=robot, + oav_to_redis_forwarder=oav_forwarder, + ) + ) + + patch_thaw.assert_called_once_with( + time_to_thaw=10, rotation=360, thawer=thawer, smargon=smargon + ) + patch_thaw.reset_mock() From 779870c998df27c5a92e5854729108bbb52b6843 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Mon, 10 Nov 2025 15:24:55 +0000 Subject: [PATCH 02/14] Clean up tests --- .../unit_tests/beamlines/i04/test_thawing.py | 106 +++++++++++------- 1 file changed, 67 insertions(+), 39 deletions(-) diff --git a/tests/unit_tests/beamlines/i04/test_thawing.py b/tests/unit_tests/beamlines/i04/test_thawing.py index 928f1c619..d452def86 100644 --- a/tests/unit_tests/beamlines/i04/test_thawing.py +++ b/tests/unit_tests/beamlines/i04/test_thawing.py @@ -1,4 +1,5 @@ import json +import logging from functools import partial from unittest.mock import ANY, AsyncMock, MagicMock, call, patch @@ -116,15 +117,6 @@ async def oav_forwarder(oav_full_screen: OAV, oav_roi: OAV) -> OAVToRedisForward return oav_forwarder -@pytest.fixture -def thaw_and_murko_centre_with_mock_check_redis_connection(): - with patch( - "mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection" - ) as mock_check_redis_connection: - mock_check_redis_connection.return_value = True - yield thaw_and_murko_centre - - @pytest.fixture def robot() -> BartRobot: return i04.robot(connect_immediately=True, mock=True) @@ -211,15 +203,18 @@ def test_given_different_rotations_then_motor_moved_relative( ] +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") async def test_thaw_and_stream_sets_sample_id_and_kicks_off_forwarder( patch_murko_callback: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, robot: BartRobot, run_engine: RunEngine, ): + patch_check_redis_connection.return_value = True set_mock_value(robot.sample_id, 100) run_engine( thaw_and_stream_to_redis( @@ -236,15 +231,18 @@ async def test_thaw_and_stream_sets_sample_id_and_kicks_off_forwarder( oav_forwarder.complete.assert_called() # type: ignore +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_thaw_and_stream_adds_murko_callback_and_produces_expected_messages( patch_murko_callback: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, oav_forwarder: OAVToRedisForwarder, run_engine: RunEngine, ): + patch_check_redis_connection.return_value = True patch_murko_instance = patch_murko_callback.return_value run_engine( thaw_and_stream_to_redis( @@ -270,17 +268,21 @@ def test_thaw_and_stream_adds_murko_callback_and_produces_expected_messages( assert len(smargon_updates) > 0 +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback.stop") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback.call_murko") def test_thaw_and_stream_will_produce_events_that_call_murko( patch_murko_call: MagicMock, patch_stop_call: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, oav_forwarder: OAVToRedisForwarder, run_engine: RunEngine, ): + patch_check_redis_connection.return_value = True + class StopPlanError(Exception): pass @@ -328,13 +330,16 @@ def _test_plan_will_switch_murko_source_half_way_through_thaw( ) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thaw_and_stream_will_switch_murko_source_half_way_through_thaw( + patch_check_redis_connection: MagicMock, sim_run_engine, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, robot: BartRobot, ): + patch_check_redis_connection.return_value = True _test_plan_will_switch_murko_source_half_way_through_thaw( sim_run_engine, thaw_and_stream_to_redis(10, 360, robot, thawer, smargon, oav_forwarder), @@ -374,9 +379,11 @@ def _run_thaw_and_stream_and_assert_zoom_changes( mock_level_set.assert_has_calls([call("1.0x", wait=True), call("2.0x", wait=True)]) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_given_thaw_succeeds_then_thaw_and_stream_sets_zoom_to_1_and_back( - patch_murko_callback, + patch_murko_callback: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -384,16 +391,19 @@ def test_given_thaw_succeeds_then_thaw_and_stream_sets_zoom_to_1_and_back( robot: BartRobot, run_engine: RunEngine, ): + patch_check_redis_connection.return_value = True _run_thaw_and_stream_and_assert_zoom_changes( smargon, thawer, oav_forwarder, oav_full_screen, robot, run_engine ) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") @patch("mx_bluesky.beamlines.i04.thawing_plan.bps.monitor") def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( - mock__thaw, - patch_murko_callback, + mock__thaw: MagicMock, + patch_murko_callback: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -401,20 +411,22 @@ def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( robot: BartRobot, run_engine: RunEngine, ): + patch_check_redis_connection.return_value = True mock__thaw.side_effect = Exception() _run_thaw_and_stream_and_assert_zoom_changes( smargon, thawer, oav_forwarder, oav_full_screen, robot, run_engine, Exception ) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") @patch( "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" ) def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( - mock_rotate_and_stream, - patch_murko_callback, - thaw_and_murko_centre_with_mock_check_redis_connection, + mock_rotate_and_stream: MagicMock, + patch_murko_callback: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -423,7 +435,7 @@ def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( run_engine: RunEngine, ): run_engine( - thaw_and_murko_centre_with_mock_check_redis_connection( + thaw_and_murko_centre( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -431,12 +443,13 @@ def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( assert murko_results.unstage.call_count == 2 # type: ignore +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") @patch("mx_bluesky.beamlines.i04.thawing_plan.bps.monitor") def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( - mock__thaw, - patch_murko_callback, - thaw_and_murko_centre_with_mock_check_redis_connection, + mock__thaw: MagicMock, + patch_murko_callback: MagicMock, + patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -448,7 +461,7 @@ def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( with pytest.raises(ValueError): run_engine( - thaw_and_murko_centre_with_mock_check_redis_connection( + thaw_and_murko_centre( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -457,8 +470,9 @@ def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( murko_results.unstage.assert_called_once() # type: ignore +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( - thaw_and_murko_centre_with_mock_check_redis_connection, + patch_check_redis_connection: MagicMock, sim_run_engine, smargon: Smargon, thawer: Thawer, @@ -466,18 +480,20 @@ def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( robot: BartRobot, murko_results: MurkoResultsDevice, ): + patch_check_redis_connection.return_value = True _test_plan_will_switch_murko_source_half_way_through_thaw( sim_run_engine, - thaw_and_murko_centre_with_mock_check_redis_connection( + thaw_and_murko_centre( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_thaw_and_murko_centre_will_centre_based_on_murko_results_after_both_rotations( patch_murko_callback, - thaw_and_murko_centre_with_mock_check_redis_connection, + patch_check_redis_connection: MagicMock, run_engine, smargon: Smargon, thawer: Thawer, @@ -485,6 +501,8 @@ def test_thaw_and_murko_centre_will_centre_based_on_murko_results_after_both_rot robot: BartRobot, murko_results: MurkoResultsDevice, ): + patch_check_redis_connection.return_value = True + def fake_trigger(call_count): if call_count == 1: murko_results._x_mm_setter(1) @@ -507,7 +525,7 @@ def side_effect(): mock_trigger.side_effect = side_effect run_engine( - thaw_and_murko_centre_with_mock_check_redis_connection( + thaw_and_murko_centre( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -520,8 +538,9 @@ def side_effect(): get_mock_put(smargon.z.user_setpoint).assert_has_calls([call(9.0, wait=True)]) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( - thaw_and_murko_centre_with_mock_check_redis_connection, + patch_check_redis_connection: MagicMock, sim_run_engine, smargon: Smargon, thawer: Thawer, @@ -529,10 +548,11 @@ def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( robot: BartRobot, murko_results: MurkoResultsDevice, ): + patch_check_redis_connection.return_value = True sim_run_engine.add_read_handler_for(robot.sample_id, "1234") msgs = sim_run_engine.simulate_plan( - thaw_and_murko_centre_with_mock_check_redis_connection( + thaw_and_murko_centre( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ) ) @@ -548,8 +568,9 @@ def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( ) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( - thaw_and_murko_centre_with_mock_check_redis_connection, + patch_check_redis_connection: MagicMock, run_engine, smargon: Smargon, thawer: Thawer, @@ -558,12 +579,13 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( murko_results: MurkoResultsDevice, murko_callback: MurkoCallback, ): + patch_check_redis_connection.return_value = True with patch( "mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback" ) as mock_murko_callback: mock_murko_callback.return_value = murko_callback run_engine( - thaw_and_murko_centre_with_mock_check_redis_connection( + thaw_and_murko_centre( 10, 360, robot, thawer, smargon, murko_results, oav_forwarder ), ) @@ -600,30 +622,36 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( assert publish_call_args_list[3].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) -@patch("mx_bluesky.beamlines.i04.thawing_plan.RedisConstants") @patch("mx_bluesky.beamlines.i04.thawing_plan.thaw") def test_plans_do_thaw_if_redis_connection_check_fails( patch_thaw: MagicMock, - patch_redis_constants: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, oav_forwarder: OAVToRedisForwarder, run_engine: RunEngine, + caplog: pytest.LogCaptureFixture, ): for plan in (thaw_and_murko_centre, thaw_and_stream_to_redis): - run_engine( - plan( - 10, - 360, - thawer=thawer, - smargon=smargon, - robot=robot, - oav_to_redis_forwarder=oav_forwarder, + with caplog.at_level(logging.WARNING): + run_engine( + plan( + 10, + 360, + thawer=thawer, + smargon=smargon, + robot=robot, + oav_to_redis_forwarder=oav_forwarder, + ) ) - ) patch_thaw.assert_called_once_with( time_to_thaw=10, rotation=360, thawer=thawer, smargon=smargon ) + assert any( + record.message.startswith("Failed to connect to redis") + and record.levelname == "WARNING" + for record in caplog.records + ) + caplog.clear() patch_thaw.reset_mock() From bfd0963f92e738c9b771c192109329335e34361b Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Mon, 10 Nov 2025 15:44:06 +0000 Subject: [PATCH 03/14] Small change --- src/mx_bluesky/beamlines/i04/thawing_plan.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/thawing_plan.py b/src/mx_bluesky/beamlines/i04/thawing_plan.py index 2992bc4af..60eeff64c 100644 --- a/src/mx_bluesky/beamlines/i04/thawing_plan.py +++ b/src/mx_bluesky/beamlines/i04/thawing_plan.py @@ -57,9 +57,7 @@ def check_redis_connection(redis_host, redis_password, redis_db) -> bool: redis_client.ping() return True except ConnectionError: - LOGGER.warning( - f"Failed to connect to redis with \nhost: {redis_host}, password: {redis_password}, db: {redis_db}" - ) + LOGGER.warning(f"Failed to connect to redis host: {redis_host}") return False From dfd1cb95cbba878e8075bcad69752a7009c0ad67 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Mon, 17 Nov 2025 16:35:27 +0000 Subject: [PATCH 04/14] Add test --- .../unit_tests/beamlines/i04/test_thawing.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/unit_tests/beamlines/i04/test_thawing.py b/tests/unit_tests/beamlines/i04/test_thawing.py index d452def86..3d1c874ec 100644 --- a/tests/unit_tests/beamlines/i04/test_thawing.py +++ b/tests/unit_tests/beamlines/i04/test_thawing.py @@ -622,9 +622,13 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( assert publish_call_args_list[3].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) +@patch( + "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" +) @patch("mx_bluesky.beamlines.i04.thawing_plan.thaw") def test_plans_do_thaw_if_redis_connection_check_fails( patch_thaw: MagicMock, + patch_rotate_in_one_direction_and_stream_to_redis: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, @@ -648,6 +652,7 @@ def test_plans_do_thaw_if_redis_connection_check_fails( patch_thaw.assert_called_once_with( time_to_thaw=10, rotation=360, thawer=thawer, smargon=smargon ) + patch_rotate_in_one_direction_and_stream_to_redis.assert_not_called() assert any( record.message.startswith("Failed to connect to redis") and record.levelname == "WARNING" @@ -655,3 +660,50 @@ def test_plans_do_thaw_if_redis_connection_check_fails( ) caplog.clear() patch_thaw.reset_mock() + patch_rotate_in_one_direction_and_stream_to_redis.reset_mock() + + +@patch("mx_bluesky.beamlines.i04.thawing_plan.thaw") +@patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") +@patch( + "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" +) +@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") +def test_plans_continue_as_normal_if_redis_connection_check_passes( + patch_redis_connection: MagicMock, + patch_rotate_in_one_direction_and_stream_to_redis: MagicMock, + patch_murko_callback: MagicMock, + patch_thaw: MagicMock, + smargon: Smargon, + thawer: Thawer, + robot: BartRobot, + oav_forwarder: OAVToRedisForwarder, + run_engine: RunEngine, + murko_results: MurkoResultsDevice, +): + patch_redis_connection.return_value = True + for plan in ( + thaw_and_murko_centre( + 10, + 360, + thawer=thawer, + smargon=smargon, + robot=robot, + oav_to_redis_forwarder=oav_forwarder, + murko_results=murko_results, + ), + thaw_and_stream_to_redis( + 10, + 360, + thawer=thawer, + smargon=smargon, + robot=robot, + oav_to_redis_forwarder=oav_forwarder, + ), + ): + run_engine(plan) + + patch_thaw.assert_not_called() + assert patch_rotate_in_one_direction_and_stream_to_redis.call_count == 2 + patch_thaw.reset_mock() + patch_rotate_in_one_direction_and_stream_to_redis.reset_mock() From 37fda8c3a24f445616f24370ee427bbde42e11fe Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 2 Dec 2025 10:19:00 +0000 Subject: [PATCH 05/14] Move connection check into murko_callback --- .../beamlines/i04/callbacks/murko_callback.py | 24 +++++++++---- src/mx_bluesky/beamlines/i04/thawing_plan.py | 34 ------------------- 2 files changed, 17 insertions(+), 41 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py index ade2bb9b8..d308a3b9a 100644 --- a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py +++ b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py @@ -4,6 +4,7 @@ from typing import TypedDict from bluesky.callbacks import CallbackBase +from dodal.devices.i04.murko_results import MurkoResultsDevice from dodal.log import LOGGER from event_model.documents import Event, RunStart, RunStop from redis import StrictRedis @@ -56,6 +57,13 @@ def __init__(self, redis_host: str, redis_password: str, redis_db: int = 0): ) self.last_uuid = None self.previous_omegas: list[OmegaReading] = [] + self.redis_connected = MurkoResultsDevice.check_redis_connection( + self.redis_client + ) + if not self.redis_connected: + LOGGER.warning( + f"Failed to connect to redis: {self.redis_client}. Murko callback will not run" + ) def start(self, doc: RunStart) -> RunStart | None: self.murko_metadata: dict = {"sample_id": doc.get("sample_id")} @@ -109,17 +117,19 @@ def call_murko(self, uuid: str, omega: float): # Send metadata to REDIS and trigger murko redis_key = f"murko:{metadata['sample_id']}:metadata" - self.redis_client.hset(redis_key, uuid, json.dumps(metadata)) - self.redis_client.expire(redis_key, timedelta(days=self.DATA_EXPIRY_DAYS)) - self.redis_client.publish("murko", json.dumps(metadata)) + if self.redis_connected: + self.redis_client.hset(redis_key, uuid, json.dumps(metadata)) + self.redis_client.expire(redis_key, timedelta(days=self.DATA_EXPIRY_DAYS)) + self.redis_client.publish("murko", json.dumps(metadata)) def stop(self, doc: RunStop) -> RunStop | None: LOGGER.info(f"Finished streaming {self.murko_metadata['sample_id']} to murko") LOGGER.info( f"Publishing forwarding complete message: {FORWARDING_COMPLETE_MESSAGE}" ) - self.redis_client.publish( - "murko", - json.dumps(FORWARDING_COMPLETE_MESSAGE), - ) + if self.redis_connected: + self.redis_client.publish( + "murko", + json.dumps(FORWARDING_COMPLETE_MESSAGE), + ) return doc diff --git a/src/mx_bluesky/beamlines/i04/thawing_plan.py b/src/mx_bluesky/beamlines/i04/thawing_plan.py index 60eeff64c..3a3cd6bc2 100644 --- a/src/mx_bluesky/beamlines/i04/thawing_plan.py +++ b/src/mx_bluesky/beamlines/i04/thawing_plan.py @@ -10,8 +10,6 @@ from dodal.devices.smargon import Smargon from dodal.devices.thawer import OnOff, Thawer from dodal.log import LOGGER -from redis import StrictRedis -from redis.exceptions import ConnectionError from mx_bluesky.beamlines.i04.callbacks.murko_callback import MurkoCallback @@ -51,16 +49,6 @@ def cleanup(): ) -def check_redis_connection(redis_host, redis_password, redis_db) -> bool: - redis_client = StrictRedis(host=redis_host, password=redis_password, db=redis_db) - try: - redis_client.ping() - return True - except ConnectionError: - LOGGER.warning(f"Failed to connect to redis host: {redis_host}") - return False - - def thaw_and_murko_centre( time_to_thaw: float, rotation: float = 360, @@ -90,17 +78,6 @@ def thaw_and_murko_centre( ... devices: These are the specific ophyd-devices used for the plan, the defaults are always correct """ - - if not check_redis_connection( - RedisConstants.REDIS_HOST, - RedisConstants.REDIS_PASSWORD, - RedisConstants.MURKO_REDIS_DB, - ): - yield from thaw( - time_to_thaw=time_to_thaw, rotation=rotation, thawer=thawer, smargon=smargon - ) - return - murko_results_group = "get_results" sample_id = yield from bps.rd(robot.sample_id) @@ -198,17 +175,6 @@ def thaw_and_stream_to_redis( ... devices: These are the specific ophyd-devices used for the plan, the defaults are always correct """ - - if not check_redis_connection( - RedisConstants.REDIS_HOST, - RedisConstants.REDIS_PASSWORD, - RedisConstants.MURKO_REDIS_DB, - ): - yield from thaw( - time_to_thaw=time_to_thaw, rotation=rotation, thawer=thawer, smargon=smargon - ) - return - sample_id = yield from bps.rd(robot.sample_id) sample_id = int(sample_id) From 15d5a5c50d8f43123fdec9e419e209e8fdbc98a6 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 2 Dec 2025 14:29:12 +0000 Subject: [PATCH 06/14] Fix --- .../beamlines/i04/callbacks/murko_callback.py | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py index d308a3b9a..094bced68 100644 --- a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py +++ b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py @@ -4,10 +4,10 @@ from typing import TypedDict from bluesky.callbacks import CallbackBase -from dodal.devices.i04.murko_results import MurkoResultsDevice from dodal.log import LOGGER from event_model.documents import Event, RunStart, RunStop from redis import StrictRedis +from redis.exceptions import ConnectionError FORWARDING_COMPLETE_MESSAGE = "image_forwarding_complete" @@ -57,15 +57,22 @@ def __init__(self, redis_host: str, redis_password: str, redis_db: int = 0): ) self.last_uuid = None self.previous_omegas: list[OmegaReading] = [] - self.redis_connected = MurkoResultsDevice.check_redis_connection( - self.redis_client - ) + self.redis_connected = self._check_redis_connection() if not self.redis_connected: LOGGER.warning( f"Failed to connect to redis: {self.redis_client}. Murko callback will not run" ) + def _check_redis_connection(self): + try: + self.redis_client.ping() + return True + except ConnectionError: + return False + def start(self, doc: RunStart) -> RunStart | None: + if not self.redis_connected: + return doc self.murko_metadata: dict = {"sample_id": doc.get("sample_id")} self.last_uuid = None self.previous_omegas = [] @@ -75,6 +82,8 @@ def start(self, doc: RunStart) -> RunStart | None: return doc def event(self, doc: Event) -> Event: + if not self.redis_connected: + return doc data = doc["data"] for prefix in ("oav", "oav_full_screen"): if f"{prefix}-beam_centre_j" in data: @@ -117,19 +126,19 @@ def call_murko(self, uuid: str, omega: float): # Send metadata to REDIS and trigger murko redis_key = f"murko:{metadata['sample_id']}:metadata" - if self.redis_connected: - self.redis_client.hset(redis_key, uuid, json.dumps(metadata)) - self.redis_client.expire(redis_key, timedelta(days=self.DATA_EXPIRY_DAYS)) - self.redis_client.publish("murko", json.dumps(metadata)) + self.redis_client.hset(redis_key, uuid, json.dumps(metadata)) + self.redis_client.expire(redis_key, timedelta(days=self.DATA_EXPIRY_DAYS)) + self.redis_client.publish("murko", json.dumps(metadata)) def stop(self, doc: RunStop) -> RunStop | None: + if not self.redis_connected: + return doc LOGGER.info(f"Finished streaming {self.murko_metadata['sample_id']} to murko") LOGGER.info( f"Publishing forwarding complete message: {FORWARDING_COMPLETE_MESSAGE}" ) - if self.redis_connected: - self.redis_client.publish( - "murko", - json.dumps(FORWARDING_COMPLETE_MESSAGE), - ) + self.redis_client.publish( + "murko", + json.dumps(FORWARDING_COMPLETE_MESSAGE), + ) return doc From 641bbbc25551584865381ae16d45168a8b043341 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 2 Dec 2025 14:29:28 +0000 Subject: [PATCH 07/14] Fix and add tests --- .../i04/callbacks/test_murko_callback.py | 16 +++- tests/unit_tests/beamlines/i04/conftest.py | 1 + .../unit_tests/beamlines/i04/test_thawing.py | 95 ++----------------- 3 files changed, 24 insertions(+), 88 deletions(-) diff --git a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py index 71d71f715..f9641eff9 100644 --- a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py +++ b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py @@ -1,5 +1,5 @@ import json -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest from event_model import Event @@ -275,3 +275,17 @@ def test_when_murko_called_with_full_screen_and_roi_event_then_metadata_updates_ ) def test_extrapolate_omega(latest_omega, previous_omega, now, expected): assert extrapolate_omega(latest_omega, previous_omega, now) == expected + + +@patch( + "mx_bluesky.beamlines.i04.callbacks.murko_callback.MurkoCallback._check_redis_connection" +) +def test_if_redis_connection_fails_then_there_is_no_error( + mock_check_redis_connection: MagicMock, +): + mock_check_redis_connection.return_value = False + callback = MurkoCallback("", "") + doc = {} + callback.start(doc) + callback.event(doc) + callback.stop(doc) diff --git a/tests/unit_tests/beamlines/i04/conftest.py b/tests/unit_tests/beamlines/i04/conftest.py index 51b4ec363..88bd66d84 100644 --- a/tests/unit_tests/beamlines/i04/conftest.py +++ b/tests/unit_tests/beamlines/i04/conftest.py @@ -9,4 +9,5 @@ def murko_callback() -> MurkoCallback: callback = MurkoCallback("", "") callback.redis_client = MagicMock() + callback.redis_connected = True return callback diff --git a/tests/unit_tests/beamlines/i04/test_thawing.py b/tests/unit_tests/beamlines/i04/test_thawing.py index 3d1c874ec..435a0cac0 100644 --- a/tests/unit_tests/beamlines/i04/test_thawing.py +++ b/tests/unit_tests/beamlines/i04/test_thawing.py @@ -1,5 +1,4 @@ import json -import logging from functools import partial from unittest.mock import ANY, AsyncMock, MagicMock, call, patch @@ -203,18 +202,15 @@ def test_given_different_rotations_then_motor_moved_relative( ] -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") async def test_thaw_and_stream_sets_sample_id_and_kicks_off_forwarder( patch_murko_callback: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, robot: BartRobot, run_engine: RunEngine, ): - patch_check_redis_connection.return_value = True set_mock_value(robot.sample_id, 100) run_engine( thaw_and_stream_to_redis( @@ -231,18 +227,15 @@ async def test_thaw_and_stream_sets_sample_id_and_kicks_off_forwarder( oav_forwarder.complete.assert_called() # type: ignore -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_thaw_and_stream_adds_murko_callback_and_produces_expected_messages( patch_murko_callback: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, oav_forwarder: OAVToRedisForwarder, run_engine: RunEngine, ): - patch_check_redis_connection.return_value = True patch_murko_instance = patch_murko_callback.return_value run_engine( thaw_and_stream_to_redis( @@ -268,13 +261,13 @@ def test_thaw_and_stream_adds_murko_callback_and_produces_expected_messages( assert len(smargon_updates) > 0 -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback.stop") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback.call_murko") +@patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback._check_redis_connection") def test_thaw_and_stream_will_produce_events_that_call_murko( + patch_check_redis_connection: MagicMock, patch_murko_call: MagicMock, patch_stop_call: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, @@ -330,16 +323,13 @@ def _test_plan_will_switch_murko_source_half_way_through_thaw( ) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thaw_and_stream_will_switch_murko_source_half_way_through_thaw( - patch_check_redis_connection: MagicMock, sim_run_engine, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, robot: BartRobot, ): - patch_check_redis_connection.return_value = True _test_plan_will_switch_murko_source_half_way_through_thaw( sim_run_engine, thaw_and_stream_to_redis(10, 360, robot, thawer, smargon, oav_forwarder), @@ -379,11 +369,9 @@ def _run_thaw_and_stream_and_assert_zoom_changes( mock_level_set.assert_has_calls([call("1.0x", wait=True), call("2.0x", wait=True)]) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_given_thaw_succeeds_then_thaw_and_stream_sets_zoom_to_1_and_back( patch_murko_callback: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -391,19 +379,16 @@ def test_given_thaw_succeeds_then_thaw_and_stream_sets_zoom_to_1_and_back( robot: BartRobot, run_engine: RunEngine, ): - patch_check_redis_connection.return_value = True _run_thaw_and_stream_and_assert_zoom_changes( smargon, thawer, oav_forwarder, oav_full_screen, robot, run_engine ) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") @patch("mx_bluesky.beamlines.i04.thawing_plan.bps.monitor") def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( mock__thaw: MagicMock, patch_murko_callback: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -411,14 +396,12 @@ def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( robot: BartRobot, run_engine: RunEngine, ): - patch_check_redis_connection.return_value = True mock__thaw.side_effect = Exception() _run_thaw_and_stream_and_assert_zoom_changes( smargon, thawer, oav_forwarder, oav_full_screen, robot, run_engine, Exception ) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") @patch( "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" @@ -426,7 +409,6 @@ def test_given_thaw_fails_then_thaw_and_stream_sets_zoom_to_1_and_back( def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( mock_rotate_and_stream: MagicMock, patch_murko_callback: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -443,13 +425,11 @@ def test_thaw_and_murko_centre_stages_and_unstages_murko_results_twice( assert murko_results.unstage.call_count == 2 # type: ignore -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") @patch("mx_bluesky.beamlines.i04.thawing_plan.bps.monitor") def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( mock__thaw: MagicMock, patch_murko_callback: MagicMock, - patch_check_redis_connection: MagicMock, smargon: Smargon, thawer: Thawer, oav_forwarder: OAVToRedisForwarder, @@ -470,9 +450,7 @@ def test_given_thaw_and_murko_centre_errors_then_murko_results_still_unstaged( murko_results.unstage.assert_called_once() # type: ignore -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( - patch_check_redis_connection: MagicMock, sim_run_engine, smargon: Smargon, thawer: Thawer, @@ -480,7 +458,6 @@ def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( robot: BartRobot, murko_results: MurkoResultsDevice, ): - patch_check_redis_connection.return_value = True _test_plan_will_switch_murko_source_half_way_through_thaw( sim_run_engine, thaw_and_murko_centre( @@ -489,11 +466,9 @@ def test_thaw_and_murko_centre_will_switch_murko_source_half_way_through_thaw( ) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") def test_thaw_and_murko_centre_will_centre_based_on_murko_results_after_both_rotations( patch_murko_callback, - patch_check_redis_connection: MagicMock, run_engine, smargon: Smargon, thawer: Thawer, @@ -501,8 +476,6 @@ def test_thaw_and_murko_centre_will_centre_based_on_murko_results_after_both_rot robot: BartRobot, murko_results: MurkoResultsDevice, ): - patch_check_redis_connection.return_value = True - def fake_trigger(call_count): if call_count == 1: murko_results._x_mm_setter(1) @@ -538,9 +511,7 @@ def side_effect(): get_mock_put(smargon.z.user_setpoint).assert_has_calls([call(9.0, wait=True)]) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( - patch_check_redis_connection: MagicMock, sim_run_engine, smargon: Smargon, thawer: Thawer, @@ -548,7 +519,6 @@ def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( robot: BartRobot, murko_results: MurkoResultsDevice, ): - patch_check_redis_connection.return_value = True sim_run_engine.add_read_handler_for(robot.sample_id, "1234") msgs = sim_run_engine.simulate_plan( @@ -568,9 +538,7 @@ def test_thaw_and_murko_centre_will_set_sample_id_before_triggering_results( ) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( - patch_check_redis_connection: MagicMock, run_engine, smargon: Smargon, thawer: Thawer, @@ -579,7 +547,6 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( murko_results: MurkoResultsDevice, murko_callback: MurkoCallback, ): - patch_check_redis_connection.return_value = True with patch( "mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback" ) as mock_murko_callback: @@ -625,63 +592,19 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( @patch( "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" ) -@patch("mx_bluesky.beamlines.i04.thawing_plan.thaw") -def test_plans_do_thaw_if_redis_connection_check_fails( - patch_thaw: MagicMock, +@patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback._check_redis_connection") +def test_plans_carry_on_thaw_if_redis_connection_check_fails( + patch_callback_check_redis_connection: MagicMock, patch_rotate_in_one_direction_and_stream_to_redis: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, oav_forwarder: OAVToRedisForwarder, run_engine: RunEngine, - caplog: pytest.LogCaptureFixture, -): - for plan in (thaw_and_murko_centre, thaw_and_stream_to_redis): - with caplog.at_level(logging.WARNING): - run_engine( - plan( - 10, - 360, - thawer=thawer, - smargon=smargon, - robot=robot, - oav_to_redis_forwarder=oav_forwarder, - ) - ) - - patch_thaw.assert_called_once_with( - time_to_thaw=10, rotation=360, thawer=thawer, smargon=smargon - ) - patch_rotate_in_one_direction_and_stream_to_redis.assert_not_called() - assert any( - record.message.startswith("Failed to connect to redis") - and record.levelname == "WARNING" - for record in caplog.records - ) - caplog.clear() - patch_thaw.reset_mock() - patch_rotate_in_one_direction_and_stream_to_redis.reset_mock() - - -@patch("mx_bluesky.beamlines.i04.thawing_plan.thaw") -@patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback") -@patch( - "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" -) -@patch("mx_bluesky.beamlines.i04.thawing_plan.check_redis_connection") -def test_plans_continue_as_normal_if_redis_connection_check_passes( - patch_redis_connection: MagicMock, - patch_rotate_in_one_direction_and_stream_to_redis: MagicMock, - patch_murko_callback: MagicMock, - patch_thaw: MagicMock, - smargon: Smargon, - thawer: Thawer, - robot: BartRobot, - oav_forwarder: OAVToRedisForwarder, - run_engine: RunEngine, - murko_results: MurkoResultsDevice, ): - patch_redis_connection.return_value = True + patch_callback_check_redis_connection.return_value = False + murko_results = MurkoResultsDevice() + murko_results._check_redis_connection = AsyncMock(return_value=False) for plan in ( thaw_and_murko_centre( 10, @@ -703,7 +626,5 @@ def test_plans_continue_as_normal_if_redis_connection_check_passes( ): run_engine(plan) - patch_thaw.assert_not_called() assert patch_rotate_in_one_direction_and_stream_to_redis.call_count == 2 - patch_thaw.reset_mock() patch_rotate_in_one_direction_and_stream_to_redis.reset_mock() From ea92171e36d50c109a5185756d02d55360f753bd Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 2 Dec 2025 14:44:36 +0000 Subject: [PATCH 08/14] Move warning into function --- src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py index 094bced68..deb1f2808 100644 --- a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py +++ b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py @@ -58,16 +58,15 @@ def __init__(self, redis_host: str, redis_password: str, redis_db: int = 0): self.last_uuid = None self.previous_omegas: list[OmegaReading] = [] self.redis_connected = self._check_redis_connection() - if not self.redis_connected: - LOGGER.warning( - f"Failed to connect to redis: {self.redis_client}. Murko callback will not run" - ) def _check_redis_connection(self): try: self.redis_client.ping() return True except ConnectionError: + LOGGER.warning( + f"Failed to connect to redis: {self.redis_client}. Murko callback will not run" + ) return False def start(self, doc: RunStart) -> RunStart | None: From ffc19e8667d07b5fcc8de0bd07ff1245cd7939b4 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 2 Dec 2025 15:06:44 +0000 Subject: [PATCH 09/14] Pin dodal --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f22c5df38..99e4b5c23 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,7 @@ dependencies = [ "ophyd >= 1.10.5", "ophyd-async >= 0.14.0", "bluesky >= 1.14.6", - "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main", + "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@mx_bluesky_810_continue_thaw_if_redis_is_down", ] From 82f35a3040bb2970837026d348e8e3366d0913ac Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 19 Dec 2025 14:31:24 +0000 Subject: [PATCH 10/14] Check connection in start rather than callback init --- src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py index deb1f2808..782a43e27 100644 --- a/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py +++ b/src/mx_bluesky/beamlines/i04/callbacks/murko_callback.py @@ -57,7 +57,6 @@ def __init__(self, redis_host: str, redis_password: str, redis_db: int = 0): ) self.last_uuid = None self.previous_omegas: list[OmegaReading] = [] - self.redis_connected = self._check_redis_connection() def _check_redis_connection(self): try: @@ -70,6 +69,7 @@ def _check_redis_connection(self): return False def start(self, doc: RunStart) -> RunStart | None: + self.redis_connected = self._check_redis_connection() if not self.redis_connected: return doc self.murko_metadata: dict = {"sample_id": doc.get("sample_id")} From 6bb52f147892207e7c38b228170f3ca26e049857 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 19 Dec 2025 14:41:38 +0000 Subject: [PATCH 11/14] Improve test --- tests/unit_tests/beamlines/i04/test_thawing.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/unit_tests/beamlines/i04/test_thawing.py b/tests/unit_tests/beamlines/i04/test_thawing.py index 435a0cac0..2ce510b13 100644 --- a/tests/unit_tests/beamlines/i04/test_thawing.py +++ b/tests/unit_tests/beamlines/i04/test_thawing.py @@ -589,13 +589,9 @@ def test_thawing_plan_with_murko_callback_puts_correct_metadata_into_redis( assert publish_call_args_list[3].args[1] == json.dumps(FORWARDING_COMPLETE_MESSAGE) -@patch( - "mx_bluesky.beamlines.i04.thawing_plan._rotate_in_one_direction_and_stream_to_redis" -) @patch("mx_bluesky.beamlines.i04.thawing_plan.MurkoCallback._check_redis_connection") def test_plans_carry_on_thaw_if_redis_connection_check_fails( patch_callback_check_redis_connection: MagicMock, - patch_rotate_in_one_direction_and_stream_to_redis: MagicMock, smargon: Smargon, thawer: Thawer, robot: BartRobot, @@ -626,5 +622,11 @@ def test_plans_carry_on_thaw_if_redis_connection_check_fails( ): run_engine(plan) - assert patch_rotate_in_one_direction_and_stream_to_redis.call_count == 2 - patch_rotate_in_one_direction_and_stream_to_redis.reset_mock() + omega_put = get_mock_put(smargon.omega.user_setpoint) + + assert omega_put.call_args_list == [ + call(360.0, wait=True), + call(0.0, wait=True), + ] + + omega_put.reset_mock() From f75054ea5f4663781336cb90b3fb6fe1fabb23b0 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 19 Dec 2025 15:02:26 +0000 Subject: [PATCH 12/14] Add test --- .../beamlines/i04/callbacks/test_murko_callback.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py index f9641eff9..8bff7589b 100644 --- a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py +++ b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py @@ -289,3 +289,12 @@ def test_if_redis_connection_fails_then_there_is_no_error( callback.start(doc) callback.event(doc) callback.stop(doc) + + +def test_rwarning_is_logged_if_redis_connection_fails(caplog): + callback = MurkoCallback("", "") + doc = {} + callback.start(doc) + log_message = caplog.records[-1] + assert log_message.levelname == "WARNING" + assert "Failed to connect to redis: " in log_message.message From 64261f8c94997a3ee3e4cd2d43ddf2599bcb630d Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 19 Dec 2025 15:33:34 +0000 Subject: [PATCH 13/14] Update dls-dodal dependency to use main branch --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 99e4b5c23..f22c5df38 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,7 @@ dependencies = [ "ophyd >= 1.10.5", "ophyd-async >= 0.14.0", "bluesky >= 1.14.6", - "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@mx_bluesky_810_continue_thaw_if_redis_is_down", + "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main", ] From 23cad0901fa27d5a83d686841183595bb8c6d7ab Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Mon, 5 Jan 2026 09:21:14 +0000 Subject: [PATCH 14/14] Fix typo --- tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py index 8bff7589b..717cf649d 100644 --- a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py +++ b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py @@ -291,7 +291,7 @@ def test_if_redis_connection_fails_then_there_is_no_error( callback.stop(doc) -def test_rwarning_is_logged_if_redis_connection_fails(caplog): +def test_warning_is_logged_if_redis_connection_fails(caplog): callback = MurkoCallback("", "") doc = {} callback.start(doc)