From 664489cb3062429b786a5d79f6ea4a376c301dd5 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Wed, 22 Jan 2025 16:51:42 +0000 Subject: [PATCH 01/18] Flush functions need renaming --- src/murfey/server/api/__init__.py | 2 +- src/murfey/workflows/spa/flush_spa_preprocess.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/murfey/server/api/__init__.py b/src/murfey/server/api/__init__.py index 4873f81ef..7fa7048d1 100644 --- a/src/murfey/server/api/__init__.py +++ b/src/murfey/server/api/__init__.py @@ -579,7 +579,7 @@ def flush_spa_processing( visit_name: str, session_id: MurfeySessionID, tag: Tag, db=murfey_db ): zocalo_message = { - "register": "flush_spa_preprocess", + "register": "spa.flush_spa_preprocess", "session_id": session_id, "tag": tag.tag, } diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index f2d68b36c..451c25c27 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -281,7 +281,7 @@ def _flush_position_analysis( return foil_hole -def flush_spa_preprocessing(message: dict, db: Session, demo: bool = False): +def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): session_id = message["session_id"] stashed_files = murfey_db.exec( select(db.PreprocessStash) From 3ea7b6679414874617e31fe1ee8b8d37af7d046a Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Wed, 22 Jan 2025 16:54:15 +0000 Subject: [PATCH 02/18] Only look in gridsquare dm files --- src/murfey/client/contexts/spa_metadata.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/murfey/client/contexts/spa_metadata.py b/src/murfey/client/contexts/spa_metadata.py index d049c2a7a..5e99a7b7a 100644 --- a/src/murfey/client/contexts/spa_metadata.py +++ b/src/murfey/client/contexts/spa_metadata.py @@ -181,7 +181,11 @@ def post_transfer( }, ) - elif transferred_file.suffix == ".dm" and environment: + elif ( + transferred_file.suffix == ".dm" + and transferred_file.name.startswith("GridSquare") + and environment + ): gs_name = transferred_file.stem.split("_")[1] fh_positions = _foil_hole_positions(transferred_file, int(gs_name)) source = _get_source(transferred_file, environment=environment) From 8fac8f01bea99f2a211e73c76e8df2a9cf1c98e0 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Wed, 22 Jan 2025 17:12:28 +0000 Subject: [PATCH 03/18] Don't import murfey_db --- .../workflows/spa/flush_spa_preprocess.py | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index 451c25c27..444faaba8 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -9,7 +9,6 @@ from murfey.server import _murfey_id, _transport_object, sanitise from murfey.server.api.auth import MurfeySessionID -from murfey.server.murfey_db import murfey_db from murfey.util.config import get_machine_config, get_microscope from murfey.util.db import DataCollectionGroup, FoilHole, GridSquare from murfey.util.models import FoilHoleParameters, GridSquareParameters @@ -30,7 +29,7 @@ def register_grid_square( session_id: MurfeySessionID, gsid: int, grid_square_params: GridSquareParameters, - db=murfey_db, + db: Session, ): try: grid_square = db.exec( @@ -99,7 +98,7 @@ def register_foil_hole( session_id: MurfeySessionID, gs_name: int, foil_hole_params: FoilHoleParameters, - db=murfey_db, + db: Session, ): try: gs = db.exec( @@ -201,7 +200,7 @@ def _flush_position_analysis( movie_path: Path, dcg_id: int, session_id: int, db: Session ) -> Optional[int]: """Register a grid square and foil hole in the database""" - data_collection_group = murfey_db.exec( + data_collection_group = db.exec( select(db.DataCollectionGroup).where(db.DataCollectionGroup.id == dcg_id) ).one() @@ -246,7 +245,7 @@ def _flush_position_analysis( image=gs.image, ) # Insert or update this grid square in the database - register_grid_square(session_id, gs.id, grid_square_parameters, murfey_db) + register_grid_square(session_id, gs.id, grid_square_parameters, db) # Find the foil hole info and register it foil_hole = foil_hole_from_file(movie_path) @@ -277,13 +276,13 @@ def _flush_position_analysis( name=foil_hole, ) # Insert or update this foil hole in the database - register_foil_hole(session_id, gs.id, foil_hole_parameters, murfey_db) + register_foil_hole(session_id, gs.id, foil_hole_parameters, db) return foil_hole def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): session_id = message["session_id"] - stashed_files = murfey_db.exec( + stashed_files = db.exec( select(db.PreprocessStash) .where(db.PreprocessStash.session_id == session_id) .where(db.PreprocessStash.tag == message["tag"]) @@ -291,7 +290,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): if not stashed_files: return None instrument_name = ( - murfey_db.exec(select(db.Session).where(db.Session.id == message["session_id"])) + db.exec(select(db.Session).where(db.Session.id == message["session_id"])) .one() .instrument_name ) @@ -299,7 +298,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): instrument_name ] recipe_name = machine_config.recipes.get("em-spa-preprocess", "em-spa-preprocess") - collected_ids = murfey_db.exec( + collected_ids = db.exec( select( db.DataCollectionGroup, db.DataCollection, @@ -313,7 +312,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): .where(db.AutoProcProgram.pj_id == db.ProcessingJob.id) .where(db.ProcessingJob.recipe == recipe_name) ).one() - params = murfey_db.exec( + params = db.exec( select(db.SPARelionParameters, db.SPAFeedbackParameters) .where(db.SPARelionParameters.pj_id == collected_ids[2].id) .where(db.SPAFeedbackParameters.pj_id == db.SPARelionParameters.pj_id) @@ -330,13 +329,13 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): murfey_ids = _murfey_id( collected_ids[3].id, - murfey_db, + db, number=2 * len(stashed_files), close=False, ) if feedback_params.picker_murfey_id is None: feedback_params.picker_murfey_id = murfey_ids[1] - murfey_db.add(feedback_params) + db.add(feedback_params) for i, f in enumerate(stashed_files): if f.foil_hole_id: @@ -368,7 +367,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): tag=f.tag, foil_hole_id=foil_hole_id, ) - murfey_db.add(movie) + db.add(movie) zocalo_message: dict = { "recipes": [recipe_name], "parameters": { @@ -400,11 +399,11 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): _transport_object.send( "processing_recipe", zocalo_message, new_connection=True ) - murfey_db.delete(f) + db.delete(f) else: logger.error( f"Pre-processing was requested for {ppath.name} but no Zocalo transport object was found" ) - murfey_db.commit() - murfey_db.close() + db.commit() + db.close() return None From dbd6ed7609ef421742ee0044365b24d0b91d6559 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Wed, 22 Jan 2025 17:23:49 +0000 Subject: [PATCH 04/18] Need to import db models individually --- .../workflows/spa/flush_spa_preprocess.py | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index 444faaba8..996d48175 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -10,7 +10,18 @@ from murfey.server import _murfey_id, _transport_object, sanitise from murfey.server.api.auth import MurfeySessionID from murfey.util.config import get_machine_config, get_microscope -from murfey.util.db import DataCollectionGroup, FoilHole, GridSquare +from murfey.util.db import ( + AutoProcProgram, + DataCollection, + DataCollectionGroup, + FoilHole, + GridSquare, + Movie, + PreprocessStash, + ProcessingJob, +) +from murfey.util.db import Session as MurfeySession +from murfey.util.db import SPAFeedbackParameters, SPARelionParameters from murfey.util.models import FoilHoleParameters, GridSquareParameters from murfey.util.processing_params import default_spa_parameters from murfey.util.spa_metadata import ( @@ -201,7 +212,7 @@ def _flush_position_analysis( ) -> Optional[int]: """Register a grid square and foil hole in the database""" data_collection_group = db.exec( - select(db.DataCollectionGroup).where(db.DataCollectionGroup.id == dcg_id) + select(DataCollectionGroup).where(DataCollectionGroup.id == dcg_id) ).one() # Work out the grid square and associated metadata file @@ -283,14 +294,14 @@ def _flush_position_analysis( def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): session_id = message["session_id"] stashed_files = db.exec( - select(db.PreprocessStash) - .where(db.PreprocessStash.session_id == session_id) - .where(db.PreprocessStash.tag == message["tag"]) + select(PreprocessStash) + .where(PreprocessStash.session_id == session_id) + .where(PreprocessStash.tag == message["tag"]) ).all() if not stashed_files: return None instrument_name = ( - db.exec(select(db.Session).where(db.Session.id == message["session_id"])) + db.exec(select(MurfeySession).where(MurfeySession.id == message["session_id"])) .one() .instrument_name ) @@ -300,22 +311,22 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): recipe_name = machine_config.recipes.get("em-spa-preprocess", "em-spa-preprocess") collected_ids = db.exec( select( - db.DataCollectionGroup, - db.DataCollection, - db.ProcessingJob, - db.AutoProcProgram, + DataCollectionGroup, + DataCollection, + ProcessingJob, + AutoProcProgram, ) - .where(db.DataCollectionGroup.session_id == session_id) - .where(db.DataCollectionGroup.tag == message["tag"]) - .where(db.DataCollection.dcg_id == db.DataCollectionGroup.id) - .where(db.ProcessingJob.dc_id == db.DataCollection.id) - .where(db.AutoProcProgram.pj_id == db.ProcessingJob.id) - .where(db.ProcessingJob.recipe == recipe_name) + .where(DataCollectionGroup.session_id == session_id) + .where(DataCollectionGroup.tag == message["tag"]) + .where(DataCollection.dcg_id == DataCollectionGroup.id) + .where(ProcessingJob.dc_id == DataCollection.id) + .where(AutoProcProgram.pj_id == ProcessingJob.id) + .where(ProcessingJob.recipe == recipe_name) ).one() params = db.exec( - select(db.SPARelionParameters, db.SPAFeedbackParameters) - .where(db.SPARelionParameters.pj_id == collected_ids[2].id) - .where(db.SPAFeedbackParameters.pj_id == db.SPARelionParameters.pj_id) + select(SPARelionParameters, SPAFeedbackParameters) + .where(SPARelionParameters.pj_id == collected_ids[2].id) + .where(SPAFeedbackParameters.pj_id == SPARelionParameters.pj_id) ).one() proc_params = params[0] feedback_params = params[1] @@ -360,7 +371,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): ppath = Path(f.file_path) if not mrcp.parent.exists(): mrcp.parent.mkdir(parents=True) - movie = db.Movie( + movie = Movie( murfey_id=murfey_ids[2 * i], path=f.file_path, image_number=f.image_number, From 7ba1d9de4e4a361f72f83795990f25b30ee845bf Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Thu, 23 Jan 2025 08:59:36 +0000 Subject: [PATCH 05/18] Workflow should return a boolean --- src/murfey/server/__init__.py | 3 +++ src/murfey/workflows/spa/flush_spa_preprocess.py | 10 ++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/murfey/server/__init__.py b/src/murfey/server/__init__.py index 39786ff2f..431385508 100644 --- a/src/murfey/server/__init__.py +++ b/src/murfey/server/__init__.py @@ -2948,7 +2948,10 @@ def feedback_callback(header: dict, message: dict) -> None: else: # Send it directly to DLQ without trying to rerun it _transport_object.transport.nack(header, requeue=False) + if not result: + logger.error(f"Workflow {message['register']} returned {result}") return None + logger.error(f"No workflow found for {message['register']}") if _transport_object: _transport_object.transport.nack(header, requeue=False) return None diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index 996d48175..03fc93802 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -291,7 +291,7 @@ def _flush_position_analysis( return foil_hole -def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): +def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool: session_id = message["session_id"] stashed_files = db.exec( select(PreprocessStash) @@ -299,7 +299,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): .where(PreprocessStash.tag == message["tag"]) ).all() if not stashed_files: - return None + return True instrument_name = ( db.exec(select(MurfeySession).where(MurfeySession.id == message["session_id"])) .one() @@ -334,9 +334,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): logger.warning( f"No SPA processing parameters found for client processing job ID {collected_ids[2].id}" ) - raise ValueError( - "No processing parameters were found in the database when flushing SPA preprocessing" - ) + return False murfey_ids = _murfey_id( collected_ids[3].id, @@ -417,4 +415,4 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False): ) db.commit() db.close() - return None + return True From 953f2fb6ca93ac7a2ca676e7aac18dff76fd4a44 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Thu, 23 Jan 2025 09:07:07 +0000 Subject: [PATCH 06/18] switch db to murfey_db --- .../workflows/spa/flush_spa_preprocess.py | 58 ++++++++++--------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index 03fc93802..e7f70e8bf 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -40,10 +40,10 @@ def register_grid_square( session_id: MurfeySessionID, gsid: int, grid_square_params: GridSquareParameters, - db: Session, + murfey_db: Session, ): try: - grid_square = db.exec( + grid_square = murfey_db.exec( select(GridSquare) .where(GridSquare.name == gsid) .where(GridSquare.tag == grid_square_params.tag) @@ -61,7 +61,7 @@ def register_grid_square( _transport_object.do_update_grid_square(grid_square.id, grid_square_params) except Exception: if _transport_object: - dcg = db.exec( + dcg = murfey_db.exec( select(DataCollectionGroup) .where(DataCollectionGroup.session_id == session_id) .where(DataCollectionGroup.tag == grid_square_params.tag) @@ -100,19 +100,19 @@ def register_grid_square( pixel_size=grid_square_params.pixel_size, image=secured_grid_square_image_path, ) - db.add(grid_square) - db.commit() - db.close() + murfey_db.add(grid_square) + murfey_db.commit() + murfey_db.close() def register_foil_hole( session_id: MurfeySessionID, gs_name: int, foil_hole_params: FoilHoleParameters, - db: Session, + murfey_db: Session, ): try: - gs = db.exec( + gs = murfey_db.exec( select(GridSquare) .where(GridSquare.tag == foil_hole_params.tag) .where(GridSquare.session_id == session_id) @@ -130,7 +130,7 @@ def register_foil_hole( else: jpeg_size = (0, 0) try: - foil_hole = db.exec( + foil_hole = murfey_db.exec( select(FoilHole) .where(FoilHole.name == foil_hole_params.name) .where(FoilHole.grid_square_id == gsid) @@ -190,9 +190,9 @@ def register_foil_hole( pixel_size=foil_hole_params.pixel_size, image=secured_foil_hole_image_path, ) - db.add(foil_hole) - db.commit() - db.close() + murfey_db.add(foil_hole) + murfey_db.commit() + murfey_db.close() def _grid_square_metadata_file(f: Path, grid_square: int) -> Optional[Path]: @@ -208,10 +208,10 @@ def _grid_square_metadata_file(f: Path, grid_square: int) -> Optional[Path]: def _flush_position_analysis( - movie_path: Path, dcg_id: int, session_id: int, db: Session + movie_path: Path, dcg_id: int, session_id: int, murfey_db: Session ) -> Optional[int]: """Register a grid square and foil hole in the database""" - data_collection_group = db.exec( + data_collection_group = murfey_db.exec( select(DataCollectionGroup).where(DataCollectionGroup.id == dcg_id) ).one() @@ -256,7 +256,7 @@ def _flush_position_analysis( image=gs.image, ) # Insert or update this grid square in the database - register_grid_square(session_id, gs.id, grid_square_parameters, db) + register_grid_square(session_id, gs.id, grid_square_parameters, murfey_db) # Find the foil hole info and register it foil_hole = foil_hole_from_file(movie_path) @@ -287,13 +287,13 @@ def _flush_position_analysis( name=foil_hole, ) # Insert or update this foil hole in the database - register_foil_hole(session_id, gs.id, foil_hole_parameters, db) + register_foil_hole(session_id, gs.id, foil_hole_parameters, murfey_db) return foil_hole -def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool: +def flush_spa_preprocess(message: dict, murfey_db: Session, demo: bool = False) -> bool: session_id = message["session_id"] - stashed_files = db.exec( + stashed_files = murfey_db.exec( select(PreprocessStash) .where(PreprocessStash.session_id == session_id) .where(PreprocessStash.tag == message["tag"]) @@ -301,7 +301,9 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool if not stashed_files: return True instrument_name = ( - db.exec(select(MurfeySession).where(MurfeySession.id == message["session_id"])) + murfey_db.exec( + select(MurfeySession).where(MurfeySession.id == message["session_id"]) + ) .one() .instrument_name ) @@ -309,7 +311,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool instrument_name ] recipe_name = machine_config.recipes.get("em-spa-preprocess", "em-spa-preprocess") - collected_ids = db.exec( + collected_ids = murfey_db.exec( select( DataCollectionGroup, DataCollection, @@ -323,7 +325,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool .where(AutoProcProgram.pj_id == ProcessingJob.id) .where(ProcessingJob.recipe == recipe_name) ).one() - params = db.exec( + params = murfey_db.exec( select(SPARelionParameters, SPAFeedbackParameters) .where(SPARelionParameters.pj_id == collected_ids[2].id) .where(SPAFeedbackParameters.pj_id == SPARelionParameters.pj_id) @@ -338,13 +340,13 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool murfey_ids = _murfey_id( collected_ids[3].id, - db, + murfey_db, number=2 * len(stashed_files), close=False, ) if feedback_params.picker_murfey_id is None: feedback_params.picker_murfey_id = murfey_ids[1] - db.add(feedback_params) + murfey_db.add(feedback_params) for i, f in enumerate(stashed_files): if f.foil_hole_id: @@ -356,7 +358,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool movie_path=f.file_path, dcg_id=collected_ids[0].id, session_id=session_id, - db=db, + murfey_db=murfey_db, ) except Exception as e: logger.error( @@ -376,7 +378,7 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool tag=f.tag, foil_hole_id=foil_hole_id, ) - db.add(movie) + murfey_db.add(movie) zocalo_message: dict = { "recipes": [recipe_name], "parameters": { @@ -408,11 +410,11 @@ def flush_spa_preprocess(message: dict, db: Session, demo: bool = False) -> bool _transport_object.send( "processing_recipe", zocalo_message, new_connection=True ) - db.delete(f) + murfey_db.delete(f) else: logger.error( f"Pre-processing was requested for {ppath.name} but no Zocalo transport object was found" ) - db.commit() - db.close() + murfey_db.commit() + murfey_db.close() return True From 632285cc5e49cf10024e9c27387cf7662e8d8ea4 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Thu, 23 Jan 2025 11:05:26 +0000 Subject: [PATCH 07/18] Sanitise the logs --- src/murfey/server/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/murfey/server/__init__.py b/src/murfey/server/__init__.py index 431385508..11c4b3b35 100644 --- a/src/murfey/server/__init__.py +++ b/src/murfey/server/__init__.py @@ -2949,9 +2949,11 @@ def feedback_callback(header: dict, message: dict) -> None: # Send it directly to DLQ without trying to rerun it _transport_object.transport.nack(header, requeue=False) if not result: - logger.error(f"Workflow {message['register']} returned {result}") + logger.error( + f"Workflow {sanitise(message['register'])} returned {result}" + ) return None - logger.error(f"No workflow found for {message['register']}") + logger.error(f"No workflow found for {sanitise(message['register'])}") if _transport_object: _transport_object.transport.nack(header, requeue=False) return None From 57426366ea04aee7a021f1f2e42dcf336f3de909 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 09:34:23 +0000 Subject: [PATCH 08/18] Attempt to make postgres in actions --- .github/workflows/ci.yml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 02eb3b6f2..263aa832d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,16 @@ jobs: - 3306:3306 options: --health-cmd="healthcheck.sh --connect --innodb_initialized" --health-interval=10s --health-timeout=5s --health-retries=3 + postgres: + image: postgres:latest + env: + POSTGRES_DB: murfey_test_db + POSTGRES_PASSWORD: psql_pwd + POSTGRES_USER: psql_user + ports: + 5432: 5432 + options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 + steps: - uses: actions/checkout@v4 - name: Use Python ${{ matrix.python-version }} @@ -74,7 +84,7 @@ jobs: mysql-version: "11.3" auto-start: false - - name: Set up test database + - name: Set up test ipsyb database run: | set -eu cp ".github/workflows/config/my.cnf" .my.cnf @@ -99,10 +109,22 @@ jobs: 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@'%';" rm .my.cnf + - name: Connect to PostgreSQL + run: node client.js + env: + POSTGRES_HOST: postgres + POSTGRES_PORT: 5432 + - name: Check RabbitMQ is alive run: wget -t 10 -w 1 http://127.0.0.1:15672 -O - - name: Run tests + env: + POSTGRES_HOST: postgres + 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 From 14d1cf23ff48da4a831cc7294ec3c97482267ccd Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 09:35:04 +0000 Subject: [PATCH 09/18] Tests for registering grid squares --- .../spa/test_flush_spa_preprocess.py | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 tests/workflows/spa/test_flush_spa_preprocess.py diff --git a/tests/workflows/spa/test_flush_spa_preprocess.py b/tests/workflows/spa/test_flush_spa_preprocess.py new file mode 100644 index 000000000..5ffbbdb1b --- /dev/null +++ b/tests/workflows/spa/test_flush_spa_preprocess.py @@ -0,0 +1,166 @@ +import os +from unittest import mock + +import pytest +from sqlalchemy import Session, create_engine, select + +from murfey.util.db import DataCollectionGroup, GridSquare, clear, setup +from murfey.util.models import GridSquareParameters +from murfey.workflows.spa import flush_spa_preprocess + +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']}" +) + + +@pytest.fixture +def start_postgres(): + clear(url) + setup(url) + + +engine = create_engine(url) + + +@mock.patch("murfey.workflows.spa.flush_spa_preprocess._transport_object") +def test_register_grid_square_update_add_locations(mock_transport, start_postgres): + """Test the updating of an existing grid square""" + # Create a grid square to update + grid_square = GridSquare( + id=1, + name=101, + session_id=2, + tag="session_tag", + ) + with Session(engine) as murfey_db: + murfey_db.add(grid_square) + murfey_db.commit() + + # Parameters to update with + new_parameters = GridSquareParameters( + tag="session_tag", + x_location=1.1, + y_location=1.2, + x_stage_position=1.3, + y_stage_position=1.4, + ) + + # Run the registration + with Session(engine) as murfey_db: + flush_spa_preprocess.register_grid_square(2, 101, new_parameters, murfey_db) + + # Check this would have updated ispyb + mock_transport.do_update_grid_square.assert_called_with(101, new_parameters) + + # Confirm the database was updated + with Session(engine) as murfey_db: + grid_square_final_parameters = murfey_db.exec(select(GridSquare)).one() + assert grid_square_final_parameters.x_location == new_parameters.x_location + assert grid_square_final_parameters.y_location == new_parameters.y_location + assert ( + grid_square_final_parameters.x_stage_position == new_parameters.x_stage_position + ) + assert ( + grid_square_final_parameters.y_stage_position == new_parameters.y_stage_position + ) + + +@mock.patch("murfey.workflows.spa.flush_spa_preprocess._transport_object") +def test_register_grid_square_update_add_nothing(mock_transport, start_postgres): + """Test the updating of an existing grid square, but with nothing to update with""" + # Create a grid square to update + grid_square = GridSquare( + id=1, + name=101, + session_id=2, + tag="session_tag", + x_location=0.1, + y_location=0.2, + x_stage_location=0.3, + y_stage_location=0.4, + ) + with Session(engine) as murfey_db: + murfey_db.add(grid_square) + murfey_db.commit() + + # Parameters to update with + new_parameters = GridSquareParameters(tag="session_tag") + + # Run the registration + with Session(engine) as murfey_db: + flush_spa_preprocess.register_grid_square(2, 101, new_parameters, murfey_db) + + # Check this would have updated ispyb + mock_transport.do_update_grid_square.assert_called_with(101, new_parameters) + + # Confirm the database was not updated + with Session(engine) as murfey_db: + grid_square_final_parameters = murfey_db.exec(select(GridSquare)).one() + assert grid_square_final_parameters.x_location == grid_square.x_location + assert grid_square_final_parameters.y_location == grid_square.y_location + assert grid_square_final_parameters.x_stage_position == grid_square.x_stage_position + assert grid_square_final_parameters.y_stage_position == grid_square.y_stage_position + + +@mock.patch("murfey.workflows.spa.flush_spa_preprocess._transport_object") +def test_register_grid_square_insert_with_ispyb( + mock_transport, start_postgres, tmp_path +): + # Create a data collection group for lookups + grid_square = DataCollectionGroup( + id=1, + session_id=2, + tag="session_tag", + atlas_id=90, + ) + with Session(engine) as murfey_db: + murfey_db.add(grid_square) + murfey_db.commit() + + # Set the ispyb return + mock_transport.do_insert_grid_square.return_value = { + "return_value": 1, + "success": True, + } + + # Parameters to update with + new_parameters = GridSquareParameters( + tag="session_tag", + x_location=1.1, + y_location=1.2, + x_stage_position=1.3, + y_stage_position=1.4, + readout_area_x=2048, + readout_area_y=1024, + thumbnail_size_x=512, + thumbnail_size_y=256, + pixel_size=1.02, + image=f"{tmp_path}/image_path", + angle=12.5, + ) + + # Run the registration + with Session(engine) as murfey_db: + flush_spa_preprocess.register_grid_square(2, 101, new_parameters, murfey_db) + + # Check this would have updated ispyb + mock_transport.do_insert_grid_square.assert_called_with(90, 101, new_parameters) + + # Confirm the database entry was made + with Session(engine) as murfey_db: + grid_square_final_parameters = murfey_db.exec(select(GridSquare)).one() + assert grid_square_final_parameters.id == 1 + assert grid_square_final_parameters.name == 101 + assert grid_square_final_parameters.session_id == 2 + assert grid_square_final_parameters.tag == "session_tag" + assert grid_square_final_parameters.x_location == 1.1 + assert grid_square_final_parameters.y_location == 1.2 + assert grid_square_final_parameters.x_stage_position == 1.3 + assert grid_square_final_parameters.y_stage_position == 1.4 + assert grid_square_final_parameters.readout_area_x == 2048 + assert grid_square_final_parameters.readout_area_y == 1024 + assert grid_square_final_parameters.thumbnail_size_x == 512 + assert grid_square_final_parameters.thumbnail_size_y == 256 + assert grid_square_final_parameters.pixel_size == 1.02 + assert grid_square_final_parameters.image == f"{tmp_path}/image_path" From 2adccecd0458dde77cad1a1fd5dd60a302352d06 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 09:40:01 +0000 Subject: [PATCH 10/18] Fix port mapping --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 263aa832d..f5ea3c3ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: POSTGRES_PASSWORD: psql_pwd POSTGRES_USER: psql_user ports: - 5432: 5432 + - 5432:5432 options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 steps: From d17ff5ee4c2461603275e50b777ab7ce97d571fb Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 09:43:40 +0000 Subject: [PATCH 11/18] Some wrong bits --- .github/workflows/ci.yml | 8 +------- tests/workflows/spa/test_flush_spa_preprocess.py | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5ea3c3ba..6ac2fb8b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,7 +67,7 @@ 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 database + - name: Get ispyb database uses: actions/download-artifact@v4 with: name: database @@ -109,12 +109,6 @@ jobs: 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@'%';" rm .my.cnf - - name: Connect to PostgreSQL - run: node client.js - env: - POSTGRES_HOST: postgres - POSTGRES_PORT: 5432 - - name: Check RabbitMQ is alive run: wget -t 10 -w 1 http://127.0.0.1:15672 -O - diff --git a/tests/workflows/spa/test_flush_spa_preprocess.py b/tests/workflows/spa/test_flush_spa_preprocess.py index 5ffbbdb1b..914409103 100644 --- a/tests/workflows/spa/test_flush_spa_preprocess.py +++ b/tests/workflows/spa/test_flush_spa_preprocess.py @@ -9,7 +9,7 @@ from murfey.workflows.spa import flush_spa_preprocess url = ( - f"postgresql+psycopg2://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" + f"postgresql://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" ) From 1799b5343199e4302d282c3b2df0e2bcb8cc150c Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 09:52:11 +0000 Subject: [PATCH 12/18] Create conftest for common fixtures --- tests/__init__.py | 10 +++++++++ tests/conftest.py | 10 +++++++++ .../spa/test_flush_spa_preprocess.py | 21 +++---------------- 3 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 000000000..a35807396 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,10 @@ +import os + +from sqlmodel import create_engine + +url = ( + f"postgresql://{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) diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 000000000..02f0edcca --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,10 @@ +import pytest + +from murfey.util.db import clear, setup +from tests import url + + +@pytest.fixture +def start_postgres(): + clear(url) + setup(url) diff --git a/tests/workflows/spa/test_flush_spa_preprocess.py b/tests/workflows/spa/test_flush_spa_preprocess.py index 914409103..cddd10676 100644 --- a/tests/workflows/spa/test_flush_spa_preprocess.py +++ b/tests/workflows/spa/test_flush_spa_preprocess.py @@ -1,26 +1,11 @@ -import os from unittest import mock -import pytest -from sqlalchemy import Session, create_engine, select +from sqlmodel import Session, select -from murfey.util.db import DataCollectionGroup, GridSquare, clear, setup +from murfey.util.db import DataCollectionGroup, GridSquare from murfey.util.models import GridSquareParameters from murfey.workflows.spa import flush_spa_preprocess - -url = ( - f"postgresql://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" - f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" -) - - -@pytest.fixture -def start_postgres(): - clear(url) - setup(url) - - -engine = create_engine(url) +from tests import engine @mock.patch("murfey.workflows.spa.flush_spa_preprocess._transport_object") From b246c1a47ab3bc1c59e781eb9730afcfe1010789 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 09:57:29 +0000 Subject: [PATCH 13/18] Might be running on localhost --- .github/workflows/ci.yml | 2 +- tests/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ac2fb8b8..fd6a98ede 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,7 +114,7 @@ jobs: - name: Run tests env: - POSTGRES_HOST: postgres + POSTGRES_HOST: localhost POSTGRES_PORT: 5432 POSTGRES_DB: murfey_test_db POSTGRES_PASSWORD: psql_pwd diff --git a/tests/__init__.py b/tests/__init__.py index a35807396..92085ab06 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -3,7 +3,7 @@ from sqlmodel import create_engine url = ( - f"postgresql://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" + f"postgresql+psycopg2://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" ) From 005d81c09a1cdfe42634d5c02dcebe1c964da56c Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 10:25:38 +0000 Subject: [PATCH 14/18] add session to test db --- tests/conftest.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 02f0edcca..93c00686d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,15 @@ import pytest -from murfey.util.db import clear, setup -from tests import url +from murfey.util.db import Session, clear, setup +from tests import engine, url @pytest.fixture def start_postgres(): clear(url) setup(url) + + murfey_session = Session(id=2, name="cm12345-6") + with Session(engine) as murfey_db: + murfey_db.add(murfey_session) + murfey_db.commit() From 6dfe84703ac21e0f977946bfcffcdb1bfcd010f5 Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 10:30:09 +0000 Subject: [PATCH 15/18] Conflicting session names --- tests/conftest.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 93c00686d..1b87c0217 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,8 @@ import pytest +from sqlmodel import Session -from murfey.util.db import Session, clear, setup +from murfey.util.db import Session as MurfeySession +from murfey.util.db import clear, setup from tests import engine, url @@ -9,7 +11,7 @@ def start_postgres(): clear(url) setup(url) - murfey_session = Session(id=2, name="cm12345-6") + murfey_session = MurfeySession(id=2, name="cm12345-6") with Session(engine) as murfey_db: murfey_db.add(murfey_session) murfey_db.commit() From c82cfbdf0d14153f691f1414140d790c78c6f01d Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 11:33:31 +0000 Subject: [PATCH 16/18] Should secure the path not the name --- .../workflows/spa/flush_spa_preprocess.py | 17 +++++++---------- .../workflows/spa/test_flush_spa_preprocess.py | 4 ++-- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index e7f70e8bf..f8a722b46 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -5,10 +5,10 @@ from PIL import Image from sqlalchemy.exc import NoResultFound from sqlmodel import Session, select -from werkzeug.utils import secure_filename from murfey.server import _murfey_id, _transport_object, sanitise from murfey.server.api.auth import MurfeySessionID +from murfey.util import secure_path from murfey.util.config import get_machine_config, get_microscope from murfey.util.db import ( AutoProcProgram, @@ -72,11 +72,8 @@ def register_grid_square( else: # mock up response so that below still works gs_ispyb_response = {"success": False, "return_value": None} - secured_grid_square_image_path = secure_filename(grid_square_params.image) - if ( - secured_grid_square_image_path - and Path(secured_grid_square_image_path).is_file() - ): + secured_grid_square_image_path = secure_path(Path(grid_square_params.image)) + if secured_grid_square_image_path and secured_grid_square_image_path.is_file(): jpeg_size = Image.open(secured_grid_square_image_path).size else: jpeg_size = (0, 0) @@ -98,7 +95,7 @@ def register_grid_square( thumbnail_size_x=grid_square_params.thumbnail_size_x or jpeg_size[0], thumbnail_size_y=grid_square_params.thumbnail_size_y or jpeg_size[1], pixel_size=grid_square_params.pixel_size, - image=secured_grid_square_image_path, + image=str(secured_grid_square_image_path), ) murfey_db.add(grid_square) murfey_db.commit() @@ -124,8 +121,8 @@ def register_foil_hole( f"Foil hole {sanitise(str(foil_hole_params.name))} could not be registered as grid square {sanitise(str(gs_name))} was not found" ) return - secured_foil_hole_image_path = secure_filename(foil_hole_params.image) - if foil_hole_params.image and Path(secured_foil_hole_image_path).is_file(): + secured_foil_hole_image_path = secure_path(Path(foil_hole_params.image)) + if foil_hole_params.image and secured_foil_hole_image_path.is_file(): jpeg_size = Image.open(secured_foil_hole_image_path).size else: jpeg_size = (0, 0) @@ -188,7 +185,7 @@ def register_foil_hole( thumbnail_size_x=foil_hole_params.thumbnail_size_x or jpeg_size[0], thumbnail_size_y=foil_hole_params.thumbnail_size_y or jpeg_size[1], pixel_size=foil_hole_params.pixel_size, - image=secured_foil_hole_image_path, + image=str(secured_foil_hole_image_path), ) murfey_db.add(foil_hole) murfey_db.commit() diff --git a/tests/workflows/spa/test_flush_spa_preprocess.py b/tests/workflows/spa/test_flush_spa_preprocess.py index cddd10676..ff9f331f4 100644 --- a/tests/workflows/spa/test_flush_spa_preprocess.py +++ b/tests/workflows/spa/test_flush_spa_preprocess.py @@ -36,7 +36,7 @@ def test_register_grid_square_update_add_locations(mock_transport, start_postgre flush_spa_preprocess.register_grid_square(2, 101, new_parameters, murfey_db) # Check this would have updated ispyb - mock_transport.do_update_grid_square.assert_called_with(101, new_parameters) + mock_transport.do_update_grid_square.assert_called_with(1, new_parameters) # Confirm the database was updated with Session(engine) as murfey_db: @@ -77,7 +77,7 @@ def test_register_grid_square_update_add_nothing(mock_transport, start_postgres) flush_spa_preprocess.register_grid_square(2, 101, new_parameters, murfey_db) # Check this would have updated ispyb - mock_transport.do_update_grid_square.assert_called_with(101, new_parameters) + mock_transport.do_update_grid_square.assert_called_with(1, new_parameters) # Confirm the database was not updated with Session(engine) as murfey_db: From 010cf68c0e80400bee540626523a6fdec331bf4a Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 11:42:19 +0000 Subject: [PATCH 17/18] Doesn't like using params from db model --- tests/workflows/spa/test_flush_spa_preprocess.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/workflows/spa/test_flush_spa_preprocess.py b/tests/workflows/spa/test_flush_spa_preprocess.py index ff9f331f4..42cccc31c 100644 --- a/tests/workflows/spa/test_flush_spa_preprocess.py +++ b/tests/workflows/spa/test_flush_spa_preprocess.py @@ -82,10 +82,10 @@ def test_register_grid_square_update_add_nothing(mock_transport, start_postgres) # Confirm the database was not updated with Session(engine) as murfey_db: grid_square_final_parameters = murfey_db.exec(select(GridSquare)).one() - assert grid_square_final_parameters.x_location == grid_square.x_location - assert grid_square_final_parameters.y_location == grid_square.y_location - assert grid_square_final_parameters.x_stage_position == grid_square.x_stage_position - assert grid_square_final_parameters.y_stage_position == grid_square.y_stage_position + assert grid_square_final_parameters.x_location == 0.1 + assert grid_square_final_parameters.y_location == 0.2 + assert grid_square_final_parameters.x_stage_position == 0.3 + assert grid_square_final_parameters.y_stage_position == 0.4 @mock.patch("murfey.workflows.spa.flush_spa_preprocess._transport_object") From e81d16d0caecb17405f74377c373b9a4af7bf18b Mon Sep 17 00:00:00 2001 From: yxd92326 Date: Fri, 24 Jan 2025 11:45:51 +0000 Subject: [PATCH 18/18] Mis-named attributes --- tests/workflows/spa/test_flush_spa_preprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/workflows/spa/test_flush_spa_preprocess.py b/tests/workflows/spa/test_flush_spa_preprocess.py index 42cccc31c..f55f9838c 100644 --- a/tests/workflows/spa/test_flush_spa_preprocess.py +++ b/tests/workflows/spa/test_flush_spa_preprocess.py @@ -62,8 +62,8 @@ def test_register_grid_square_update_add_nothing(mock_transport, start_postgres) tag="session_tag", x_location=0.1, y_location=0.2, - x_stage_location=0.3, - y_stage_location=0.4, + x_stage_position=0.3, + y_stage_position=0.4, ) with Session(engine) as murfey_db: murfey_db.add(grid_square)