Remove need for 'ISPYB_CREDENTIALS' environment variable (#588)

'ispyb.sqlalchemy.url()' takes credentials as an argument, through which the config file can be passed directly to the function instead of relying on the 'ISPYB_CREDENTIALS' environment variable. This eliminates the last traces of our repo's need to explicitly define 'ISPYB_CREDENTIALS'.

Additionally, elements of the CI workflow and test code related to the Murfey and ISPyB databases have also been modified. 'ISPYB_CREDENTIALS' has also been dropped successfully from the test workflow, and fixtures have been put in place to allow for the creation of databases that will be reset to a desired initial state at the end of every test. This means that the database does not have to be shut down and rebooted with every database-related test, which should help with our testing times as we continue to improve Murfey's test coverage.

NOTE: Within our code base, it looks like ISPyB spawns SQLAlchemy Session objects, whereas the Murfey database spawns SQLModel ones. As such, SQLModel's functions might not work when querying the ISPyB database. The database session fixtures for the ISPyB and Murfey test databases have been written to reflect this.

Author: tieneupin

.github/workflows/ci.yml

@@ -41,6 +41,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
+
       - name: Use Python ${{ matrix.python-version }}
         uses: actions/setup-python@v5
         with:
@@ -67,13 +68,13 @@ jobs:
           docker run --detach --name rabbitmq -p 127.0.0.1:5672:5672 -p 127.0.0.1:15672:15672 test-rabbitmq
           docker container list -a
 
-      - name: Get ispyb database
+      - name: Get ISPyB database
         uses: actions/download-artifact@v4
         with:
           name: database
           path: database/
 
-      - name: Install package
+      - name: Install Murfey
         run: |
           set -eux
           pip install --disable-pip-version-check -e "."[cicd,client,server,developer]
@@ -84,7 +85,7 @@ jobs:
           mysql-version: "11.3"
           auto-start: false
 
-      - name: Set up test ipsyb database
+      - name: Set up test ISPyB database
         run: |
           set -eu
           cp ".github/workflows/config/my.cnf" .my.cnf
@@ -101,9 +102,14 @@ jobs:
                    schemas/ispyb/routines.sql \
                    grants/ispyb_processing.sql \
                    grants/ispyb_import.sql; do
-              echo Importing ${f}...
+
+              echo "Patching ${f} in SQL files to fix CLI escape issues..."
+              sed -i 's/\\-/-/g' "$f"
+
+              echo "Importing ${f}..."
               mariadb --defaults-file=.my.cnf < $f
           done
+
           mariadb --defaults-file=.my.cnf -e "CREATE USER ispyb_api@'%' IDENTIFIED BY 'password_1234'; GRANT ispyb_processing to ispyb_api@'%'; GRANT ispyb_import to ispyb_api@'%'; SET DEFAULT ROLE ispyb_processing FOR ispyb_api@'%';"
           mariadb --defaults-file=.my.cnf -e "CREATE USER ispyb_api_future@'%' IDENTIFIED BY 'password_4321'; GRANT SELECT ON ispybtest.* to ispyb_api_future@'%';"
           mariadb --defaults-file=.my.cnf -e "CREATE USER ispyb_api_sqlalchemy@'%' IDENTIFIED BY 'password_5678'; GRANT SELECT ON ispybtest.* to ispyb_api_sqlalchemy@'%'; GRANT INSERT ON ispybtest.* to ispyb_api_sqlalchemy@'%'; GRANT UPDATE ON ispybtest.* to ispyb_api_sqlalchemy@'%';"
@@ -112,18 +118,17 @@ jobs:
       - name: Check RabbitMQ is alive
         run: wget -t 10 -w 1 http://127.0.0.1:15672 -O -
 
-      - name: Run tests
+      - name: Run Murfey tests
         env:
           POSTGRES_HOST: localhost
           POSTGRES_PORT: 5432
           POSTGRES_DB: murfey_test_db
           POSTGRES_PASSWORD: psql_pwd
           POSTGRES_USER: psql_user
         run: |
-          export ISPYB_CREDENTIALS=".github/workflows/config/ispyb.cfg"
           PYTHONDEVMODE=1 pytest -v -ra --cov=murfey --cov-report=xml --cov-branch
 
-      - name: Upload to Codecov
+      - name: Upload test results to Codecov
         uses: codecov/codecov-action@v5
         with:
           name: ${{ matrix.python-version }}

.github/workflows/test.yml

@@ -3,11 +3,12 @@ name: Build and test
 on: [push, pull_request]
 
 env:
-  DATABASE_SCHEMA: 4.2.1 # released 2024-08-19
+  ISPYB_DATABASE_SCHEMA: 4.6.0
   # Installs from GitHub
   # Versions: https://github.com/DiamondLightSource/ispyb-database/tags
   # Previous version(s):
-  # 4.1.0
+  # 4.2.1 # released 2024-08-19
+  # 4.1.0 # released 2024-03-26
 
 permissions:
   contents: read
@@ -53,10 +54,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - name: Download ISPyB DB schema v${{ env.DATABASE_SCHEMA }} for tests
+      - name: Download ISPyB DB schema v${{ env.ISPYB_DATABASE_SCHEMA }} for tests
         run: |
           mkdir database
-          wget -t 3 --waitretry=20 https://github.com/DiamondLightSource/ispyb-database/releases/download/v${{ env.DATABASE_SCHEMA }}/ispyb-database-${{ env.DATABASE_SCHEMA }}.tar.gz -O database/ispyb-database.tar.gz
+          wget -t 3 --waitretry=20 https://github.com/DiamondLightSource/ispyb-database/releases/download/v${{ env.ISPYB_DATABASE_SCHEMA }}/ispyb-database-${{ env.ISPYB_DATABASE_SCHEMA }}.tar.gz -O database/ispyb-database.tar.gz
       - name: Store database artifact
         uses: actions/upload-artifact@v4
         with:

src/murfey/cli/spa_ispyb_messages.py

@@ -19,7 +19,7 @@
 
 from murfey.client.contexts.spa import _get_xml_list_index
 from murfey.server import _murfey_id, _register
-from murfey.server.ispyb import Session, TransportManager, get_session_id
+from murfey.server.ispyb import ISPyBSession, TransportManager, get_session_id
 from murfey.server.murfey_db import url
 from murfey.util import db
 from murfey.util.config import get_machine_config, get_microscope, get_security_config
@@ -256,7 +256,7 @@ def run():
             proposal_code=args.visit[:2],
             proposal_number=args.visit[2:].split("-")[0],
             visit_number=args.visit.split("-")[1],
-            db=Session(),
+            db=ISPyBSession(),
         ),
     )
 

