Instrument Server Fixes (#560)

- Added logic to instrument server's 'upload_gain_reference()' function to look for 'rsync_path' key from MachineConfig before settling on using Murfey's URL.

- Added logs to describe parameters passed to functions for more effective debuging.

Author: tieneupin

src/murfey/instrument_server/api.py

@@ -7,7 +7,7 @@
 from functools import partial
 from logging import getLogger
 from pathlib import Path
-from typing import Annotated, Dict, List, Optional, Union
+from typing import Annotated, Any, Dict, List, Optional, Union
 from urllib.parse import urlparse
 
 import requests
@@ -328,19 +328,43 @@ def upload_gain_reference(
     safe_gain_path = sanitise(str(gain_reference.gain_path))
     safe_visit_path = sanitise(gain_reference.visit_path)
     safe_destination_dir = sanitise(gain_reference.gain_destination_dir)
-    machine_config = requests.get(
+
+    # Load machine config and other needed properties
+    machine_config: dict[str, Any] = requests.get(
         f"{_get_murfey_url()}/instruments/{sanitise_nonpath(instrument_name)}/machine",
         headers={"Authorization": f"Bearer {tokens[session_id]}"},
     ).json()
+
+    # Validate that file passed is from the gain reference directory
+    gain_ref_dir = machine_config.get("gain_reference_directory", "")
+    if not safe_gain_path.startswith(gain_ref_dir):
+        raise ValueError(
+            "Gain reference file does not originate from the gain reference directory "
+            f"{gain_ref_dir!r}"
+        )
+
+    # Return the rsync URL if set, otherwise assume you are syncing via Murfey
+    rsync_url = urlparse(str(machine_config.get("rsync_url", _get_murfey_url())))
+    rsync_module = machine_config.get("rsync_module", "data")
+    rsync_path = f"{rsync_url.hostname}::{rsync_module}/{safe_visit_path}/{safe_destination_dir}/{secure_filename(gain_reference.gain_path.name)}"
+
+    # Run rsync subprocess to transfer gain reference
     cmd = [
         "rsync",
         posix_path(Path(safe_gain_path)),
-        f"{urlparse(_get_murfey_url(), allow_fragments=False).hostname}::{machine_config.get('rsync_module', 'data')}/{safe_visit_path}/{safe_destination_dir}/{secure_filename(gain_reference.gain_path.name)}",
+        rsync_path,
     ]
-    gain_rsync = subprocess.run(cmd)
+    gain_rsync = subprocess.run(
+        cmd,
+        capture_output=True,
+        text=True,
+    )
     if gain_rsync.returncode:
         logger.warning(
-            f"Gain reference file {safe_gain_path} was not successfully transferred to {safe_visit_path}/processing"
+            f"Failed to transfer gain reference file {safe_gain_path!r} to {f'{safe_visit_path}/processing'!r} \n"
+            f"Executed the following command: {' '.join(cmd)!r} \n"
+            f"Returned the following error: \n"
+            f"{gain_rsync.stderr}"
         )
         return {"success": False}
     return {"success": True}

src/murfey/server/__init__.py

@@ -2291,6 +2291,10 @@ def feedback_callback(header: dict, message: dict) -> None:
                 _transport_object.transport.ack(header)
             return None
         elif message["register"] == "data_collection":
+            logger.debug(
+                f"Received message named 'data_collection' containing the following items:\n"
+                f"{', '.join([f'{sanitise(key)}: {sanitise(value)}' for key, value in message.items()])}"
+            )
             murfey_session_id = message["session_id"]
             ispyb_session_id = murfey.server.ispyb.get_session_id(
                 microscope=message["microscope"],
@@ -2309,7 +2313,9 @@ def feedback_callback(header: dict, message: dict) -> None:
                 # flush_data_collections(message["source"], murfey_db)
             else:
                 logger.warning(
-                    f"No data collection group ID was found for image directory {sanitise(message['image_directory'])} and source {sanitise(message['source'])}"
+                    "No data collection group ID was found for image directory "
+                    f"{sanitise(message['image_directory'])} and source "
+                    f"{sanitise(message['source'])}"
                 )
                 if _transport_object:
                     _transport_object.transport.nack(header, requeue=True)

src/murfey/server/ispyb.py

@@ -2,7 +2,7 @@
 
 import datetime
 import logging
-from typing import Callable, List, Literal, Optional
+from typing import Callable, Generator, List, Literal, Optional
 
 import ispyb
 
@@ -30,6 +30,7 @@
     url,
 )
 
+from murfey.util import sanitise
 from murfey.util.config import get_security_config
 from murfey.util.models import FoilHoleParameters, GridSquareParameters, Sample, Visit
 
@@ -535,7 +536,7 @@ def do_buffer_lookup(self, app_id: int, uuid: int) -> Optional[int]:
         return reference
 
 
-def _get_session() -> sqlalchemy.orm.Session:
+def _get_session() -> Generator[Optional[sqlalchemy.orm.Session], None, None]:
     db = Session()
     if db is None:
         yield None
@@ -557,6 +558,17 @@ def get_session_id(
     visit_number: str,
     db: sqlalchemy.orm.Session | None,
 ) -> int | None:
+
+    # Log received lookup parameters
+    log.debug(
+        "Looking up ISPyB BLSession ID using the following values:\n"
+        f"microscope: {sanitise(microscope)}\n"
+        f"proposal_code: {sanitise(proposal_code)}\n"
+        f"proposal_number: {sanitise(proposal_number)}\n"
+        f"visit_number: {sanitise(visit_number)}\n"
+    )
+
+    # Lookup BLSession ID
     if db is None:
         return None
     query = (