From 1d8d3acb7802988724cd2fed0e8cac7b132dc14b Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 13 Feb 2026 09:19:24 +0000 Subject: [PATCH 1/5] WIP --- run_hyperion.sh | 10 +++++ src/mx_bluesky/common/utils/log.py | 44 +++++++++++++------ .../hyperion/blueapi_plans/__init__.py | 10 +++++ .../hyperion/blueapi_plans/plans.py | 0 tests/conftest.py | 6 +++ 5 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 src/mx_bluesky/hyperion/blueapi_plans/plans.py diff --git a/run_hyperion.sh b/run_hyperion.sh index 8d2f930ece..e930421bad 100755 --- a/run_hyperion.sh +++ b/run_hyperion.sh @@ -141,6 +141,16 @@ if [[ $START == 1 ]]; then echo "$(date) Logging to $LOG_DIR" export LOG_DIR mkdir -p "$LOG_DIR" + if [ -z "$DEBUG_LOG_DIR" ]; then + if [ $IN_DEV = true ]; then + DEBUG_LOG_DIR=$LOG_DIR + else + DEBUG_LOG_DIR=/dls/tmp/$BEAMLINE/logs/bluesky + fi + fi + echo "Debug log file set to $DEBUG_LOG_DIR" + export DEBUG_LOG_DIR + mkdir -p "$DEBUG_LOG_DIR" if [ $MODE = "supervisor" ]; then start_log_path=$LOG_DIR/supervisor_start_log.log else diff --git a/src/mx_bluesky/common/utils/log.py b/src/mx_bluesky/common/utils/log.py index 4df619304f..74c0cd4c99 100644 --- a/src/mx_bluesky/common/utils/log.py +++ b/src/mx_bluesky/common/utils/log.py @@ -10,6 +10,7 @@ DodalLogHandlers, integrate_bluesky_and_ophyd_logging, set_up_all_logging_handlers, + set_up_debug_memory_handler, ) from dodal.log import LOGGER as DODAL_LOGGER @@ -66,6 +67,18 @@ def set_uid_tag(uid): tag_filter.run_uid = uid +def setup_hyperion_blueapi_logging(log_file_name: str): + """Configure debug logging for hyperion-blueapi. + Args: + log_file_name: Base name of the log file. + """ + root_logger = logging.getLogger() + logging_path, debug_logging_path = _get_logging_dirs(False) + set_up_debug_memory_handler( + root_logger, debug_logging_path, log_file_name, ERROR_LOG_BUFFER_LINES + ) + + def do_default_logging_setup( file_name: str, graylog_port: int, @@ -115,26 +128,31 @@ def flush_debug_handler() -> str: def _get_logging_dirs(dev_mode: bool) -> tuple[Path, Path]: """Get the paths to write the mx_bluesky log files to. - Log location can be specified in the LOG_DIR environment variable, otherwise MX bluesky logs are written to 'dls_sw/ixx/logs/bluesky'. - This directory will be created if it is not found + Log location must be specified in the LOG_DIR environment variable, + and the debug log location specified in the DEBUG_LOG_DIR environment variable. + This directory will be created if it is not found. Logs are written to ./tmp/logs/bluesky if BEAMLINE environment variable is not found + Args: + dev_mode (bool): If True, the logs will be written to /tmp/logs/bluesky and environment variables ignored Returns: tuple[Path, Path]: Paths to the standard log file and to the debug log file, for the file handlers to write to + Raises: + ValueError: If LOG_DIR or DEBUG_LOG_DIR environment variable is not set and dev_mode is False """ - - beamline = environ.get("BEAMLINE") - - if beamline and not dev_mode: - default_logging_str = f"/dls_sw/{beamline}/logs/bluesky/" - default_debug_logging_str = f"/dls/tmp/{beamline}/logs/bluesky/" + if dev_mode: + logging_path = Path(environ.get("LOG_DIR", "/tmp/logs/bluesky")) + debug_logging_path = Path(environ.get("DEBUG_LOG_DIR", "/tmp/logs/bluesky")) else: - default_logging_str = "/tmp/logs/bluesky" - default_debug_logging_str = default_logging_str - - logging_path = Path(environ.get("LOG_DIR", default_logging_str)) - debug_logging_path = Path(environ.get("DEBUG_LOG_DIR", default_debug_logging_str)) + logging_dir = environ.get("LOG_DIR") + if not logging_dir: + raise ValueError("LOG_DIR environment variable is not set") + debug_logging_dir = environ.get("DEBUG_LOG_DIR") + if not debug_logging_dir: + raise ValueError("DEBUG_LOG_DIR environment variable is not set") + logging_path = Path(logging_dir) + debug_logging_path = Path(debug_logging_dir) Path.mkdir(logging_path, exist_ok=True, parents=True) Path.mkdir(debug_logging_path, exist_ok=True, parents=True) diff --git a/src/mx_bluesky/hyperion/blueapi_plans/__init__.py b/src/mx_bluesky/hyperion/blueapi_plans/__init__.py index 7bb0ee1372..3b144c1202 100644 --- a/src/mx_bluesky/hyperion/blueapi_plans/__init__.py +++ b/src/mx_bluesky/hyperion/blueapi_plans/__init__.py @@ -17,6 +17,7 @@ from mx_bluesky.common.device_setup_plans.robot_load_unload import ( robot_unload as _robot_unload, ) +from mx_bluesky.common.utils.log import setup_hyperion_blueapi_logging from mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan import ( LoadCentreCollectComposite, ) @@ -29,6 +30,7 @@ from mx_bluesky.hyperion.experiment_plans.udc_default_state import ( move_to_udc_default_state as _move_to_udc_default_state, ) +from mx_bluesky.hyperion.parameters.constants import CONST from mx_bluesky.hyperion.parameters.load_centre_collect import LoadCentreCollect __all__ = [ @@ -42,6 +44,14 @@ ] +def _init_plan_module(): + """Initialisation hooks for hyperion-blueapi""" + setup_hyperion_blueapi_logging(CONST.LOG_FILE_NAME) + + +_init_plan_module() + + def load_centre_collect( parameters: LoadCentreCollect, composite: LoadCentreCollectComposite = inject() ) -> MsgGenerator: diff --git a/src/mx_bluesky/hyperion/blueapi_plans/plans.py b/src/mx_bluesky/hyperion/blueapi_plans/plans.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/conftest.py b/tests/conftest.py index 2abbd2fb4f..e02be0b796 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1745,3 +1745,9 @@ def mock_alert_service(): create=True, ) as service: yield service + + +@pytest.fixture(scope="session", autouse=True) +def override_hyperion_blueapi_logging_setup(): + with patch("mx_bluesky.hyperion.blueapi_plans.__init__._init_plan_module"): + yield From 2ca6b5619af5bc35d28c6097ff40865cf4276a3c Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 13 Feb 2026 11:58:18 +0000 Subject: [PATCH 2/5] Changes to implement debug logging for hyperion-blueapi --- conftest.py | 3 + src/mx_bluesky/common/utils/log.py | 2 +- src/mx_bluesky/hyperion/blueapi_config.yaml | 2 +- .../hyperion/blueapi_dev_config.yaml | 2 +- .../hyperion/blueapi_plans/__init__.py | 106 ------------------ .../hyperion/blueapi_plans/blueapi.py | 33 ++++++ .../hyperion/blueapi_plans/in_process.py | 100 +++++++++++++++++ .../hyperion/blueapi_plans/plans.py | 0 src/mx_bluesky/hyperion/in_process_runner.py | 5 +- tests/conftest.py | 6 - tests/unit_tests/common/utils/test_log.py | 51 ++++----- tests/unit_tests/hyperion/conftest.py | 10 ++ .../unit_tests/hyperion/test_baton_handler.py | 10 +- .../unit_tests/hyperion/test_blueapi_plans.py | 4 +- 14 files changed, 183 insertions(+), 151 deletions(-) create mode 100644 src/mx_bluesky/hyperion/blueapi_plans/blueapi.py create mode 100644 src/mx_bluesky/hyperion/blueapi_plans/in_process.py delete mode 100644 src/mx_bluesky/hyperion/blueapi_plans/plans.py diff --git a/conftest.py b/conftest.py index 5a0b1146da..2d3c2534c5 100644 --- a/conftest.py +++ b/conftest.py @@ -4,6 +4,9 @@ import pytest +# Ensure that the blueapi entry point is not invoked by doctest as this will fail +collect_ignore = ["src/mx_bluesky/hyperion/blueapi_plans/blueapi.py"] + environ["HYPERION_TEST_MODE"] = "true" diff --git a/src/mx_bluesky/common/utils/log.py b/src/mx_bluesky/common/utils/log.py index 74c0cd4c99..8a23fa2947 100644 --- a/src/mx_bluesky/common/utils/log.py +++ b/src/mx_bluesky/common/utils/log.py @@ -135,7 +135,7 @@ def _get_logging_dirs(dev_mode: bool) -> tuple[Path, Path]: Logs are written to ./tmp/logs/bluesky if BEAMLINE environment variable is not found Args: - dev_mode (bool): If True, the logs will be written to /tmp/logs/bluesky and environment variables ignored + dev_mode (bool): If True, the logs will be written to /tmp/logs/bluesky if the environment variables are not set Returns: tuple[Path, Path]: Paths to the standard log file and to the debug log file, for the file handlers to write to Raises: diff --git a/src/mx_bluesky/hyperion/blueapi_config.yaml b/src/mx_bluesky/hyperion/blueapi_config.yaml index 67c8f43dd1..9b1864a786 100644 --- a/src/mx_bluesky/hyperion/blueapi_config.yaml +++ b/src/mx_bluesky/hyperion/blueapi_config.yaml @@ -3,7 +3,7 @@ env: - kind: deviceManager module: dodal.beamlines.i03 - kind: planFunctions - module: mx_bluesky.hyperion.blueapi_plans + module: mx_bluesky.hyperion.blueapi_plans.blueapi events: broadcast_status_events: false api: diff --git a/src/mx_bluesky/hyperion/blueapi_dev_config.yaml b/src/mx_bluesky/hyperion/blueapi_dev_config.yaml index f02f833073..581a454dc5 100644 --- a/src/mx_bluesky/hyperion/blueapi_dev_config.yaml +++ b/src/mx_bluesky/hyperion/blueapi_dev_config.yaml @@ -4,7 +4,7 @@ env: module: dodal.beamlines.i03 mock: true - kind: planFunctions - module: mx_bluesky.hyperion.blueapi_plans + module: mx_bluesky.hyperion.blueapi_plans.blueapi events: broadcast_status_events: false api: diff --git a/src/mx_bluesky/hyperion/blueapi_plans/__init__.py b/src/mx_bluesky/hyperion/blueapi_plans/__init__.py index 3b144c1202..e69de29bb2 100644 --- a/src/mx_bluesky/hyperion/blueapi_plans/__init__.py +++ b/src/mx_bluesky/hyperion/blueapi_plans/__init__.py @@ -1,106 +0,0 @@ -""" -This module contains the bluesky plan entry points for use with hyperion-blueapi. -The json schema and documentation therein generated by the blueapi /plans endpoint -from this file constitutes the hyperion-blueapi interface to the hyperion supervisor -process. -""" - -from bluesky import plan_stubs as bps -from bluesky.utils import MsgGenerator -from dodal.common import inject -from dodal.devices.aperturescatterguard import ApertureScatterguard -from dodal.devices.detector.detector_motion import DetectorMotion, ShutterState -from dodal.devices.motors import XYZStage -from dodal.devices.robot import BartRobot -from dodal.devices.smargon import Smargon - -from mx_bluesky.common.device_setup_plans.robot_load_unload import ( - robot_unload as _robot_unload, -) -from mx_bluesky.common.utils.log import setup_hyperion_blueapi_logging -from mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan import ( - LoadCentreCollectComposite, -) -from mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan import ( - load_centre_collect_full as _load_centre_collect_full, -) -from mx_bluesky.hyperion.experiment_plans.udc_default_state import ( - UDCDefaultDevices, -) -from mx_bluesky.hyperion.experiment_plans.udc_default_state import ( - move_to_udc_default_state as _move_to_udc_default_state, -) -from mx_bluesky.hyperion.parameters.constants import CONST -from mx_bluesky.hyperion.parameters.load_centre_collect import LoadCentreCollect - -__all__ = [ - "LoadCentreCollectComposite", - "LoadCentreCollect", - "UDCDefaultDevices", - "clean_up_udc", - "load_centre_collect", - "move_to_udc_default_state", - "robot_unload", -] - - -def _init_plan_module(): - """Initialisation hooks for hyperion-blueapi""" - setup_hyperion_blueapi_logging(CONST.LOG_FILE_NAME) - - -_init_plan_module() - - -def load_centre_collect( - parameters: LoadCentreCollect, composite: LoadCentreCollectComposite = inject() -) -> MsgGenerator: - """ - Attempt a complete data collection experiment, consisting of the following: - * Load the sample if necessary - * Move to the specified goniometer start angles - * Perform optical centring, then X-ray centring - * If X-ray centring finds one or more diffracting centres then for each centre - that satisfies the chosen selection function, - move to that centre and do a collection with the specified parameters. - """ - yield from _load_centre_collect_full(composite, parameters) - - -def robot_unload( - visit: str, - robot: BartRobot = inject("robot"), - smargon: Smargon = inject("gonio"), - aperture_scatterguard: ApertureScatterguard = inject("aperture_scatterguard"), - lower_gonio: XYZStage = inject("lower_gonio"), -) -> MsgGenerator: - """ - Unload the currently mounted pin into the location that it was loaded from. - This is to be invoked as the final step upon successful completion of the UDC queue. - """ - yield from _robot_unload(robot, smargon, aperture_scatterguard, lower_gonio, visit) - - -def clean_up_udc( - visit: str, - cleanup_group: str = "cleanup", - robot: BartRobot = inject("robot"), - smargon: Smargon = inject("gonio"), - aperture_scatterguard: ApertureScatterguard = inject("aperture_scatterguard"), - lower_gonio: XYZStage = inject("lower_gonio"), - detector_motion: DetectorMotion = inject("detector_motion"), -) -> MsgGenerator: - yield from bps.abs_set( - detector_motion.shutter, ShutterState.CLOSED, group=cleanup_group - ) - yield from _robot_unload(robot, smargon, aperture_scatterguard, lower_gonio, visit) - yield from bps.wait(cleanup_group) - - -def move_to_udc_default_state( - composite: UDCDefaultDevices = inject(), -) -> MsgGenerator: - """ - Move beamline hardware to known positions prior to UDC start. - """ - yield from _move_to_udc_default_state(composite) diff --git a/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py b/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py new file mode 100644 index 0000000000..7bc2c37e3d --- /dev/null +++ b/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py @@ -0,0 +1,33 @@ +""" +This module is the bluesky plan module for use with hyperion-blueapi. +""" + +from mx_bluesky.common.utils.log import setup_hyperion_blueapi_logging +from mx_bluesky.hyperion.blueapi_plans.in_process import ( + LoadCentreCollect, + LoadCentreCollectComposite, + UDCDefaultDevices, + clean_up_udc, + load_centre_collect, + move_to_udc_default_state, + robot_unload, +) +from mx_bluesky.hyperion.parameters.constants import CONST + +__all__ = [ + "LoadCentreCollectComposite", + "LoadCentreCollect", + "UDCDefaultDevices", + "clean_up_udc", + "load_centre_collect", + "move_to_udc_default_state", + "robot_unload", +] + + +def _init_plan_module(): + """Initialisation hooks for hyperion-blueapi""" + setup_hyperion_blueapi_logging(CONST.LOG_FILE_NAME) + + +_init_plan_module() diff --git a/src/mx_bluesky/hyperion/blueapi_plans/in_process.py b/src/mx_bluesky/hyperion/blueapi_plans/in_process.py new file mode 100644 index 0000000000..df77e55863 --- /dev/null +++ b/src/mx_bluesky/hyperion/blueapi_plans/in_process.py @@ -0,0 +1,100 @@ +""" +This module contains the bluesky plan entry points for use with hyperion. +hyperion-blueapi loads this module via blueapi.py which contains module-load initialisation +hooks that are run when blueapi loads the plan module, which are not needed by the +in-process runner and so not defined here. + +The json schema and documentation therein generated by the blueapi /plans endpoint +from this file constitutes the hyperion-blueapi interface to the hyperion supervisor +process. +""" + +from bluesky import plan_stubs as bps +from bluesky.utils import MsgGenerator +from dodal.common import inject +from dodal.devices.aperturescatterguard import ApertureScatterguard +from dodal.devices.detector.detector_motion import DetectorMotion, ShutterState +from dodal.devices.motors import XYZStage +from dodal.devices.robot import BartRobot +from dodal.devices.smargon import Smargon + +from mx_bluesky.common.device_setup_plans.robot_load_unload import ( + robot_unload as _robot_unload, +) +from mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan import ( + LoadCentreCollectComposite, +) +from mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan import ( + load_centre_collect_full as _load_centre_collect_full, +) +from mx_bluesky.hyperion.experiment_plans.udc_default_state import ( + UDCDefaultDevices, +) +from mx_bluesky.hyperion.experiment_plans.udc_default_state import ( + move_to_udc_default_state as _move_to_udc_default_state, +) +from mx_bluesky.hyperion.parameters.load_centre_collect import LoadCentreCollect + +__all__ = [ + "LoadCentreCollectComposite", + "LoadCentreCollect", + "UDCDefaultDevices", + "clean_up_udc", + "load_centre_collect", + "move_to_udc_default_state", + "robot_unload", +] + + +def load_centre_collect( + parameters: LoadCentreCollect, composite: LoadCentreCollectComposite = inject() +) -> MsgGenerator: + """ + Attempt a complete data collection experiment, consisting of the following: + * Load the sample if necessary + * Move to the specified goniometer start angles + * Perform optical centring, then X-ray centring + * If X-ray centring finds one or more diffracting centres then for each centre + that satisfies the chosen selection function, + move to that centre and do a collection with the specified parameters. + """ + yield from _load_centre_collect_full(composite, parameters) + + +def robot_unload( + visit: str, + robot: BartRobot = inject("robot"), + smargon: Smargon = inject("gonio"), + aperture_scatterguard: ApertureScatterguard = inject("aperture_scatterguard"), + lower_gonio: XYZStage = inject("lower_gonio"), +) -> MsgGenerator: + """ + Unload the currently mounted pin into the location that it was loaded from. + This is to be invoked as the final step upon successful completion of the UDC queue. + """ + yield from _robot_unload(robot, smargon, aperture_scatterguard, lower_gonio, visit) + + +def clean_up_udc( + visit: str, + cleanup_group: str = "cleanup", + robot: BartRobot = inject("robot"), + smargon: Smargon = inject("gonio"), + aperture_scatterguard: ApertureScatterguard = inject("aperture_scatterguard"), + lower_gonio: XYZStage = inject("lower_gonio"), + detector_motion: DetectorMotion = inject("detector_motion"), +) -> MsgGenerator: + yield from bps.abs_set( + detector_motion.shutter, ShutterState.CLOSED, group=cleanup_group + ) + yield from _robot_unload(robot, smargon, aperture_scatterguard, lower_gonio, visit) + yield from bps.wait(cleanup_group) + + +def move_to_udc_default_state( + composite: UDCDefaultDevices = inject(), +) -> MsgGenerator: + """ + Move beamline hardware to known positions prior to UDC start. + """ + yield from _move_to_udc_default_state(composite) diff --git a/src/mx_bluesky/hyperion/blueapi_plans/plans.py b/src/mx_bluesky/hyperion/blueapi_plans/plans.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/mx_bluesky/hyperion/in_process_runner.py b/src/mx_bluesky/hyperion/in_process_runner.py index 7f5264e9f5..ace141ae85 100644 --- a/src/mx_bluesky/hyperion/in_process_runner.py +++ b/src/mx_bluesky/hyperion/in_process_runner.py @@ -19,7 +19,10 @@ ) from mx_bluesky.common.utils.exceptions import WarningError from mx_bluesky.common.utils.log import LOGGER -from mx_bluesky.hyperion.blueapi_plans import clean_up_udc, move_to_udc_default_state +from mx_bluesky.hyperion.blueapi_plans.in_process import ( + clean_up_udc, + move_to_udc_default_state, +) from mx_bluesky.hyperion.experiment_plans import load_centre_collect_full from mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan import ( create_devices, diff --git a/tests/conftest.py b/tests/conftest.py index e02be0b796..2abbd2fb4f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1745,9 +1745,3 @@ def mock_alert_service(): create=True, ) as service: yield service - - -@pytest.fixture(scope="session", autouse=True) -def override_hyperion_blueapi_logging_setup(): - with patch("mx_bluesky.hyperion.blueapi_plans.__init__._init_plan_module"): - yield diff --git a/tests/unit_tests/common/utils/test_log.py b/tests/unit_tests/common/utils/test_log.py index 35a2744601..d7c98a8c6f 100644 --- a/tests/unit_tests/common/utils/test_log.py +++ b/tests/unit_tests/common/utils/test_log.py @@ -20,6 +20,12 @@ TEST_GRAYLOG_PORT = 5555 +@pytest.fixture(autouse=True) +def patch_log_dirs(monkeypatch, tmp_path): + monkeypatch.setenv("LOG_DIR", str(tmp_path)) + monkeypatch.setenv("DEBUG_LOG_DIR", str(tmp_path)) + + @pytest.fixture(scope="function") def clear_and_mock_loggers(): clear_log_handlers([*log.ALL_LOGGERS, DODAL_LOGGER]) @@ -39,7 +45,10 @@ def clear_and_mock_loggers(): @pytest.mark.skip_log_setup def test_no_env_variable_sets_correct_file_handler( clear_and_mock_loggers, + monkeypatch, ) -> None: + monkeypatch.delenv("LOG_DIR") + monkeypatch.delenv("DEBUG_LOG_DIR") log.do_default_logging_setup("hyperion.log", TEST_GRAYLOG_PORT, dev_mode=True) file_handlers: FileHandler = next( filter(lambda h: isinstance(h, FileHandler), DODAL_LOGGER.handlers) # type: ignore @@ -209,46 +218,32 @@ def test_get_logging_dir_uses_env_var(mock_mkdir: MagicMock, dev_mode: bool): assert mock_mkdir.call_count == 2 -@pytest.mark.parametrize( - "dev_mode, expected_log_dir, expected_debug_log_dir", - [ - [True, "/tmp/logs/bluesky/", "/tmp/logs/bluesky/"], - [False, "/dls_sw/test/logs/bluesky/", "/dls/tmp/test/logs/bluesky/"], - ], -) -@patch("mx_bluesky.common.utils.log.Path.mkdir") -def test_get_logging_dir_uses_beamline_if_no_dir_env_var( - mock_mkdir: MagicMock, - dev_mode: bool, - expected_log_dir: str, - expected_debug_log_dir: str, +@pytest.mark.parametrize("env_var_name", ["LOG_DIR", "DEBUG_LOG_DIR"]) +def test_get_logging_dir_raises_valueerror_if_no_env_var_and_dev_mode_false( + env_var_name: str, monkeypatch ): - with patch.dict(os.environ, {"BEAMLINE": "test"}, clear=True): - assert log._get_logging_dirs(dev_mode) == ( - Path(expected_log_dir), - Path(expected_debug_log_dir), - ) - assert mock_mkdir.call_count == 2 + monkeypatch.delenv(env_var_name) + with pytest.raises( + ValueError, match=f"{env_var_name} environment variable is not set" + ): + log._get_logging_dirs(False) -@pytest.mark.parametrize("dev_mode", [True, False]) -@patch("mx_bluesky.common.utils.log.Path.mkdir") -def test_get_logging_dir_uses_tmp_if_no_env_var(mock_mkdir: MagicMock, dev_mode: bool): - assert log._get_logging_dirs(dev_mode) == ( +@patch("mx_bluesky.common.utils.log.Path.mkdir", MagicMock()) +def test_get_logging_dir_dev_mode_defaults_to_tmp(monkeypatch): + monkeypatch.delenv("LOG_DIR") + monkeypatch.delenv("DEBUG_LOG_DIR") + assert log._get_logging_dirs(True) == ( Path("/tmp/logs/bluesky"), Path("/tmp/logs/bluesky"), ) - assert mock_mkdir.call_count == 2 @pytest.mark.skip_log_setup -@patch("mx_bluesky.common.utils.log.Path.mkdir") @patch( "mx_bluesky.common.utils.log.integrate_bluesky_and_ophyd_logging", ) -def test_default_logging_setup_integrate_logs_flag( - mock_integrate_logs: MagicMock, mock_mkdir -): +def test_default_logging_setup_integrate_logs_flag(mock_integrate_logs: MagicMock): log.do_default_logging_setup( "hyperion.log", TEST_GRAYLOG_PORT, dev_mode=True, integrate_all_logs=False ) diff --git a/tests/unit_tests/hyperion/conftest.py b/tests/unit_tests/hyperion/conftest.py index 437c5c86dd..69a232a6d6 100644 --- a/tests/unit_tests/hyperion/conftest.py +++ b/tests/unit_tests/hyperion/conftest.py @@ -44,6 +44,16 @@ ) +@pytest.fixture(autouse=True) +def override_hyperion_blueapi_logging_setup(request): + log_path = Path("/tmp/logs/bluesky") + with patch( + "mx_bluesky.common.utils.log._get_logging_dirs", + return_value=(log_path, log_path), + ): + yield + + @pytest.fixture(scope="session") def executor() -> Generator[Executor, Any, Any]: ex = ThreadPoolExecutor(max_workers=1, thread_name_prefix="test thread") diff --git a/tests/unit_tests/hyperion/test_baton_handler.py b/tests/unit_tests/hyperion/test_baton_handler.py index 31520ea779..3a16a370bd 100644 --- a/tests/unit_tests/hyperion/test_baton_handler.py +++ b/tests/unit_tests/hyperion/test_baton_handler.py @@ -901,7 +901,7 @@ def wait_then_nothing(): ) -@patch("mx_bluesky.hyperion.blueapi_plans._robot_unload") +@patch("mx_bluesky.hyperion.blueapi_plans.in_process._robot_unload") def test_robot_unload_performed_when_no_more_agamemnon_instructions( mock_robot_unload, bluesky_context: BlueskyContext, @@ -935,7 +935,7 @@ def request_baton_away_from_hyperion(*args): mock_load_centre_collect.side_effect = request_baton_away_from_hyperion -@patch("mx_bluesky.hyperion.blueapi_plans._robot_unload") +@patch("mx_bluesky.hyperion.blueapi_plans.in_process._robot_unload") def test_robot_unload_performed_when_baton_requested_away_from_hyperion( mock_robot_unload, bluesky_context: BlueskyContext, @@ -956,7 +956,7 @@ def test_robot_unload_performed_when_baton_requested_away_from_hyperion( ) -@patch("mx_bluesky.hyperion.blueapi_plans._robot_unload") +@patch("mx_bluesky.hyperion.blueapi_plans.in_process._robot_unload") def test_robot_unload_not_performed_when_beamline_error( mock_robot_unload, bluesky_context: BlueskyContext, @@ -972,7 +972,7 @@ def test_robot_unload_not_performed_when_beamline_error( mock_robot_unload.assert_not_called() -@patch("mx_bluesky.hyperion.blueapi_plans._robot_unload") +@patch("mx_bluesky.hyperion.blueapi_plans.in_process._robot_unload") def test_robot_unload_still_performed_when_sample_exception( mock_robot_unload, bluesky_context: BlueskyContext, @@ -996,7 +996,7 @@ def test_robot_unload_still_performed_when_sample_exception( ) -@patch("mx_bluesky.hyperion.blueapi_plans._robot_unload") +@patch("mx_bluesky.hyperion.blueapi_plans.in_process._robot_unload") def test_detector_shutter_closed_when_baton_requested_away_from_hyperion( mock_robot_unload, bluesky_context: BlueskyContext, diff --git a/tests/unit_tests/hyperion/test_blueapi_plans.py b/tests/unit_tests/hyperion/test_blueapi_plans.py index b59f7e0e0d..7121431611 100644 --- a/tests/unit_tests/hyperion/test_blueapi_plans.py +++ b/tests/unit_tests/hyperion/test_blueapi_plans.py @@ -23,7 +23,7 @@ def bluesky_context(run_engine: RunEngine): }, { "kind": "planFunctions", - "module": "mx_bluesky.hyperion.blueapi_plans", + "module": "mx_bluesky.hyperion.blueapi_plans.blueapi", }, ] } @@ -64,7 +64,7 @@ def _call_blueapi_plan( parameters: dict, ): with patch( - f"mx_bluesky.hyperion.blueapi_plans.{internal_name}", + f"mx_bluesky.hyperion.blueapi_plans.in_process.{internal_name}", return_value=iter([]), create=False, ) as mock_plan: From c10cd6b0c0a6e1646a12d4660159ee081d670089 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 13 Feb 2026 16:26:18 +0000 Subject: [PATCH 3/5] Additional unit test --- src/mx_bluesky/common/utils/log.py | 10 ++++++++-- tests/unit_tests/common/utils/test_log.py | 21 ++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/mx_bluesky/common/utils/log.py b/src/mx_bluesky/common/utils/log.py index 8a23fa2947..86a95f7621 100644 --- a/src/mx_bluesky/common/utils/log.py +++ b/src/mx_bluesky/common/utils/log.py @@ -11,6 +11,7 @@ integrate_bluesky_and_ophyd_logging, set_up_all_logging_handlers, set_up_debug_memory_handler, + set_up_info_file_handler, ) from dodal.log import LOGGER as DODAL_LOGGER @@ -72,10 +73,15 @@ def setup_hyperion_blueapi_logging(log_file_name: str): Args: log_file_name: Base name of the log file. """ - root_logger = logging.getLogger() + dodal_logger = DODAL_LOGGER logging_path, debug_logging_path = _get_logging_dirs(False) set_up_debug_memory_handler( - root_logger, debug_logging_path, log_file_name, ERROR_LOG_BUFFER_LINES + dodal_logger, debug_logging_path, log_file_name, ERROR_LOG_BUFFER_LINES + ) + set_up_info_file_handler( + dodal_logger, + logging_path, + log_file_name, ) diff --git a/tests/unit_tests/common/utils/test_log.py b/tests/unit_tests/common/utils/test_log.py index d7c98a8c6f..26934415ff 100644 --- a/tests/unit_tests/common/utils/test_log.py +++ b/tests/unit_tests/common/utils/test_log.py @@ -7,8 +7,11 @@ import pytest from bluesky import plan_stubs as bps from bluesky import preprocessors as bpp +from dodal.log import ( + ERROR_LOG_BUFFER_LINES, + set_up_all_logging_handlers, +) from dodal.log import LOGGER as DODAL_LOGGER -from dodal.log import set_up_all_logging_handlers import mx_bluesky.common.utils.log as log from mx_bluesky.common.external_interaction.callbacks.common.log_uid_tag_callback import ( @@ -248,3 +251,19 @@ def test_default_logging_setup_integrate_logs_flag(mock_integrate_logs: MagicMoc "hyperion.log", TEST_GRAYLOG_PORT, dev_mode=True, integrate_all_logs=False ) mock_integrate_logs.assert_not_called() + + +@patch("mx_bluesky.common.utils.log.set_up_debug_memory_handler") +@patch("mx_bluesky.common.utils.log.set_up_info_file_handler") +def test_setup_hyperion_blueapi_logging_configures_debug_and_file_handlers( + mock_info_setup: MagicMock, + mock_debug_setup: MagicMock, + clear_and_mock_loggers, + tmp_path: Path, +): + log.setup_hyperion_blueapi_logging("hyperion-test.log") + fh_emit, _ = clear_and_mock_loggers + mock_info_setup.assert_called_once_with(DODAL_LOGGER, tmp_path, "hyperion-test.log") + mock_debug_setup.assert_called_once_with( + DODAL_LOGGER, tmp_path, "hyperion-test.log", ERROR_LOG_BUFFER_LINES + ) From 81bea588a6fd5299db6995c9e10ad5a21646cebb Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 13 Feb 2026 16:38:40 +0000 Subject: [PATCH 4/5] Update docs re. log dirs and remove hyperion support for runnning hyperion in podman-compose --- docs/user/hyperion/logging.rst | 6 +- run_hyperion_in_podman.sh | 149 ------------------------- utility_scripts/docker/i03-compose.yml | 58 ---------- 3 files changed, 3 insertions(+), 210 deletions(-) delete mode 100755 run_hyperion_in_podman.sh delete mode 100644 utility_scripts/docker/i03-compose.yml diff --git a/docs/user/hyperion/logging.rst b/docs/user/hyperion/logging.rst index aa5e69d518..b6b4f42e96 100644 --- a/docs/user/hyperion/logging.rst +++ b/docs/user/hyperion/logging.rst @@ -38,6 +38,6 @@ Debug Log The standard logs files do not record all messages, only those at INFO level or higher, in order to keep storage requirements to a minimum. In the event of an error occurring, then trace-level logging for the most recent events (by default 5000) is flushed -to a separate set of log files. Due to the size of these files, they are stored separately from the main log files -. These logs are located in ``/dls/tmp//logs/bluesky`` by default, or -otherwise as specified by the ``DEBUG_LOG_DIR`` environment variable. +to a separate set of log files. Due to the size of these files, they are stored separately from the main +log files. These logs are typically located in ``/dls/tmp//logs/bluesky``, +as specified by the ``DEBUG_LOG_DIR`` environment variable. diff --git a/run_hyperion_in_podman.sh b/run_hyperion_in_podman.sh deleted file mode 100755 index 4adc1c8e17..0000000000 --- a/run_hyperion_in_podman.sh +++ /dev/null @@ -1,149 +0,0 @@ -#!/bin/bash - -STOP=0 -START=0 -UP=0 -IN_DEV=false - -show_help() { - echo "$(basename $0) [options...]" -cat < Date: Tue, 17 Feb 2026 17:23:54 +0000 Subject: [PATCH 5/5] Additional commentary on the blueapi module as discussed to indicate that import has side-effects --- src/mx_bluesky/hyperion/blueapi_plans/blueapi.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py b/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py index 7bc2c37e3d..ce13d54f17 100644 --- a/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py +++ b/src/mx_bluesky/hyperion/blueapi_plans/blueapi.py @@ -1,5 +1,7 @@ """ This module is the bluesky plan module for use with hyperion-blueapi. +Importing this module will configure debug and info logging as a side-effect - so this module should not be +imported directly by other components as it is intended only as the entry-point for BlueAPI. """ from mx_bluesky.common.utils.log import setup_hyperion_blueapi_logging