Remove need for 'ISPYB_CREDENTIALS' environment variable

.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/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)
 
 

src/murfey/server/ispyb.py

@@ -5,9 +5,6 @@
 from typing import Callable, Generator, List, Literal, Optional
 
 import ispyb
-
-# import ispyb.sqlalchemy
-import sqlalchemy.orm
 import workflows.transport
 from fastapi import Depends
 from ispyb.sqlalchemy import (
@@ -29,6 +26,8 @@
     ZcZocaloBuffer,
     url,
 )
+from sqlalchemy import create_engine
+from sqlalchemy.orm import Session, sessionmaker
 
 from murfey.util import sanitise
 from murfey.util.config import get_security_config
@@ -38,11 +37,16 @@
 security_config = get_security_config()
 
 try:
-    Session = sqlalchemy.orm.sessionmaker(
-        bind=sqlalchemy.create_engine(url(), connect_args={"use_pure": True})
+    ISPyBSession = sessionmaker(
+        bind=create_engine(
+            url(credentials=security_config.ispyb_credentials),
+            connect_args={"use_pure": True},
+        )
     )
+    log.info("Loaded ISPyB database session")
 except AttributeError:
-    Session = lambda: None
+    log.error("Error loading ISPyB session", exc_info=True)
+    ISPyBSession = lambda: None
 
 
 def _send_using_new_connection(transport_type: str, queue: str, message: dict) -> None:
@@ -67,6 +71,8 @@ def __init__(self, transport_type: Literal["PikaTransport"]):
             if security_config.ispyb_credentials
             else None
         )
+        if self.ispyb is not None:
+            print("Loaded ISPyB databse")
         self._connection_callback: Callable | None = None
 
     def reconnect(self):
