From b55f1e7f6460a0620e26f542ec841d9a738993bb Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 15:30:24 +0100 Subject: [PATCH 01/17] Migrated contents of 'murfey.client.__init__' to new module 'murfey.client.tui.main', since its contents are related to the initialisation of the TUI app Removed unused key 'default_destination' from 'MurfeyInstanceEnvironment' setup, along with commented out keys --- src/murfey/client/__init__.py | 348 ---------------------------------- src/murfey/client/tui/main.py | 344 +++++++++++++++++++++++++++++++++ 2 files changed, 344 insertions(+), 348 deletions(-) create mode 100644 src/murfey/client/tui/main.py diff --git a/src/murfey/client/__init__.py b/src/murfey/client/__init__.py index 736c2a5ac..e69de29bb 100644 --- a/src/murfey/client/__init__.py +++ b/src/murfey/client/__init__.py @@ -1,348 +0,0 @@ -from __future__ import annotations - -import argparse -import configparser -import logging -import os -import platform -import shutil -import sys -import time -import webbrowser -from datetime import datetime -from pathlib import Path -from pprint import pprint -from queue import Queue -from typing import Literal -from urllib.parse import ParseResult, urlparse - -import requests -from rich.prompt import Confirm - -import murfey.client.update -import murfey.client.watchdir -import murfey.client.websocket -from murfey.client.customlogging import CustomHandler, DirectableRichHandler -from murfey.client.instance_environment import MurfeyInstanceEnvironment -from murfey.client.tui.app import MurfeyTUI -from murfey.client.tui.status_bar import StatusBar -from murfey.util.api import url_path_for -from murfey.util.client import authorised_requests, read_config -from murfey.util.models import Visit - -log = logging.getLogger("murfey.client") - -requests.get, requests.post, requests.put, requests.delete = authorised_requests() - - -def _get_visit_list(api_base: ParseResult, instrument_name: str): - proxy_path = api_base.path.rstrip("/") - get_visits_url = api_base._replace( - path=f"{proxy_path}{url_path_for('session_control.router', 'get_current_visits', instrument_name=instrument_name)}" - ) - server_reply = requests.get(get_visits_url.geturl()) - if server_reply.status_code != 200: - raise ValueError(f"Server unreachable ({server_reply.status_code})") - return [Visit.parse_obj(v) for v in server_reply.json()] - - -def write_config(config: configparser.ConfigParser): - mcch = os.environ.get("MURFEY_CLIENT_CONFIG_HOME") - murfey_client_config_home = Path(mcch) if mcch else Path.home() - with open(murfey_client_config_home / ".murfey", "w") as configfile: - config.write(configfile) - - -def main_loop( - source_watchers: list[murfey.client.watchdir.DirWatcher], - appearance_time: float, - transfer_all: bool, -): - log.info( - f"Murfey {murfey.__version__} on Python {'.'.join(map(str, sys.version_info[0:3]))} entering main loop" - ) - if appearance_time > 0: - modification_time: float | None = time.time() - appearance_time * 3600 - else: - modification_time = None - while True: - for sw in source_watchers: - sw.scan(modification_time=modification_time, transfer_all=transfer_all) - time.sleep(15) - - -def _enable_webbrowser_in_cygwin(): - """Helper function to make webbrowser.open() work in CygWin""" - if "cygwin" in platform.system().lower() and shutil.which("cygstart"): - webbrowser.register("cygstart", None, webbrowser.GenericBrowser("cygstart")) - - -def _check_for_updates( - server: ParseResult, install_version: None | Literal[True] | str -): - if install_version is True: - # User requested installation of the newest version - try: - murfey.client.update.check(server, force=True) - print("\nYou are already running the newest version of Murfey") - exit() - except Exception as e: - exit(f"Murfey update check failed with {e}") - - if install_version: - # User requested installation of a specific version - if murfey.client.update.install_murfey(server, install_version): - print(f"\nMurfey has been updated to version {install_version}") - exit() - else: - exit("Error occurred while updating Murfey") - - # Otherwise run a routine update check to ensure client and server are compatible - try: - murfey.client.update.check(server) - except Exception as e: - print(f"Murfey update check failed with {e}") - - -def run(): - # Load client config and server information - config = read_config() - instrument_name = config["Murfey"]["instrument_name"] - try: - server_routing = config["ServerRouter"] - except KeyError: - server_routing = {} - server_routing_prefix_found = False - if server_routing: - for path_prefix, server in server_routing.items(): - if str(Path.cwd()).startswith(path_prefix): - known_server = server - server_routing_prefix_found = True - break - else: - known_server = None - else: - known_server = config["Murfey"].get("server") - - # Set up argument parser with dynamic defaults based on client config - parser = argparse.ArgumentParser(description="Start the Murfey client") - parser.add_argument( - "--server", - metavar="HOST:PORT", - type=str, - help=f"Murfey server to connect to ({known_server})", - default=known_server, - ) - parser.add_argument("--visit", help="Name of visit") - parser.add_argument( - "--source", help="Directory to transfer files from", type=Path, default="." - ) - parser.add_argument( - "--destination", - help="Directory to transfer files to (syntax: 'data/2022/cm31093-2/tmp/murfey')", - ) - parser.add_argument( - "--update", - metavar="VERSION", - nargs="?", - default=None, - const=True, - help="Update Murfey to the newest or to a specific version", - ) - parser.add_argument( - "--demo", - action="store_true", - ) - parser.add_argument( - "--appearance-time", - type=float, - default=-1, - help="Only consider top level directories that have appeared more recently than this many hours ago", - ) - parser.add_argument( - "--fake-dc", - action="store_true", - default=False, - help="Do not perform data collection related calls to API (avoids database inserts)", - ) - parser.add_argument( - "--time-based-transfer", - action="store_true", - help="Transfer new files", - ) - parser.add_argument( - "--no-transfer", - action="store_true", - help="Avoid actually transferring files", - ) - parser.add_argument( - "--debug", - action="store_true", - help="Turn on debugging logs", - ) - parser.add_argument( - "--local", - action="store_true", - default=False, - help="Perform rsync transfers locally rather than remotely", - ) - parser.add_argument( - "--ignore-mdoc-metadata", - action="store_true", - default=False, - help="Do not attempt to read metadata from all mdoc files", - ) - parser.add_argument( - "--remove-files", - action="store_true", - default=False, - help="Remove source files immediately after their transfer", - ) - parser.add_argument( - "--name", - type=str, - default="", - help="Name of Murfey session to be created", - ) - parser.add_argument( - "--skip-existing-processing", - action="store_true", - default=False, - help="Do not trigger processing for any data directories currently on disk (you may have started processing for them in a previous murfey run)", - ) - args = parser.parse_args() - - # Logic to exit early based on parsed args - if not args.server: - exit("Murfey server not set. Please run with --server host:port") - if not args.server.startswith(("http://", "https://")): - if "://" in args.server: - exit("Unknown server protocol. Only http:// and https:// are allowed") - args.server = f"http://{args.server}" - if args.remove_files: - remove_prompt = Confirm.ask( - f"Are you sure you want to remove files from {args.source or Path('.').absolute()}?" - ) - if not remove_prompt: - exit("Exiting") - - # If a new server URL is provided, save info to config file - murfey_url = urlparse(args.server, allow_fragments=False) - if args.server != known_server: - # New server specified. Verify that it is real - print(f"Attempting to connect to new server {args.server}") - try: - murfey.client.update.check(murfey_url, install=False) - except Exception as e: - exit(f"Could not reach Murfey server at {args.server!r} - {e}") - - # If server is reachable then update the configuration - config["Murfey"]["server"] = args.server - write_config(config) - - # If user requested installation of a specific or a newer version then - # make that happen, otherwise ensure client and server are compatible and - # update if necessary. - _check_for_updates(server=murfey_url, install_version=args.update) - - if args.no_transfer: - log.info("No files will be transferred as --no-transfer flag was specified") - - # Check ISPyB (if set up) for ongoing visits - ongoing_visits = [] - if args.visit: - ongoing_visits = [args.visit] - elif server_routing_prefix_found: - for part in Path.cwd().parts: - if "-" in part: - ongoing_visits = [part] - break - if not ongoing_visits: - print("Ongoing visits:") - ongoing_visits = _get_visit_list(murfey_url, instrument_name) - pprint(ongoing_visits) - ongoing_visits = [v.name for v in ongoing_visits] - - _enable_webbrowser_in_cygwin() - - # Set up additional log handlers - log.setLevel(logging.DEBUG) - log_queue = Queue() - input_queue = Queue() - - # Rich-based console handler - rich_handler = DirectableRichHandler(enable_link_path=False) - rich_handler.setLevel(logging.DEBUG if args.debug else logging.INFO) - - # Set up websocket app and handler - client_id_response = requests.get( - f"{murfey_url.geturl()}{url_path_for('session_control.router', 'new_client_id')}" - ) - if client_id_response.status_code == 401: - exit( - "This instrument is not authorised to run the TUI app; please use the " - "Murfey web UI instead" - ) - elif client_id_response.status_code != 200: - exit( - "Unable to establish connection to Murfey server: \n" - f"{client_id_response.json()}" - ) - client_id: dict = client_id_response.json() - ws = murfey.client.websocket.WSApp( - server=args.server, - id=client_id["new_id"], - ) - ws_handler = CustomHandler(ws.send) - - # Add additional handlers and set logging levels - logging.getLogger().addHandler(rich_handler) - logging.getLogger().addHandler(ws_handler) - logging.getLogger("murfey").setLevel(logging.INFO) - logging.getLogger("websocket").setLevel(logging.WARNING) - - log.info("Starting Websocket connection") - - # Load machine data for subsequent sections - machine_data = requests.get( - f"{murfey_url.geturl()}{url_path_for('session_control.router', 'machine_info_by_instrument', instrument_name=instrument_name)}" - ).json() - gain_ref: Path | None = None - - # Set up Murfey environment instance and map it to websocket app - instance_environment = MurfeyInstanceEnvironment( - url=murfey_url, - client_id=ws.id, - instrument_name=instrument_name, - software_versions=machine_data.get("software_versions", {}), - # sources=[Path(args.source)], - # watchers=source_watchers, - default_destination=args.destination or str(datetime.now().year), - demo=args.demo, - processing_only_mode=server_routing_prefix_found, - rsync_url=( - urlparse(machine_data["rsync_url"]).hostname - if machine_data.get("rsync_url") - else "" - ), - ) - ws.environment = instance_environment - - # Set up and run Murfey TUI app - status_bar = StatusBar() - rich_handler.redirect = True - app = MurfeyTUI( - environment=instance_environment, - visits=ongoing_visits, - queues={"input": input_queue, "logs": log_queue}, - status_bar=status_bar, - dummy_dc=args.fake_dc, - do_transfer=not args.no_transfer, - gain_ref=gain_ref, - redirected_logger=rich_handler, - force_mdoc_metadata=not args.ignore_mdoc_metadata, - processing_enabled=machine_data.get("processing_enabled", True), - skip_existing_processing=args.skip_existing_processing, - ) - app.run() - rich_handler.redirect = False diff --git a/src/murfey/client/tui/main.py b/src/murfey/client/tui/main.py new file mode 100644 index 000000000..68ff64890 --- /dev/null +++ b/src/murfey/client/tui/main.py @@ -0,0 +1,344 @@ +from __future__ import annotations + +import argparse +import configparser +import logging +import os +import platform +import shutil +import sys +import time +import webbrowser +from pathlib import Path +from pprint import pprint +from queue import Queue +from typing import Literal +from urllib.parse import ParseResult, urlparse + +import requests +from rich.prompt import Confirm + +import murfey.client.update +import murfey.client.watchdir +import murfey.client.websocket +from murfey.client.customlogging import CustomHandler, DirectableRichHandler +from murfey.client.instance_environment import MurfeyInstanceEnvironment +from murfey.client.tui.app import MurfeyTUI +from murfey.client.tui.status_bar import StatusBar +from murfey.util.api import url_path_for +from murfey.util.client import authorised_requests, read_config +from murfey.util.models import Visit + +log = logging.getLogger("murfey.client") + +requests.get, requests.post, requests.put, requests.delete = authorised_requests() + + +def _get_visit_list(api_base: ParseResult, instrument_name: str): + proxy_path = api_base.path.rstrip("/") + get_visits_url = api_base._replace( + path=f"{proxy_path}{url_path_for('session_control.router', 'get_current_visits', instrument_name=instrument_name)}" + ) + server_reply = requests.get(get_visits_url.geturl()) + if server_reply.status_code != 200: + raise ValueError(f"Server unreachable ({server_reply.status_code})") + return [Visit.parse_obj(v) for v in server_reply.json()] + + +def write_config(config: configparser.ConfigParser): + mcch = os.environ.get("MURFEY_CLIENT_CONFIG_HOME") + murfey_client_config_home = Path(mcch) if mcch else Path.home() + with open(murfey_client_config_home / ".murfey", "w") as configfile: + config.write(configfile) + + +def main_loop( + source_watchers: list[murfey.client.watchdir.DirWatcher], + appearance_time: float, + transfer_all: bool, +): + log.info( + f"Murfey {murfey.__version__} on Python {'.'.join(map(str, sys.version_info[0:3]))} entering main loop" + ) + if appearance_time > 0: + modification_time: float | None = time.time() - appearance_time * 3600 + else: + modification_time = None + while True: + for sw in source_watchers: + sw.scan(modification_time=modification_time, transfer_all=transfer_all) + time.sleep(15) + + +def _enable_webbrowser_in_cygwin(): + """Helper function to make webbrowser.open() work in CygWin""" + if "cygwin" in platform.system().lower() and shutil.which("cygstart"): + webbrowser.register("cygstart", None, webbrowser.GenericBrowser("cygstart")) + + +def _check_for_updates( + server: ParseResult, install_version: None | Literal[True] | str +): + if install_version is True: + # User requested installation of the newest version + try: + murfey.client.update.check(server, force=True) + print("\nYou are already running the newest version of Murfey") + exit() + except Exception as e: + exit(f"Murfey update check failed with {e}") + + if install_version: + # User requested installation of a specific version + if murfey.client.update.install_murfey(server, install_version): + print(f"\nMurfey has been updated to version {install_version}") + exit() + else: + exit("Error occurred while updating Murfey") + + # Otherwise run a routine update check to ensure client and server are compatible + try: + murfey.client.update.check(server) + except Exception as e: + print(f"Murfey update check failed with {e}") + + +def run(): + # Load client config and server information + config = read_config() + instrument_name = config["Murfey"]["instrument_name"] + try: + server_routing = config["ServerRouter"] + except KeyError: + server_routing = {} + server_routing_prefix_found = False + if server_routing: + for path_prefix, server in server_routing.items(): + if str(Path.cwd()).startswith(path_prefix): + known_server = server + server_routing_prefix_found = True + break + else: + known_server = None + else: + known_server = config["Murfey"].get("server") + + # Set up argument parser with dynamic defaults based on client config + parser = argparse.ArgumentParser(description="Start the Murfey client") + parser.add_argument( + "--server", + metavar="HOST:PORT", + type=str, + help=f"Murfey server to connect to ({known_server})", + default=known_server, + ) + parser.add_argument("--visit", help="Name of visit") + parser.add_argument( + "--source", help="Directory to transfer files from", type=Path, default="." + ) + parser.add_argument( + "--destination", + help="Directory to transfer files to (syntax: 'data/2022/cm31093-2/tmp/murfey')", + ) + parser.add_argument( + "--update", + metavar="VERSION", + nargs="?", + default=None, + const=True, + help="Update Murfey to the newest or to a specific version", + ) + parser.add_argument( + "--demo", + action="store_true", + ) + parser.add_argument( + "--appearance-time", + type=float, + default=-1, + help="Only consider top level directories that have appeared more recently than this many hours ago", + ) + parser.add_argument( + "--fake-dc", + action="store_true", + default=False, + help="Do not perform data collection related calls to API (avoids database inserts)", + ) + parser.add_argument( + "--time-based-transfer", + action="store_true", + help="Transfer new files", + ) + parser.add_argument( + "--no-transfer", + action="store_true", + help="Avoid actually transferring files", + ) + parser.add_argument( + "--debug", + action="store_true", + help="Turn on debugging logs", + ) + parser.add_argument( + "--local", + action="store_true", + default=False, + help="Perform rsync transfers locally rather than remotely", + ) + parser.add_argument( + "--ignore-mdoc-metadata", + action="store_true", + default=False, + help="Do not attempt to read metadata from all mdoc files", + ) + parser.add_argument( + "--remove-files", + action="store_true", + default=False, + help="Remove source files immediately after their transfer", + ) + parser.add_argument( + "--name", + type=str, + default="", + help="Name of Murfey session to be created", + ) + parser.add_argument( + "--skip-existing-processing", + action="store_true", + default=False, + help="Do not trigger processing for any data directories currently on disk (you may have started processing for them in a previous murfey run)", + ) + args = parser.parse_args() + + # Logic to exit early based on parsed args + if not args.server: + exit("Murfey server not set. Please run with --server host:port") + if not args.server.startswith(("http://", "https://")): + if "://" in args.server: + exit("Unknown server protocol. Only http:// and https:// are allowed") + args.server = f"http://{args.server}" + if args.remove_files: + remove_prompt = Confirm.ask( + f"Are you sure you want to remove files from {args.source or Path('.').absolute()}?" + ) + if not remove_prompt: + exit("Exiting") + + # If a new server URL is provided, save info to config file + murfey_url = urlparse(args.server, allow_fragments=False) + if args.server != known_server: + # New server specified. Verify that it is real + print(f"Attempting to connect to new server {args.server}") + try: + murfey.client.update.check(murfey_url, install=False) + except Exception as e: + exit(f"Could not reach Murfey server at {args.server!r} - {e}") + + # If server is reachable then update the configuration + config["Murfey"]["server"] = args.server + write_config(config) + + # If user requested installation of a specific or a newer version then + # make that happen, otherwise ensure client and server are compatible and + # update if necessary. + _check_for_updates(server=murfey_url, install_version=args.update) + + if args.no_transfer: + log.info("No files will be transferred as --no-transfer flag was specified") + + # Check ISPyB (if set up) for ongoing visits + ongoing_visits = [] + if args.visit: + ongoing_visits = [args.visit] + elif server_routing_prefix_found: + for part in Path.cwd().parts: + if "-" in part: + ongoing_visits = [part] + break + if not ongoing_visits: + print("Ongoing visits:") + ongoing_visits = _get_visit_list(murfey_url, instrument_name) + pprint(ongoing_visits) + ongoing_visits = [v.name for v in ongoing_visits] + + _enable_webbrowser_in_cygwin() + + # Set up additional log handlers + log.setLevel(logging.DEBUG) + log_queue = Queue() + input_queue = Queue() + + # Rich-based console handler + rich_handler = DirectableRichHandler(enable_link_path=False) + rich_handler.setLevel(logging.DEBUG if args.debug else logging.INFO) + + # Set up websocket app and handler + client_id_response = requests.get( + f"{murfey_url.geturl()}{url_path_for('session_control.router', 'new_client_id')}" + ) + if client_id_response.status_code == 401: + exit( + "This instrument is not authorised to run the TUI app; please use the " + "Murfey web UI instead" + ) + elif client_id_response.status_code != 200: + exit( + "Unable to establish connection to Murfey server: \n" + f"{client_id_response.json()}" + ) + client_id: dict = client_id_response.json() + ws = murfey.client.websocket.WSApp( + server=args.server, + id=client_id["new_id"], + ) + ws_handler = CustomHandler(ws.send) + + # Add additional handlers and set logging levels + logging.getLogger().addHandler(rich_handler) + logging.getLogger().addHandler(ws_handler) + logging.getLogger("murfey").setLevel(logging.INFO) + logging.getLogger("websocket").setLevel(logging.WARNING) + + log.info("Starting Websocket connection") + + # Load machine data for subsequent sections + machine_data = requests.get( + f"{murfey_url.geturl()}{url_path_for('session_control.router', 'machine_info_by_instrument', instrument_name=instrument_name)}" + ).json() + gain_ref: Path | None = None + + # Set up Murfey environment instance and map it to websocket app + instance_environment = MurfeyInstanceEnvironment( + url=murfey_url, + client_id=ws.id, + instrument_name=instrument_name, + software_versions=machine_data.get("software_versions", {}), + demo=args.demo, + processing_only_mode=server_routing_prefix_found, + rsync_url=( + urlparse(machine_data["rsync_url"]).hostname + if machine_data.get("rsync_url") + else "" + ), + ) + ws.environment = instance_environment + + # Set up and run Murfey TUI app + status_bar = StatusBar() + rich_handler.redirect = True + app = MurfeyTUI( + environment=instance_environment, + visits=ongoing_visits, + queues={"input": input_queue, "logs": log_queue}, + status_bar=status_bar, + dummy_dc=args.fake_dc, + do_transfer=not args.no_transfer, + gain_ref=gain_ref, + redirected_logger=rich_handler, + force_mdoc_metadata=not args.ignore_mdoc_metadata, + processing_enabled=machine_data.get("processing_enabled", True), + skip_existing_processing=args.skip_existing_processing, + ) + app.run() + rich_handler.redirect = False From e7d7109b3376262d6e9e44de222fdc9e0661e3a3 Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 15:31:56 +0100 Subject: [PATCH 02/17] Removed unused key 'default_destination' from 'MurfeyInstanceEnvironment' setup in 'MultigridController' --- src/murfey/client/multigrid_control.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/murfey/client/multigrid_control.py b/src/murfey/client/multigrid_control.py index 533e1d1d2..1ce69e5b2 100644 --- a/src/murfey/client/multigrid_control.py +++ b/src/murfey/client/multigrid_control.py @@ -71,7 +71,6 @@ def __post_init__(self): client_id=0, murfey_session=self.session_id, software_versions=machine_data.get("software_versions", {}), - default_destination=f"{datetime.now().year}", demo=self.demo, visit=self.visit, dose_per_frame=self.data_collection_parameters.get("dose_per_frame"), From db1f315bbd8417eaa5e26b477d0c0be82a1eb66f Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 15:34:58 +0100 Subject: [PATCH 03/17] 'murfey.client.tui.__main__' has been unused for a very long time --- src/murfey/client/tui/__main__.py | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 src/murfey/client/tui/__main__.py diff --git a/src/murfey/client/tui/__main__.py b/src/murfey/client/tui/__main__.py deleted file mode 100644 index dbfd8dfc1..000000000 --- a/src/murfey/client/tui/__main__.py +++ /dev/null @@ -1,5 +0,0 @@ -from __future__ import annotations - -# from murfey.client.tui import MurfeyTUI - -# MurfeyTUI.run(log="textual.log", log_verbosity=2) From f95dd07627275ea8453c27e57a5a68a907bd834c Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 15:36:21 +0100 Subject: [PATCH 04/17] 'murfey.__main__' also doesn't appear to have been used in a long time --- src/murfey/__main__.py | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 src/murfey/__main__.py diff --git a/src/murfey/__main__.py b/src/murfey/__main__.py deleted file mode 100644 index 996b4d3ff..000000000 --- a/src/murfey/__main__.py +++ /dev/null @@ -1,5 +0,0 @@ -from __future__ import annotations - -from murfey.client import run - -__all__ = ["run"] From ea58ed3387315f0648b044652ec47ca1de366a5b Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 15:39:44 +0100 Subject: [PATCH 05/17] Refactored 'murfey.instrument_server.__init__' so that the Murfey update checker is run through to completion before the components needed to update the server are imported and initialised, thereby avoiding the files associated with the process from becoming locked --- src/murfey/instrument_server/__init__.py | 46 +++++++++++++++--------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/src/murfey/instrument_server/__init__.py b/src/murfey/instrument_server/__init__.py index 8a7caf5c7..180d177f4 100644 --- a/src/murfey/instrument_server/__init__.py +++ b/src/murfey/instrument_server/__init__.py @@ -1,21 +1,34 @@ -import argparse import logging from urllib.parse import urlparse -import uvicorn -from rich.logging import RichHandler - -import murfey -import murfey.client.update -import murfey.client.websocket -from murfey.client.customlogging import CustomHandler -from murfey.util import LogFilter from murfey.util.client import read_config logger = logging.getLogger("murfey.instrument_server") -def run(): +def check_for_updates(): + import murfey.client.update + + murfey_url = urlparse( + read_config().get("Murfey", "server", fallback=""), allow_fragments=False + ) + try: + murfey.client.update.check(murfey_url) + except Exception as e: + print(f"Murfey update check failed with {e}") + + +def start_instrument_server(): + import argparse + + import uvicorn + from rich.logging import RichHandler + + import murfey + import murfey.client.websocket + from murfey.client.customlogging import CustomHandler + from murfey.util import LogFilter + parser = argparse.ArgumentParser(description="Start the Murfey server") parser.add_argument( "--host", @@ -30,12 +43,6 @@ def run(): ) args = parser.parse_args() - murfey_url = urlparse(read_config()["Murfey"].get("server"), allow_fragments=False) - try: - murfey.client.update.check(murfey_url) - except Exception as e: - print(f"Murfey update check failed with {e}") - LogFilter.install() rich_handler = RichHandler(enable_link_path=False) @@ -45,7 +52,7 @@ def run(): logging.getLogger("uvicorn").addHandler(rich_handler) ws = murfey.client.websocket.WSApp( - server=read_config()["Murfey"].get("server"), + server=read_config().get("Murfey", "server", fallback=""), register_client=False, ) @@ -71,3 +78,8 @@ def run(): _running_server = uvicorn.Server(config=config) _running_server.run() logger.info("Instrument server shutting down") + + +def run(): + check_for_updates() + start_instrument_server() From 61c1ba573ed5e3df0587c84961aec66b619fd28c Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 16:01:15 +0100 Subject: [PATCH 06/17] Lock FastAPI to <0.116.0, as 'fastapi-cloud-cli', which was included as a dependency under the 'standard' FastAPI package in 0.116.0, currently uses 'httpx'<0.28.0, which causes a clash with 'bump-my-version', which uses 'httpx'>=0.28.0; updates the 'murfey.client' entry point to point to where the function to initiliase the TUI app has been migrated to --- pyproject.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2ef87836d..c2c8c1e81 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,13 +55,13 @@ developer = [ ] instrument-server = [ "aiohttp", - "fastapi[standard]", + "fastapi[standard]<0.116.0", "python-jose", ] server = [ "aiohttp", "cryptography", - "fastapi[standard]", + "fastapi[standard]<0.116.0", "ispyb>=10.2.4", # Responsible for setting requirements for SQLAlchemy and mysql-connector-python; "jinja2", "mrcfile", @@ -82,7 +82,7 @@ Documentation = "https://github.com/DiamondLightSource/python-murfey" GitHub = "https://github.com/DiamondLightSource/python-murfey" [project.scripts] "murfey.add_user" = "murfey.cli.add_user:run" -"murfey.client" = "murfey.client:run" +"murfey.client" = "murfey.client.tui.main:run" "murfey.create_db" = "murfey.cli.create_db:run" "murfey.db_sql" = "murfey.cli.murfey_db_sql:run" "murfey.decrypt_password" = "murfey.cli.decrypt_db_password:run" From cda4d3590fb54f08a6e751fc4a19f6d80e9c034f Mon Sep 17 00:00:00 2001 From: Eu-Pin Tien Date: Tue, 8 Jul 2025 17:02:47 +0100 Subject: [PATCH 07/17] Migrated tests to reflect refactoring of 'murfey.client' submodules --- tests/client/tui/__init__.py | 0 tests/client/tui/test_main.py | 66 +++++++++++++++++++++++++++++++++ tests/util/test_client.py | 70 ++--------------------------------- 3 files changed, 70 insertions(+), 66 deletions(-) create mode 100644 tests/client/tui/__init__.py create mode 100644 tests/client/tui/test_main.py diff --git a/tests/client/tui/__init__.py b/tests/client/tui/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/client/tui/test_main.py b/tests/client/tui/test_main.py new file mode 100644 index 000000000..cebfdaf30 --- /dev/null +++ b/tests/client/tui/test_main.py @@ -0,0 +1,66 @@ +from unittest import mock +from unittest.mock import Mock +from urllib.parse import urlparse + +import pytest + +from murfey.client.tui.main import _get_visit_list +from murfey.util.models import Visit + +test_get_visit_list_params_matrix = ( + ("http://0.0.0.0:8000",), + ("http://0.0.0.0:8000/api",), + ("http://murfey_server",), + ("http://murfey_server/api",), + ("http://murfey_server.com",), +) + + +@pytest.mark.parametrize("test_params", test_get_visit_list_params_matrix) +@mock.patch("murfey.client.requests") +def test_get_visit_list( + mock_request, + test_params: tuple[str], + mock_client_configuration, +): + # Unpack test params and set up other params + (server_url,) = test_params + instrument_name = mock_client_configuration["Murfey"]["instrument_name"] + + # Construct the expected request response + example_visits = [ + { + "start": "1999-09-09T09:00:00", + "end": "1999-09-11T09:00:00", + "session_id": 123456789, + "name": "cm12345-0", + "beamline": "murfey", + "proposal_title": "Commissioning Session 1", + }, + { + "start": "1999-09-09T09:00:00", + "end": "1999-09-11T09:00:00", + "session_id": 246913578, + "name": "cm23456-1", + "beamline": "murfey", + "proposal_title": "Cryo-cycle 1999", + }, + ] + mock_response = Mock() + mock_response.status_code = 200 + mock_response.json.return_value = example_visits + mock_request.get.return_value = mock_response + + # read_config() has to be patched using fixture, so has to be done in function + with mock.patch("murfey.util.client.read_config", mock_client_configuration): + visits = _get_visit_list(urlparse(server_url), instrument_name) + + # Check that request was sent with the correct URL + expected_url = ( + f"{server_url}/session_control/instruments/{instrument_name}/visits_raw" + ) + mock_request.get.assert_called_once_with(expected_url) + + # Check that expected outputs are correct (order-sensitive) + for v, visit in enumerate(visits): + assert visit.dict() == Visit.parse_obj(example_visits[v]).dict() diff --git a/tests/util/test_client.py b/tests/util/test_client.py index 6bbfb468a..a98df79d4 100644 --- a/tests/util/test_client.py +++ b/tests/util/test_client.py @@ -1,14 +1,11 @@ import json import os from pathlib import Path -from unittest.mock import Mock, patch -from urllib.parse import urlparse +from unittest import mock -from pytest import mark +import pytest -from murfey.client import _get_visit_list from murfey.util.client import read_config, set_default_acquisition_output -from murfey.util.models import Visit test_read_config_params_matrix = ( # Environment variable to set | Append to tmp_path @@ -27,7 +24,7 @@ ) -@mark.parametrize("test_params", test_read_config_params_matrix) +@pytest.mark.parametrize("test_params", test_read_config_params_matrix) def test_read_config( test_params: tuple[str, str], tmp_path, @@ -53,72 +50,13 @@ def test_read_config( mock_client_configuration.write(file) # Patch the OS environment variable and run the function - with patch.dict(os.environ, env_var_dict, clear=False): + with mock.patch.dict(os.environ, env_var_dict, clear=False): config = read_config() # Compare returned config with mock one assert dict(config["Murfey"]) == dict(mock_client_configuration["Murfey"]) -test_get_visit_list_params_matrix = ( - ("http://0.0.0.0:8000",), - ("http://0.0.0.0:8000/api",), - ("http://murfey_server",), - ("http://murfey_server/api",), - ("http://murfey_server.com",), -) - - -@mark.parametrize("test_params", test_get_visit_list_params_matrix) -@patch("murfey.client.requests") -def test_get_visit_list( - mock_request, - test_params: tuple[str], - mock_client_configuration, -): - # Unpack test params and set up other params - (server_url,) = test_params - instrument_name = mock_client_configuration["Murfey"]["instrument_name"] - - # Construct the expected request response - example_visits = [ - { - "start": "1999-09-09T09:00:00", - "end": "1999-09-11T09:00:00", - "session_id": 123456789, - "name": "cm12345-0", - "beamline": "murfey", - "proposal_title": "Commissioning Session 1", - }, - { - "start": "1999-09-09T09:00:00", - "end": "1999-09-11T09:00:00", - "session_id": 246913578, - "name": "cm23456-1", - "beamline": "murfey", - "proposal_title": "Cryo-cycle 1999", - }, - ] - mock_response = Mock() - mock_response.status_code = 200 - mock_response.json.return_value = example_visits - mock_request.get.return_value = mock_response - - # read_config() has to be patched using fixture, so has to be done in function - with patch("murfey.util.client.read_config", mock_client_configuration): - visits = _get_visit_list(urlparse(server_url), instrument_name) - - # Check that request was sent with the correct URL - expected_url = ( - f"{server_url}/session_control/instruments/{instrument_name}/visits_raw" - ) - mock_request.get.assert_called_once_with(expected_url) - - # Check that expected outputs are correct (order-sensitive) - for v, visit in enumerate(visits): - assert visit.dict() == Visit.parse_obj(example_visits[v]).dict() - - def test_set_default_acquisition_output_normal_operation(tmp_path): output_dir = tmp_path / "settings.json" settings_json = { From 0661937ebca702ccf31910fe1d7c52b9c3df6cff Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 8 Jul 2025 17:09:39 +0100 Subject: [PATCH 08/17] Forgot to patch 'requests' to its new location --- tests/client/tui/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/client/tui/test_main.py b/tests/client/tui/test_main.py index cebfdaf30..555b67cb1 100644 --- a/tests/client/tui/test_main.py +++ b/tests/client/tui/test_main.py @@ -17,7 +17,7 @@ @pytest.mark.parametrize("test_params", test_get_visit_list_params_matrix) -@mock.patch("murfey.client.requests") +@mock.patch("murfey.client.tui.main.requests") def test_get_visit_list( mock_request, test_params: tuple[str], From 83346e904d38f848cc08cbc3abd36c59961a9976 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 8 Jul 2025 17:25:49 +0100 Subject: [PATCH 09/17] Updated assertion in 'test_get_visit_list' to use Pydantic v2 method --- tests/client/tui/test_main.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/client/tui/test_main.py b/tests/client/tui/test_main.py index 555b67cb1..77608ebd6 100644 --- a/tests/client/tui/test_main.py +++ b/tests/client/tui/test_main.py @@ -63,4 +63,6 @@ def test_get_visit_list( # Check that expected outputs are correct (order-sensitive) for v, visit in enumerate(visits): - assert visit.dict() == Visit.parse_obj(example_visits[v]).dict() + assert ( + visit.model_dump() == Visit.model_validate(example_visits[v]).model_dump() + ) From 45864a9b3ba4eedc4390d3604db136f52189cb0a Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 8 Jul 2025 17:56:56 +0100 Subject: [PATCH 10/17] Skip ISPyB and Murfey database-related tests if they haven't been set up properly; this allows us to run Pytest locally again even without test databases configured --- tests/conftest.py | 103 +++++++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 46 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ad55f10d8..bc8101e4b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ import pytest from ispyb.sqlalchemy import BLSession, ExperimentType, Person, Proposal, url from sqlalchemy import Engine, RootTransaction, and_, create_engine, event, select +from sqlalchemy.exc import InterfaceError from sqlalchemy.ext.declarative import DeclarativeMeta from sqlalchemy.orm import Session as SQLAlchemySession from sqlalchemy.orm import sessionmaker @@ -206,51 +207,54 @@ def ispyb_db_session_factory(ispyb_engine): @pytest.fixture(scope="session") def seed_ispyb_db(ispyb_db_session_factory): - - # Populate the ISPyB table with some initial values - # Return existing table entry if already present - ispyb_db_session: SQLAlchemySession = ispyb_db_session_factory() - person_db_entry = get_or_create_db_entry( - session=ispyb_db_session, - table=Person, - lookup_kwargs={ - "givenName": ExampleVisit.given_name, - "familyName": ExampleVisit.family_name, - "login": ExampleVisit.login, - }, - ) - proposal_db_entry = get_or_create_db_entry( - session=ispyb_db_session, - table=Proposal, - lookup_kwargs={ - "personId": person_db_entry.personId, - "proposalCode": ExampleVisit.proposal_code, - "proposalNumber": str(ExampleVisit.proposal_number), - }, - ) - _ = get_or_create_db_entry( - session=ispyb_db_session, - table=BLSession, - lookup_kwargs={ - "proposalId": proposal_db_entry.proposalId, - "beamLineName": ExampleVisit.instrument_name, - "visit_number": ExampleVisit.visit_number, - }, - ) - _ = [ - get_or_create_db_entry( + try: + # Populate the ISPyB table with some initial values + # Return existing table entry if already present + ispyb_db_session: SQLAlchemySession = ispyb_db_session_factory() + person_db_entry = get_or_create_db_entry( + session=ispyb_db_session, + table=Person, + lookup_kwargs={ + "givenName": ExampleVisit.given_name, + "familyName": ExampleVisit.family_name, + "login": ExampleVisit.login, + }, + ) + proposal_db_entry = get_or_create_db_entry( + session=ispyb_db_session, + table=Proposal, + lookup_kwargs={ + "personId": person_db_entry.personId, + "proposalCode": ExampleVisit.proposal_code, + "proposalNumber": str(ExampleVisit.proposal_number), + }, + ) + _ = get_or_create_db_entry( session=ispyb_db_session, - table=ExperimentType, + table=BLSession, lookup_kwargs={ - "experimentTypeId": id, - "name": name, - "proposalType": "em", - "active": 1, + "proposalId": proposal_db_entry.proposalId, + "beamLineName": ExampleVisit.instrument_name, + "visit_number": ExampleVisit.visit_number, }, ) - for name, id in ISPyBTableValues.experiment_types.items() - ] - ispyb_db_session.close() + _ = [ + get_or_create_db_entry( + session=ispyb_db_session, + table=ExperimentType, + lookup_kwargs={ + "experimentTypeId": id, + "name": name, + "proposalType": "em", + "active": 1, + }, + ) + for name, id in ISPyBTableValues.experiment_types.items() + ] + ispyb_db_session.close() + # Skip ISPyB-related tests if the connection fails + except InterfaceError: + pytest.skip("ISPyB database has not been set up; skipping test") @pytest.fixture @@ -285,14 +289,21 @@ def ispyb_db_session( ======================================================================================= """ -murfey_db_url = ( - f"postgresql+psycopg2://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" - f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" -) + +@pytest.fixture(scope="session") +def murfey_db_url(): + try: + return ( + f"postgresql+psycopg2://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" + f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" + ) + # Skip Murfey database-related tests if the environment for it hasn't been set up + except KeyError: + pytest.skip("Murfey PostgreSQL database has not been set up; skipping test") @pytest.fixture(scope="session") -def murfey_db_engine(): +def murfey_db_engine(murfey_db_url): engine = create_engine(murfey_db_url) SQLModel.metadata.create_all(engine) yield engine From 95052b69ffe900fddb05f812dcb8c21c20cab345 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 8 Jul 2025 19:05:45 +0100 Subject: [PATCH 11/17] Added unit test for update checker --- tests/instrument_server/__init__.py | 0 tests/instrument_server/test_init.py | 69 ++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 tests/instrument_server/__init__.py create mode 100644 tests/instrument_server/test_init.py diff --git a/tests/instrument_server/__init__.py b/tests/instrument_server/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/instrument_server/test_init.py b/tests/instrument_server/test_init.py new file mode 100644 index 000000000..3dfe677cc --- /dev/null +++ b/tests/instrument_server/test_init.py @@ -0,0 +1,69 @@ +from unittest import mock +from urllib.parse import urlparse + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +import murfey +from murfey.instrument_server import check_for_updates +from murfey.server.api.bootstrap import bootstrap as bootstrap_router +from murfey.server.api.bootstrap import pypi as pypi_router +from murfey.util.api import url_path_for + +# Set up a test router with only the essential endpoints +app = FastAPI() +for router in [pypi_router, bootstrap_router]: + app.include_router(router) +client = TestClient(app) + + +check_for_updates_test_matrix = ( + # Downgrade, upgrade, or keep client version? + ("downgrade",), + ("upgrade",), + ("keep",), +) + + +@pytest.mark.parametrize("test_params", check_for_updates_test_matrix) +def test_check_for_updates( + test_params: tuple[str], +): + + # Unpack test params + (handle_client_version,) = test_params + + with ( + mock.patch("murfey.instrument_server.urlparse") as mock_parse, + mock.patch("murfey.client.update.requests.get") as mock_get, + ): + # Return the test client URL + api_base = urlparse("http://testserver", allow_fragments=False) + mock_parse.return_value = api_base + check_for_updates() + + # Modify client version as needed + current_version = murfey.__version__ + supported_client_version = murfey.__supported_client_version__ + + # Check that a request was sent to the test_client with the correct URL + proxy_path = api_base.path.rstrip("/") + version_check_url = api_base._replace( + path=f"{proxy_path}{url_path_for('bootstrap.version', 'get_version')}", + query=f"client_version={current_version}", + ) + mock_get.assert_any_call(version_check_url.geturl()) + + # Construct the mock response + mock_response = mock.Mock() + mock_response.json.return_value = { + "server": current_version, + "oldest-supported-client": supported_client_version, + "client-needs-update": True, + "client-needs-downgrade": False, + } + mock_response.status_code = 200 + mock_get.return_value = mock_response + + pass From 40397d33b7f98b39ad0594946ab9e50993ea7673 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 9 Jul 2025 11:45:41 +0100 Subject: [PATCH 12/17] Added pytest-mock as a developer package --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index c2c8c1e81..9ae39fea6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,6 +52,7 @@ developer = [ "ipykernel", # Enable interactive coding with VS Code and Jupyter Notebook "pre-commit", # Formatting, linting, type checking, etc. "pytest", # Test code functionality + "pytest-mock", # Additional mocking tools for unit tests ] instrument-server = [ "aiohttp", From 8a00d19f5a6204539e7a28737f0a66fa143e6fa1 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 9 Jul 2025 11:52:16 +0100 Subject: [PATCH 13/17] Stored messages denoting update success or failure in variables to minimise repetition and enable ease of use in test suite; added comment for when an 'exit()' is not needed --- src/murfey/client/update.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/murfey/client/update.py b/src/murfey/client/update.py index 8d722afe2..ce7732488 100644 --- a/src/murfey/client/update.py +++ b/src/murfey/client/update.py @@ -9,6 +9,10 @@ import murfey from murfey.util.api import url_path_for +# Standardised messages to print upon exit +UPDATE_SUCCESS = "Murfey has been updated. Please restart Murfey." +UPDATE_FAILURE = "Error occurred while updating Murfey." + def check(api_base: ParseResult, install: bool = True, force: bool = False): """ @@ -40,21 +44,20 @@ def check(api_base: ParseResult, install: bool = True, force: bool = False): ) result = install_murfey(api_base, versions["server"]) if result: - print("\nMurfey has been updated. Please restart Murfey") - exit() + exit(UPDATE_SUCCESS) else: - exit("Error occurred while updating Murfey") + exit(UPDATE_FAILURE) if versions["server"] != murfey.__version__: if force: result = install_murfey(api_base, versions["server"]) if result: - print("\nMurfey has been updated. Please restart Murfey") - exit() + exit(UPDATE_SUCCESS) else: - exit("Error occurred while updating Murfey") + exit(UPDATE_FAILURE) else: - print("An update is available, install with 'murfey update'.") + # Allow Murfey to start, but print an update prompt + print("An update is available, install with 'murfey.client --update'.") def install_murfey(api_base: ParseResult, version: str) -> bool: From f73835acf45031bd0eccc7bec5673dc1be8efcd4 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 9 Jul 2025 11:53:46 +0100 Subject: [PATCH 14/17] Parametrised update checker test so that it checks for the cases where the client is correct, needs upgrading, or needs downgrading --- tests/instrument_server/test_init.py | 120 ++++++++++++++++++--------- 1 file changed, 83 insertions(+), 37 deletions(-) diff --git a/tests/instrument_server/test_init.py b/tests/instrument_server/test_init.py index 3dfe677cc..3b2363c42 100644 --- a/tests/instrument_server/test_init.py +++ b/tests/instrument_server/test_init.py @@ -1,22 +1,24 @@ -from unittest import mock from urllib.parse import urlparse import pytest from fastapi import FastAPI from fastapi.testclient import TestClient +from packaging.version import Version +from pytest_mock import MockerFixture import murfey +from murfey.client.update import UPDATE_SUCCESS from murfey.instrument_server import check_for_updates -from murfey.server.api.bootstrap import bootstrap as bootstrap_router from murfey.server.api.bootstrap import pypi as pypi_router +from murfey.server.api.bootstrap import version as version_router from murfey.util.api import url_path_for # Set up a test router with only the essential endpoints app = FastAPI() -for router in [pypi_router, bootstrap_router]: +for router in [pypi_router, version_router]: app.include_router(router) client = TestClient(app) - +base_url = str(client.base_url) check_for_updates_test_matrix = ( # Downgrade, upgrade, or keep client version? @@ -29,41 +31,85 @@ @pytest.mark.parametrize("test_params", check_for_updates_test_matrix) def test_check_for_updates( test_params: tuple[str], + mocker: MockerFixture, ): # Unpack test params - (handle_client_version,) = test_params - - with ( - mock.patch("murfey.instrument_server.urlparse") as mock_parse, - mock.patch("murfey.client.update.requests.get") as mock_get, - ): - # Return the test client URL - api_base = urlparse("http://testserver", allow_fragments=False) - mock_parse.return_value = api_base + (bump_client_version,) = test_params + + # Modify client version as needed + current_version = murfey.__version__ + supported_client_version = murfey.__supported_client_version__ + + major, minor, patch = Version(current_version).release + + # Adjust the perceived client version in the function being tested + if bump_client_version == "downgrade": + support_client_version_parts = Version(supported_client_version).release + if patch == 0: + if minor == 0: + if major == 0: + # This can't be downgraded, so skip + pytest.skip("This version can't be downgraded anymore; skipping") + else: + major = support_client_version_parts[0] - 1 + print(f"Downgraded major version to {major}") + else: + minor = support_client_version_parts[1] - 1 + print(f"Downgraded minor version to {minor}") + else: + patch = support_client_version_parts[2] - 1 + print(f"Downgraded patch version to {patch}") + elif bump_client_version == "upgrade": + patch += 1 + print(f"Bumped patch version to {patch}") + mock_client_version = f"{major}.{minor}.{patch}" + + # Run the version check query and get a response to patch in later + api_base = urlparse(base_url, allow_fragments=False) + proxy_path = api_base.path.rstrip("/") + version_check_path = url_path_for("bootstrap.version", "get_version") + version_check_query = f"client_version={mock_client_version}" + version_check_url = api_base._replace( + path=f"{proxy_path}{version_check_path}", + query=version_check_query, + ) + version_check_response = client.get(f"{version_check_path}?{version_check_query}") + + # Check that the endpoint works as expected + assert version_check_response.status_code == 200 + assert version_check_response.json() == { + "server": current_version, + "oldest-supported-client": supported_client_version, + "client-needs-update": True if bump_client_version == "downgrade" else False, + "client-needs-downgrade": True if bump_client_version == "upgrade" else False, + } + + # Patch the URL parse result + mock_parse = mocker.patch("murfey.instrument_server.urlparse") + mock_parse.return_value = api_base + + # Patch the result of get + mock_get = mocker.patch("murfey.client.update.requests.get") + mock_get.return_value = version_check_response + + # Patch the installation function + mock_install = mocker.patch("murfey.client.update.install_murfey") + + # Patch the perceived client version + mocker.patch("murfey.client.update.murfey.__version__", new=mock_client_version) + + # If changing the client version, check that 'install_murfey' and 'exit' are called + if bump_client_version in ("upgrade", "downgrade"): + with pytest.raises(SystemExit) as exc_info: + check_for_updates() + mock_install.assert_called_once() + # Check that 'exit' is called with the correct message + assert exc_info.value.code == UPDATE_SUCCESS + # If client version is the same, 'install_murfey' shouldn't be called + else: check_for_updates() + mock_install.assert_not_called() - # Modify client version as needed - current_version = murfey.__version__ - supported_client_version = murfey.__supported_client_version__ - - # Check that a request was sent to the test_client with the correct URL - proxy_path = api_base.path.rstrip("/") - version_check_url = api_base._replace( - path=f"{proxy_path}{url_path_for('bootstrap.version', 'get_version')}", - query=f"client_version={current_version}", - ) - mock_get.assert_any_call(version_check_url.geturl()) - - # Construct the mock response - mock_response = mock.Mock() - mock_response.json.return_value = { - "server": current_version, - "oldest-supported-client": supported_client_version, - "client-needs-update": True, - "client-needs-downgrade": False, - } - mock_response.status_code = 200 - mock_get.return_value = mock_response - - pass + # Check that the query URL is correct + mock_get.assert_called_once_with(version_check_url.geturl()) From f0b893c560eeef15cec74b904f1bbfef58b24b0a Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 9 Jul 2025 13:18:52 +0100 Subject: [PATCH 15/17] Added extra logic when skipping database-related tests so that they genuinely raise errors if they fail in the GitHub Actions runners --- tests/conftest.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index bc8101e4b..7ce4a42ff 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -252,8 +252,10 @@ def seed_ispyb_db(ispyb_db_session_factory): for name, id in ISPyBTableValues.experiment_types.items() ] ispyb_db_session.close() - # Skip ISPyB-related tests if the connection fails except InterfaceError: + # If this fails in the GitHub test environment, raise it as a genuine error + if os.getenv("GITHUB_ACTIONS") == "true": + raise InterfaceError pytest.skip("ISPyB database has not been set up; skipping test") @@ -299,6 +301,9 @@ def murfey_db_url(): ) # Skip Murfey database-related tests if the environment for it hasn't been set up except KeyError: + # If this fails in the GitHub test environment, raise it as a genuine error + if os.getenv("GITHUB_ACTIONS") == "true": + raise KeyError pytest.skip("Murfey PostgreSQL database has not been set up; skipping test") From 72945e294a21feeac178a5c0d672d8db992b33b9 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 9 Jul 2025 13:23:22 +0100 Subject: [PATCH 16/17] Added explicit return to 'murfey_db_url' fixture --- tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/conftest.py b/tests/conftest.py index 7ce4a42ff..247744512 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -305,6 +305,7 @@ def murfey_db_url(): if os.getenv("GITHUB_ACTIONS") == "true": raise KeyError pytest.skip("Murfey PostgreSQL database has not been set up; skipping test") + return "" @pytest.fixture(scope="session") From 76a49111a73edc3e71b91cda703e9e45a48d92b6 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 9 Jul 2025 16:02:57 +0100 Subject: [PATCH 17/17] Added unit test to check that instrument server starts correctly --- tests/instrument_server/test_init.py | 68 +++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/instrument_server/test_init.py b/tests/instrument_server/test_init.py index 3b2363c42..30e218b67 100644 --- a/tests/instrument_server/test_init.py +++ b/tests/instrument_server/test_init.py @@ -1,6 +1,9 @@ +import sys +from typing import Optional from urllib.parse import urlparse import pytest +import uvicorn from fastapi import FastAPI from fastapi.testclient import TestClient from packaging.version import Version @@ -8,7 +11,7 @@ import murfey from murfey.client.update import UPDATE_SUCCESS -from murfey.instrument_server import check_for_updates +from murfey.instrument_server import check_for_updates, start_instrument_server from murfey.server.api.bootstrap import pypi as pypi_router from murfey.server.api.bootstrap import version as version_router from murfey.util.api import url_path_for @@ -20,6 +23,7 @@ client = TestClient(app) base_url = str(client.base_url) + check_for_updates_test_matrix = ( # Downgrade, upgrade, or keep client version? ("downgrade",), @@ -113,3 +117,65 @@ def test_check_for_updates( # Check that the query URL is correct mock_get.assert_called_once_with(version_check_url.geturl()) + + +start_instrument_server_test_matrix = ( + # Host | Port + ( + None, + None, + ), # Test default values + ( + "127.0.0.1", + 8000, + ), # Test manually included values +) + + +@pytest.mark.parametrize("test_params", start_instrument_server_test_matrix) +def test_start_instrument_server( + mocker: MockerFixture, test_params: tuple[Optional[str], Optional[int]] +): + + # Unpack test params + host, port = test_params + + # Patch the Uvicorn Server instance + mock_server = mocker.patch("uvicorn.Server") + # Disable 'run'; we just want to confirm it's called correctly + mock_server.run.return_value = lambda: None + + # Patch the websocket instance + mock_wsapp = mocker.patch("murfey.client.websocket.WSApp") + mock_wsapp.return_value = mocker.Mock() # Disable functionality + + # Construct the expected Uvicorn Config object and save it as a dict + expected_config = vars( + uvicorn.Config( + "murfey.instrument_server.main:app", + host=host if host is not None else "0.0.0.0", + port=port if port is not None else 8001, + log_config=None, + ws_ping_interval=300, + ws_ping_timeout=300, + ) + ) + + # Construct the arguments to pass to the instrument server + sys.argv = [ + "murfey.instrument_server", + ] + + # Add host and port if they're present + if host is not None: + sys.argv.extend(["--host", host]) + if port is not None: + sys.argv.extend(["--port", str(port)]) + + # Run the function + start_instrument_server() + + # Check that the server was called with the correct arguments + args, kwargs = mock_server.call_args + actual_config = vars(kwargs["config"]) + assert expected_config == actual_config