From 7623c3598dc3ee43306c53069e7208e3d77271a0 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 10 Oct 2025 17:21:18 +0100 Subject: [PATCH 01/10] Updated type hinting in Websocket API module to use pipe-based unions now that Python 3.9 has been dropped --- src/murfey/server/api/websocket.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/murfey/server/api/websocket.py b/src/murfey/server/api/websocket.py index 9765dd31b..b0a012acf 100644 --- a/src/murfey/server/api/websocket.py +++ b/src/murfey/server/api/websocket.py @@ -4,7 +4,7 @@ import json import logging from datetime import datetime -from typing import Any, TypeVar, Union +from typing import Any, TypeVar from fastapi import APIRouter, WebSocket, WebSocketDisconnect from sqlmodel import Session, select @@ -27,7 +27,7 @@ def __init__(self): async def connect( self, websocket: WebSocket, - client_id: Union[int, str], + client_id: int | str, register_client: bool = True, ): await websocket.accept() @@ -48,7 +48,7 @@ def _register_new_client(client_id: int): murfey_db.commit() murfey_db.close() - def disconnect(self, client_id: Union[int, str], unregister_client: bool = True): + def disconnect(self, client_id: int | str, unregister_client: bool = True): self.active_connections.pop(client_id) if unregister_client: murfey_db: Session = next(get_murfey_db_session()) @@ -97,7 +97,7 @@ async def websocket_endpoint(websocket: WebSocket, client_id: int): @ws.websocket("/connect/{client_id}") async def websocket_connection_endpoint( websocket: WebSocket, - client_id: Union[int, str], + client_id: int | str, ): await manager.connect(websocket, client_id, register_client=False) await manager.broadcast(f"Client {client_id} joined") @@ -161,7 +161,7 @@ async def close_ws_connection(client_id: int): @ws.delete("/connect/{client_id}") -async def close_unrecorded_ws_connection(client_id: Union[int, str]): +async def close_unrecorded_ws_connection(client_id: int | str): client_id_str = str(client_id).replace("\r\n", "").replace("\n", "") log.info(f"Disconnecting {client_id_str}") manager.disconnect(client_id) From de5c74d546bc5c80e670afbbf022d3265f6d3b68 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 10 Oct 2025 17:24:27 +0100 Subject: [PATCH 02/10] Adjusted if-block conditions and updated comments --- src/murfey/util/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/murfey/util/api.py b/src/murfey/util/api.py index c402d66a6..e94a86231 100644 --- a/src/murfey/util/api.py +++ b/src/murfey/util/api.py @@ -121,10 +121,11 @@ def url_path_for( ) logger.error(message) raise KeyError(message) - # Skip complicated type resolution for now + # Skip complicated type resolution if param_type.startswith("typing."): continue - elif type(kwargs[param_name]).__name__ not in param_type: + # Validate incoming type against allowed ones + if type(kwargs[param_name]).__name__ not in param_type: message = ( f"Error validating parameters for {function_name!r}; " f"{param_name!r} must be {param_type!r}, " From 9656dcc7266aeb8eaf4f1702c36248feff5a26da Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 10 Oct 2025 17:26:26 +0100 Subject: [PATCH 03/10] Added a custom Dumper class that indents items in accordance with Prettier's standards --- src/murfey/cli/generate_route_manifest.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/murfey/cli/generate_route_manifest.py b/src/murfey/cli/generate_route_manifest.py index ba14b5cb8..39550b3df 100644 --- a/src/murfey/cli/generate_route_manifest.py +++ b/src/murfey/cli/generate_route_manifest.py @@ -17,6 +17,17 @@ import murfey +class PrettierDumper(yaml.Dumper): + """ + Custom YAML Dumper class that sets `indentless` to False. This generates a YAML + file that is then compliant with Prettier's formatting style + """ + + def increase_indent(self, flow=False, indentless=False): + # Force 'indentless=False' so list items align with Prettier + return super(PrettierDumper, self).increase_indent(flow, indentless=False) + + def find_routers(name: str) -> dict[str, APIRouter]: def _extract_routers_from_module(module: ModuleType): @@ -138,7 +149,14 @@ def run(): murfey_dir = Path(murfey.__path__[0]) manifest_file = murfey_dir / "util" / "route_manifest.yaml" with open(manifest_file, "w") as file: - yaml.dump(manifest, file, default_flow_style=False, sort_keys=False) + yaml.dump( + manifest, + file, + Dumper=PrettierDumper, + default_flow_style=False, + sort_keys=False, + indent=2, + ) print( "Route manifest for instrument and backend servers saved to " f"{str(manifest_file)!r}" From 8bc7fc03a55722765f54dc55f12941f2652ab255 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 10 Oct 2025 17:28:13 +0100 Subject: [PATCH 04/10] Updated route manifest to use the new pipe-basd union type hints --- src/murfey/util/route_manifest.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/murfey/util/route_manifest.yaml b/src/murfey/util/route_manifest.yaml index 874eb6f75..409d9589f 100644 --- a/src/murfey/util/route_manifest.yaml +++ b/src/murfey/util/route_manifest.yaml @@ -1194,7 +1194,7 @@ murfey.server.api.websocket.ws: function: websocket_connection_endpoint path_params: - name: client_id - type: typing.Union[int, str] + type: int | str methods: [] - path: /ws/test/{client_id} function: close_ws_connection @@ -1207,7 +1207,7 @@ murfey.server.api.websocket.ws: function: close_unrecorded_ws_connection path_params: - name: client_id - type: typing.Union[int, str] + type: int | str methods: - DELETE murfey.server.api.workflow.correlative_router: From a6b6d08e52aab25fe6483b314042a3857ce09236 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 10 Oct 2025 18:40:53 +0100 Subject: [PATCH 05/10] Added logic to print errors only when module imports genuinely fail; added logic to format strings according to Prettier's standards as well --- src/murfey/cli/generate_route_manifest.py | 97 ++++++++++++++++------- 1 file changed, 69 insertions(+), 28 deletions(-) diff --git a/src/murfey/cli/generate_route_manifest.py b/src/murfey/cli/generate_route_manifest.py index 39550b3df..bbea95e3d 100644 --- a/src/murfey/cli/generate_route_manifest.py +++ b/src/murfey/cli/generate_route_manifest.py @@ -3,8 +3,10 @@ server and backend server to enable lookup of the URLs based on function name. """ +import contextlib import importlib import inspect +import io import pkgutil from argparse import ArgumentParser from pathlib import Path @@ -28,6 +30,43 @@ def increase_indent(self, flow=False, indentless=False): return super(PrettierDumper, self).increase_indent(flow, indentless=False) +def prettier_str_representer(dumper, data): + """ + Helper function to format strings according to Prettier's standards: + - No quoting unless it can be misinterpreted as another data type + - When quoting, use double quotes unless string already contains double quotes + """ + + def is_implicitly_resolved(value: str) -> bool: + for ( + first_char, + resolvers, + ) in yaml.resolver.Resolver.yaml_implicit_resolvers.items(): + if first_char is None or (value and value[0] in first_char): + for resolver in resolvers: + if len(resolver) == 3: + _, regexp, _ = resolver + else: + _, regexp = resolver + if regexp.match(value): + return True + return False + + # If no quoting is needed, use default plain style + if not is_implicitly_resolved(data): + return dumper.represent_scalar("tag:yaml.org,2002:str", data) + + # If the string already contains double quotes, fall back to single quotes + if '"' in data and "'" not in data: + return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="'") + + # Otherwise, prefer double quotes + return dumper.represent_scalar("tag:yaml.org,2002:str", data, style='"') + + +PrettierDumper.add_representer(str, prettier_str_representer) + + def find_routers(name: str) -> dict[str, APIRouter]: def _extract_routers_from_module(module: ModuleType): @@ -41,34 +80,36 @@ def _extract_routers_from_module(module: ModuleType): routers = {} - # Import the module or package - try: - root = importlib.import_module(name) - except ImportError: - raise ImportError( - f"Cannot import '{name}'. Please ensure that you've installed all the " - "dependencies for the client, instrument server, and backend server " - "before running this command." - ) - - # If it's a package, walk through submodules and extract routers from each - if hasattr(root, "__path__"): - module_list = pkgutil.walk_packages(root.__path__, prefix=name + ".") - for _, module_name, _ in module_list: - try: - module = importlib.import_module(module_name) - except ImportError: - raise ImportError( - f"Cannot import '{module_name}'. Please ensure that you've " - "installed all the dependencies for the client, instrument " - "server, and backend server before running this command." - ) - - routers.update(_extract_routers_from_module(module)) - - # Extract directly from single module - else: - routers.update(_extract_routers_from_module(root)) + # Silence output during import and only return messages if imports fail + buffer = io.StringIO() + with contextlib.redirect_stdout(buffer), contextlib.redirect_stderr(buffer): + # Import the module or package + try: + root = importlib.import_module(name) + except Exception as e: + captured_logs = buffer.getvalue().strip() + message = f"Cannot import '{name}': {e}" + if captured_logs: + message += f"\n--- Captured output ---\n{captured_logs}" + raise ImportError(message) from e + + # If it's a package, walk through submodules and extract routers from each + if hasattr(root, "__path__"): + module_list = pkgutil.walk_packages(root.__path__, prefix=name + ".") + for _, module_name, _ in module_list: + try: + module = importlib.import_module(module_name) + except Exception as e: + captured_logs = buffer.getvalue().strip() + message = f"Cannot import '{name}': {e}" + if captured_logs: + message += f"\n--- Captured output ---\n{captured_logs}" + raise ImportError(message) from e + routers.update(_extract_routers_from_module(module)) + + # Extract directly from single module + else: + routers.update(_extract_routers_from_module(root)) return routers From 7f0ec4444439a2f2fb38e5e4ee466699b2f2df15 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Mon, 13 Oct 2025 10:28:11 +0100 Subject: [PATCH 06/10] Removed leftover test function from 'murfey.util.api' --- src/murfey/util/api.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/murfey/util/api.py b/src/murfey/util/api.py index e94a86231..d1544ad45 100644 --- a/src/murfey/util/api.py +++ b/src/murfey/util/api.py @@ -136,14 +136,3 @@ def url_path_for( # Render and return the path return render_path(route_path, kwargs) - - -if __name__ == "__main__": - # Run test on some existing routes - url_path = url_path_for( - "workflow.tomo_router", - "register_tilt", - visit_name="nt15587-15", - session_id=2, - ) - print(url_path) From de9922a4a4f02f95ec715721e12cdfc3a926b176 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Mon, 13 Oct 2025 10:29:32 +0100 Subject: [PATCH 07/10] Added integrated test for 'url_path_for' and its dependencies --- tests/util/test_api.py | 44 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/util/test_api.py diff --git a/tests/util/test_api.py b/tests/util/test_api.py new file mode 100644 index 000000000..d9562f627 --- /dev/null +++ b/tests/util/test_api.py @@ -0,0 +1,44 @@ +import pytest + +from murfey.util.api import url_path_for + +url_path_test_matrix: tuple[tuple[str, str, dict[str, str | int], str], ...] = ( + # Router name | Function name | kwargs | Expected URL + ( + "instrument_server.api.router", + "health", + {}, + "/health", + ), + ( + "instrument_server.api.router", + "stop_multigrid_watcher", + {"session_id": 0, "label": "some_label"}, + "/sessions/0/multigrid_watcher/some_label", + ), + ( + "api.hub.router", + "get_instrument_image", + {"instrument_name": "test"}, + "/instrument/test/image", + ), + ( + "api.instrument.router", + "check_if_session_is_active", + { + "instrument_name": "test", + "session_id": 0, + }, + "/instrument_server/instruments/test/sessions/0/active", + ), +) + + +@pytest.mark.parametrize("test_params", url_path_test_matrix) +def test_url_path_for(test_params: tuple[str, str, dict[str, str | int], str]): + # Unpack test params + router_name, function_name, kwargs, expected_url_path = test_params + assert ( + url_path_for(router_name=router_name, function_name=function_name, **kwargs) + == expected_url_path + ) From 1fe7096092d454333ff4c1d6314f77d270ce6abc Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Mon, 13 Oct 2025 10:54:13 +0100 Subject: [PATCH 08/10] Added unit test for the 'generate_route_manifest' CLI --- tests/cli/test_generate_route_manifest_cli.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/cli/test_generate_route_manifest_cli.py diff --git a/tests/cli/test_generate_route_manifest_cli.py b/tests/cli/test_generate_route_manifest_cli.py new file mode 100644 index 000000000..6b4cc7602 --- /dev/null +++ b/tests/cli/test_generate_route_manifest_cli.py @@ -0,0 +1,26 @@ +import sys + +from pytest_mock import MockerFixture + +from murfey.cli.generate_route_manifest import run + + +def test_run( + mocker: MockerFixture, +): + # Mock out print() and exit() + mock_print = mocker.patch("builtins.print") + mock_exit = mocker.patch("builtins.exit") + + # Run the function with its args + sys.argv = ["--debug"] + run() + + # Check that the final print message and exit() are called + print_calls = mock_print.call_args_list + last_print_call = print_calls[-1] + last_printed = last_print_call.args[0] + assert last_printed.startswith( + "Route manifest for instrument and backend servers saved to" + ) + mock_exit.assert_called_once() From ce7a6233b89f455119d4babd070590f072e5decc Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Mon, 13 Oct 2025 11:07:33 +0100 Subject: [PATCH 09/10] Wrong 'sys.argv' values --- tests/cli/test_generate_route_manifest_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_generate_route_manifest_cli.py b/tests/cli/test_generate_route_manifest_cli.py index 6b4cc7602..a6abcdc3d 100644 --- a/tests/cli/test_generate_route_manifest_cli.py +++ b/tests/cli/test_generate_route_manifest_cli.py @@ -13,7 +13,7 @@ def test_run( mock_exit = mocker.patch("builtins.exit") # Run the function with its args - sys.argv = ["--debug"] + sys.argv = ["", "--debug"] run() # Check that the final print message and exit() are called From f5aae34fd5cf5cd17e8237e5d4fa1228a5618758 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Mon, 13 Oct 2025 14:27:59 +0100 Subject: [PATCH 10/10] Added 'generate_route_manifest' as a pre-commit hook step that triggers when FastAPI router modules are modified --- .pre-commit-config.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 04d777267..0667d1fda 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,18 @@ # YAML formatting specifications: https://yaml.org/spec/1.2.2/ repos: + # Generate up-to-date Murfey route manifest + # NOTE: This will only work if Murfey is installed in the current Python environment + - repo: local + hooks: + - id: generate-route-manifest + name: Generating Murfey route manifest + entry: murfey.generate_route_manifest + language: system + # Only run if FastAPI router-related modules are changed + files: ^src/murfey/(instrument_server/.+\.py|server/(main|api/.+)\.py)$ + pass_filenames: false + # Syntax validation and some basic sanity checks - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.5.0 # Released 2023-10-09