Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
bb60ac9
Passed ISPyB credentials in to 'url()' function to remove the need fo…
tieneupin May 6, 2025
b48d654
Added debug log to API endpoint for loading ongoing visits
tieneupin May 6, 2025
b68a062
Simplified imports of classes and functions in 'murfey.server.ispyb'
tieneupin May 6, 2025
03897de
Renamed URL and Engine for Murfey DB to make it more self-evident
tieneupin May 6, 2025
97dc116
Added fixture for ISPyB credentials file; modified security configura…
tieneupin May 6, 2025
a78d2bb
Simplified use of security configuration fixture in 'test_run_repost_…
tieneupin May 6, 2025
992f484
Renamed the Murfey engine imported for use in other tests
tieneupin May 6, 2025
822050c
Added fixtures to create a mock ISPyB session
tieneupin May 6, 2025
b951353
Attempt at writing unit test for 'get_session_id()' function
tieneupin May 6, 2025
43b9bfd
Adjusted scopes of common fixtures in 'conftest.py'
tieneupin May 6, 2025
f0d7a39
Added fixture to create a tmp_path Path that will persist for the tes…
tieneupin May 6, 2025
38eeee6
Accidentally referred to the table class instead of the table instanc…
tieneupin May 6, 2025
6f80755
Updated ISPyB database schema version
tieneupin May 6, 2025
d0e1631
Trying mariadb v10.6, which matches the version used by dependencies
tieneupin May 6, 2025
30e0e9f
Dynamically remove '\' characters from comments in SQL files to allow…
tieneupin May 6, 2025
84fb9bd
Updated names of CI workflow steps; removed ISPYB_CREDENTIALS variabl…
tieneupin May 7, 2025
7282ebf
Switched back to using latest stable version of MariaDB for CI workflow
tieneupin May 7, 2025
5825457
Added a class to 'conftest.py' storing the database parameters needed…
tieneupin May 7, 2025
e82bd67
Simplified the test for the 'get_session_id()' function, now that the…
tieneupin May 7, 2025
46c47a1
Changed variable name
tieneupin May 7, 2025
464b32e
Add and commit tables one-by-one
tieneupin May 7, 2025
20ccaaa
ISPyB tables are specifically 'sqlalchemy.orm.Session' objects; indic…
tieneupin May 7, 2025
32fe03a
Table selection in 'sqlalchemy' is done using 'scalar_one()', not 'on…
tieneupin May 7, 2025
8336fbd
Forgot to update parameters passed to 'get_session_id()' in test
tieneupin May 7, 2025
cabec6f
Added unit test for 'get_proposal_id()' function
tieneupin May 7, 2025
86e8a07
Forgot to add ISPyB database a a parameter
tieneupin May 7, 2025
3c656de
Try using 'begin_nested()' to roll back ISPyB database after tests
tieneupin May 7, 2025
06db6fa
Moved the insertion of starting ISPyB values to the session factory f…
tieneupin May 7, 2025
af829e7
Forgot to extract results of the database search
tieneupin May 7, 2025
a475490
Updated fixture name from 'ispyb_db' to 'ispyb_db_session'
tieneupin May 7, 2025
6b6575c
Tried alternate way of setting up test ISPyB database
tieneupin May 7, 2025
ee04386
Rearranged functions and removed 'attach_event_listener()' as a function
tieneupin May 7, 2025
bd06a43
Replaced 'start_postgres' with same test-safe functions as for ISPyB …
tieneupin May 7, 2025
8f298d8
Replaced use of 'start_postgres' with test-safe 'murfey_db_session' f…
tieneupin May 7, 2025
89a02b9
Populate Murfey database tables using 'SQLModel.metadata.create_all()'
tieneupin May 7, 2025
16fbc41
Murfey's Session table requires a value for 'id'; it doesn't auto-inc…
tieneupin May 7, 2025
f065efc
Wrong column name
tieneupin May 7, 2025
162288c
Murfey database uses SQLModel commands; recreate Murfey databaes fixt…
tieneupin May 7, 2025
8c37fde
Forgot a type hint
tieneupin May 7, 2025
8e1e920
Missed replacing some Murfey session ID parameters
tieneupin May 7, 2025
eb762cd
'tests/__init__.py' can be emptied
tieneupin May 8, 2025
88b3bee
Placeholder for future tests
tieneupin May 8, 2025
7f84d4c
Added 'ISPyBTableValues' class to store default table values in ISPyB…
tieneupin May 8, 2025
1cd0aab
'execute()' is deprecated for SQLModel Session objects, so use 'exec(…
tieneupin May 8, 2025
06aad02
Changed variable names; added unit test for 'get_data_collection_grou…
tieneupin May 8, 2025
d66c453
'get_data_collection_group_ids' does not appear to be in use; deleted…
tieneupin May 8, 2025
d849fd1
Merged recent changes from 'main' branch
tieneupin May 8, 2025
f0f70f3
Updated functions to use renamed ISPyBSession() variable
tieneupin May 9, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
python-version: ["3.9", "3.10", "3.11"]
services:
mariadb:
image: mariadb:11.7.2 # released 2024-05-06
image: mariadb:10.6 # released 2024-05-06
Comment thread
tieneupin marked this conversation as resolved.
Outdated
# Pulls image from DockerHub
# Docker images: https://hub.docker.com/_/mariadb
# Previous version(s):
Expand All @@ -41,6 +41,7 @@ jobs:

steps:
- uses: actions/checkout@v4

- name: Use Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
Expand All @@ -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]
Expand All @@ -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
Expand All @@ -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@'%';"
Expand All @@ -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 }}
Expand Down
9 changes: 5 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions src/murfey/server/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,9 @@