src/murfey/server/__init__.py

@@ -46,7 +46,7 @@
 import murfey
 import murfey.server.prometheus as prom
 import murfey.util.db as db
-from murfey.server.ispyb import get_session_id
+from murfey.server.ispyb import ISPyBSession, get_session_id
 from murfey.server.murfey_db import url  # murfey_db
 from murfey.util import LogFilter
 from murfey.util.config import (
@@ -2203,7 +2203,7 @@ def feedback_callback(header: dict, message: dict) -> None:
                 proposal_code=message["proposal_code"],
                 proposal_number=message["proposal_number"],
                 visit_number=message["visit_number"],
-                db=murfey.server.ispyb.Session(),
+                db=ISPyBSession(),
             )
             if dcg_murfey := murfey_db.exec(
                 select(db.DataCollectionGroup)
@@ -2281,7 +2281,7 @@ def feedback_callback(header: dict, message: dict) -> None:
                 proposal_code=message["proposal_code"],
                 proposal_number=message["proposal_number"],
                 visit_number=message["visit_number"],
-                db=murfey.server.ispyb.Session(),
+                db=ISPyBSession(),
             )
             dcg = murfey_db.exec(
                 select(db.DataCollectionGroup)
@@ -2380,7 +2380,7 @@ def feedback_callback(header: dict, message: dict) -> None:
             ).all():
                 pid = pj_murfey[0].id
             else:
-                if murfey.server.ispyb.Session() is None:
+                if ISPyBSession() is None:
                     murfey_pj = db.ProcessingJob(recipe=message["recipe"], dc_id=_dcid)
                 else:
                     record = ProcessingJob(
@@ -2410,7 +2410,7 @@ def feedback_callback(header: dict, message: dict) -> None:
             if not murfey_db.exec(
                 select(db.AutoProcProgram).where(db.AutoProcProgram.pj_id == pid)
             ).all():
-                if murfey.server.ispyb.Session() is None:
+                if ISPyBSession() is None:
                     murfey_app = db.AutoProcProgram(pj_id=pid)
                 else:
                     record = AutoProcProgram(

src/murfey/server/api/__init__.py

@@ -892,6 +892,9 @@ def _add_tilt():
 
 @router.get("/instruments/{instrument_name}/visits_raw", response_model=List[Visit])
 def get_current_visits(instrument_name: str, db=murfey.server.ispyb.DB):
+    log.debug(
+        f"Received request to look up ongoing visits for {sanitise(instrument_name)}"
+    )
     return murfey.server.ispyb.get_all_ongoing_visits(instrument_name, db)