@@ -87,7 +93,7 @@ def do_insert_data_collection_group(
         **kwargs,
     ):
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created DataCollectionGroup {record.dataCollectionGroupId}")
@@ -102,7 +108,7 @@ def do_insert_data_collection_group(
 
     def do_insert_atlas(self, record: Atlas):
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created Atlas {record.atlasId}")
@@ -119,7 +125,7 @@ def do_update_atlas(
         self, atlas_id: int, atlas_image: str, pixel_size: float, slot: int
     ):
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 atlas = db.query(Atlas).filter(Atlas.atlasId == atlas_id).one()
                 atlas.atlasImage = atlas_image or atlas.atlasImage
                 atlas.pixelSize = pixel_size or atlas.pixelSize
@@ -187,7 +193,7 @@ def do_insert_grid_square(
             pixelSize=grid_square_parameters.pixel_size,
         )
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created GridSquare {record.gridSquareId}")
@@ -204,7 +210,7 @@ def do_update_grid_square(
         self, grid_square_id: int, grid_square_parameters: GridSquareParameters
     ):
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 grid_square = (
                     db.query(GridSquare)
                     .filter(GridSquare.gridSquareId == grid_square_id)
@@ -295,7 +301,7 @@ def do_insert_foil_hole(
             pixelSize=foil_hole_parameters.pixel_size,
         )
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created FoilHole {record.foilHoleId}")
@@ -315,7 +321,7 @@ def do_update_foil_hole(
         foil_hole_parameters: FoilHoleParameters,
     ):
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 foil_hole = (
                     db.query(FoilHole).filter(FoilHole.foilHoleId == foil_hole_id).one()
                 )
@@ -373,7 +379,7 @@ def do_insert_data_collection(self, record: DataCollection, message=None, **kwar
             else "Created for Murfey"
         )
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 record.comments = comment
                 db.add(record)
                 db.commit()
@@ -389,7 +395,7 @@ def do_insert_data_collection(self, record: DataCollection, message=None, **kwar
 
     def do_insert_sample_group(self, record: BLSampleGroup) -> dict:
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created BLSampleGroup {record.blSampleGroupId}")
@@ -404,7 +410,7 @@ def do_insert_sample_group(self, record: BLSampleGroup) -> dict:
 
     def do_insert_sample(self, record: BLSample, sample_group_id: int) -> dict:
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created BLSample {record.blSampleId}")
@@ -427,7 +433,7 @@ def do_insert_sample(self, record: BLSample, sample_group_id: int) -> dict:
 
     def do_insert_subsample(self, record: BLSubSample) -> dict:
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created BLSubSample {record.blSubSampleId}")
@@ -442,7 +448,7 @@ def do_insert_subsample(self, record: BLSubSample) -> dict:
 
     def do_insert_sample_image(self, record: BLSampleImage) -> dict:
         try:
-            with Session() as db:
+            with ISPyBSession() as db:
                 db.add(record)
                 db.commit()
                 log.info(f"Created BLSampleImage {record.blSampleImageId}")
@@ -525,7 +531,7 @@ def do_update_processing_status(self, record: AutoProcProgram, **kwargs):
             return {"success": False, "return_value": None}
 
     def do_buffer_lookup(self, app_id: int, uuid: int) -> Optional[int]:
-        with Session() as db:
+        with ISPyBSession() as db:
             buffer_objects = (
                 db.query(ZcZocaloBuffer)
                 .filter_by(AutoProcProgramID=app_id, UUID=uuid)
@@ -536,8 +542,8 @@ def do_buffer_lookup(self, app_id: int, uuid: int) -> Optional[int]:
         return reference
 
 
-def _get_session() -> Generator[Optional[sqlalchemy.orm.Session], None, None]:
-    db = Session()
+def _get_session() -> Generator[Optional[Session], None, None]:
+    db = ISPyBSession()
     if db is None:
         yield None
         return
@@ -556,7 +562,7 @@ def get_session_id(
     proposal_code: str,
     proposal_number: str,
     visit_number: str,
-    db: sqlalchemy.orm.Session | None,
+    db: Session | None,
 ) -> int | None:
 
     # Log received lookup parameters
@@ -589,9 +595,7 @@ def get_session_id(
     return res
 
 
-def get_proposal_id(
-    proposal_code: str, proposal_number: str, db: sqlalchemy.orm.Session
-) -> int:
+def get_proposal_id(proposal_code: str, proposal_number: str, db: Session) -> int:
     query = (
         db.query(Proposal)
         .filter(
@@ -603,7 +607,7 @@ def get_proposal_id(
     return query[0].proposalId
 
 
-def get_sub_samples_from_visit(visit: str, db: sqlalchemy.orm.Session) -> List[Sample]:
+def get_sub_samples_from_visit(visit: str, db: Session) -> List[Sample]:
     proposal_id = get_proposal_id(visit[:2], visit.split("-")[0][2:], db)
     samples = (
         db.query(BLSampleGroup, BLSampleGroupHasBLSample, BLSample, BLSubSample)
@@ -628,10 +632,9 @@ def get_sub_samples_from_visit(visit: str, db: sqlalchemy.orm.Session) -> List[S
     return res
 
 
-def get_all_ongoing_visits(
-    microscope: str, db: sqlalchemy.orm.Session | None
-) -> list[Visit]:
+def get_all_ongoing_visits(microscope: str, db: Session | None) -> list[Visit]:
     if db is None:
+        print("No database found")
         return []
     query = (
         db.query(BLSession)
@@ -668,7 +671,7 @@ def get_all_ongoing_visits(
 
 def get_data_collection_group_ids(session_id):
     query = (
-        Session()
+        ISPyBSession()
         .query(DataCollectionGroup)
         .filter(
             DataCollectionGroup.sessionId == session_id,

tests/__init__.py

@@ -2,9 +2,9 @@
 
 from sqlmodel import create_engine
 
-url = (
+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']}"
 )
 
-engine = create_engine(url)
+murfey_db_engine = create_engine(murfey_db_url)

tests/cli/test_repost_failed_calls.py

@@ -6,7 +6,6 @@
 from unittest import mock
 
 from murfey.cli import repost_failed_calls
-from tests.conftest import mock_security_config_name
 
 
 @mock.patch("murfey.cli.repost_failed_calls.PikaTransport")
@@ -167,12 +166,11 @@ def test_run_repost_failed_calls(
     mock_repost,
     mock_purge,
     mock_security_configuration,
-    tmp_path,
 ):
     mock_jwt.encode.return_value = "dummy_token"
     mock_purge.return_value = ["/path/to/msg1"]
 
-    config_file = tmp_path / mock_security_config_name
+    config_file = mock_security_configuration
     with open(config_file) as f:
         security_config = json.load(f)