@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(

Check warning on line 895 in src/murfey/server/api/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/api/__init__.py#L895

Added line #L895 was not covered by tests
f"Received request to look up ongoing visits for {sanitise(instrument_name)}"
)
return murfey.server.ispyb.get_all_ongoing_visits(instrument_name, db)


Expand Down
63 changes: 33 additions & 30 deletions src/murfey/server/ispyb.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand All @@ -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")

Check warning on line 46 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L46

Added line #L46 was not covered by tests
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:
Expand All @@ -67,6 +71,8 @@
if security_config.ispyb_credentials
else None
)
if self.ispyb is not None:
print("Loaded ISPyB databse")

Check warning on line 75 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L75

Added line #L75 was not covered by tests
self._connection_callback: Callable | None = None

def reconnect(self):
Expand All @@ -87,7 +93,7 @@
**kwargs,
):
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 96 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L96

Added line #L96 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created DataCollectionGroup {record.dataCollectionGroupId}")
Expand All @@ -102,7 +108,7 @@

def do_insert_atlas(self, record: Atlas):
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 111 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L111

Added line #L111 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created Atlas {record.atlasId}")
Expand All @@ -119,7 +125,7 @@
self, atlas_id: int, atlas_image: str, pixel_size: float, slot: int
):
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 128 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L128

Added line #L128 was not covered by tests
atlas = db.query(Atlas).filter(Atlas.atlasId == atlas_id).one()
atlas.atlasImage = atlas_image or atlas.atlasImage
atlas.pixelSize = pixel_size or atlas.pixelSize
Expand Down Expand Up @@ -187,7 +193,7 @@
pixelSize=grid_square_parameters.pixel_size,
)
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 196 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L196

Added line #L196 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created GridSquare {record.gridSquareId}")
Expand All @@ -204,7 +210,7 @@
self, grid_square_id: int, grid_square_parameters: GridSquareParameters
):
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 213 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L213

Added line #L213 was not covered by tests
grid_square = (
db.query(GridSquare)
.filter(GridSquare.gridSquareId == grid_square_id)
Expand Down Expand Up @@ -295,7 +301,7 @@
pixelSize=foil_hole_parameters.pixel_size,
)
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 304 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L304

Added line #L304 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created FoilHole {record.foilHoleId}")
Expand All @@ -315,7 +321,7 @@
foil_hole_parameters: FoilHoleParameters,
):
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 324 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L324

Added line #L324 was not covered by tests
foil_hole = (
db.query(FoilHole).filter(FoilHole.foilHoleId == foil_hole_id).one()
)
Expand Down Expand Up @@ -373,7 +379,7 @@
else "Created for Murfey"
)
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 382 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L382

Added line #L382 was not covered by tests
record.comments = comment
db.add(record)
db.commit()
Expand All @@ -389,7 +395,7 @@

def do_insert_sample_group(self, record: BLSampleGroup) -> dict:
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 398 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L398

Added line #L398 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created BLSampleGroup {record.blSampleGroupId}")
Expand All @@ -404,7 +410,7 @@

def do_insert_sample(self, record: BLSample, sample_group_id: int) -> dict:
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 413 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L413

Added line #L413 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created BLSample {record.blSampleId}")
Expand All @@ -427,7 +433,7 @@

def do_insert_subsample(self, record: BLSubSample) -> dict:
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 436 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L436

Added line #L436 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created BLSubSample {record.blSubSampleId}")
Expand All @@ -442,7 +448,7 @@

def do_insert_sample_image(self, record: BLSampleImage) -> dict:
try:
with Session() as db:
with ISPyBSession() as db:

Check warning on line 451 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L451

Added line #L451 was not covered by tests
db.add(record)
db.commit()
log.info(f"Created BLSampleImage {record.blSampleImageId}")
Expand Down Expand Up @@ -525,7 +531,7 @@
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:

Check warning on line 534 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L534

Added line #L534 was not covered by tests
buffer_objects = (
db.query(ZcZocaloBuffer)
.filter_by(AutoProcProgramID=app_id, UUID=uuid)
Expand All @@ -536,8 +542,8 @@
return reference


def _get_session() -> Generator[Optional[sqlalchemy.orm.Session], None, None]:
db = Session()
def _get_session() -> Generator[Optional[Session], None, None]:
db = ISPyBSession()

Check warning on line 546 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L546

Added line #L546 was not covered by tests
if db is None:
yield None
return
Expand All @@ -556,7 +562,7 @@
proposal_code: str,
proposal_number: str,
visit_number: str,
db: sqlalchemy.orm.Session | None,
db: Session | None,
) -> int | None:

# Log received lookup parameters
Expand Down Expand Up @@ -589,9 +595,7 @@
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(
Expand All @@ -603,7 +607,7 @@
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)
Expand All @@ -628,10 +632,9 @@
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")

Check warning on line 637 in src/murfey/server/ispyb.py

View check run for this annotation

Codecov / codecov/patch

src/murfey/server/ispyb.py#L637

Added line #L637 was not covered by tests
return []
query = (
db.query(BLSession)
Expand Down Expand Up @@ -668,7 +671,7 @@

def get_data_collection_group_ids(session_id):
query = (
Session()
ISPyBSession()
.query(DataCollectionGroup)
.filter(
DataCollectionGroup.sessionId == session_id,
Expand Down
4 changes: 2 additions & 2 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
4 changes: 1 addition & 3 deletions tests/cli/test_repost_failed_calls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)

Expand Down
Loading