Skip to content

Doppio live processing requirements - #790

Open
stephen-riggs wants to merge 29 commits into
mainfrom
doppio-live-processing
Open

Doppio live processing requirements#790
stephen-riggs wants to merge 29 commits into
mainfrom
doppio-live-processing

Conversation

@stephen-riggs

@stephen-riggs stephen-riggs commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Various changes are needed for doppio-live to work.

These are primarily connected to the job number assigned, as we need to be able to vary these if the default pipeline file is being changed by a user. For eBIC operations, the behaviour should be as before, but for doppio-live the jobs are assigned properly using ccpem-pipeliner.

Changes are also needed for the output motion corrected mrc file. The output path for doppio uses the pipeline star file to find a suitable job number. It currently does not include the GridSquare folder so may need changing again in future. The behaviour at eBIC should be unchanged.

Other bits:

  • Removes the picker_murfey_id from the database tables
  • Standardise to import murfey.server and not the _transport_object directly
  • Typing hints

Comment thread src/murfey/util/processing_params.py Dismissed
Comment thread src/murfey/util/processing_params.py Fixed
Comment thread src/murfey/util/processing_params.py Fixed
Comment thread src/murfey/server/api/workflow.py Fixed
Comment thread src/murfey/util/processing_params.py Fixed
Comment thread src/murfey/util/processing_params.py Fixed
Comment thread src/murfey/util/processing_params.py Fixed
Comment thread src/murfey/util/processing_params.py Dismissed
Comment thread src/murfey/util/processing_params.py Dismissed
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 38.67596% with 176 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.67%. Comparing base (e1ae7ee) to head (b821126).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #790      +/-   ##
==========================================
- Coverage   54.71%   54.67%   -0.04%     
==========================================
  Files         104      104              
  Lines       11185    11262      +77     
  Branches     1481     1485       +4     
==========================================
+ Hits         6120     6158      +38     
- Misses       4740     4774      +34     
- Partials      325      330       +5     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stephen-riggs
stephen-riggs marked this pull request as ready for review August 14, 2026 15:18
@Koldstream

Copy link
Copy Markdown
Collaborator

I've had a good look over and I feel pretty happy with it! I had forgotten how many changes had been made overall.

As discussed on Friday the logic for the paths in processing_params.py will need looking at, and further testing for the job reservation system... I'll have another look at this soon.

Comment on lines +82 to +83
core = Path("/") / Path(*parts[: visit_idx + 1])
ppath = Path("/") / Path(*parts)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the limited testing I've done, it looks like Path(*parts...) will give you the same result as Path("/") / Path(*parts...), so this could actually be simplified.

Comment on lines +512 to +521
if murfey.server._transport_object:
murfey.server._transport_object.send(
murfey.server._transport_object.feedback_queue, zocalo_message
)
else:
logger.error(
f"Pre-processing was requested for {sanitise(str(session_id))} "
"but no Zocalo transport object was found"
)
return proc_params

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading this correctly, it will return None when successful and proc_params if unsucessful. Is this intended behaviour?

Comment thread src/murfey/server/api/workflow.py Outdated
session_id: MurfeySessionID,
dcg_params: DCGroupParameters,
db=murfey_db,
db: Session = murfey_db,

@tieneupin tieneupin Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You appear to be using murfey.util.db.Session as the type hint, which is an SQLModel table. I'm not sure how the tests have passed, but this should be pointing to sqlmodel.Session instead.

This would be where I'd usually alias the SQLModel Session and Murfey DB Session so that they are clearly distinguishable.

e.g.

from sqlmodel import Session as SQLModelSession

import murfey.util.db as MurfeyDB

Comment thread src/murfey/server/api/session_info.py Outdated

@router.get("/sessions/{session_id}/rsyncers", response_model=List[RsyncInstance])
def get_rsyncers_for_client(session_id: MurfeySessionID, db=murfey_db):
def get_rsyncers_for_client(session_id: MurfeySessionID, db: Session = murfey_db):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same error in this module. Session is referring to murfey.util.db.Session, which is an SQLModel table. The type hint should be pointing to sqlmodel.Session.

SPARelionParameters.pj_id == collected_ids[2].id
)
).one()
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an SQLModel table, so it's apparently possible to use table.model_dump() to convert this into a dictionary structure.

proc_params = db.exec(
    select(SPARelionParameters).where(
        SPARelionParameters.pj_id == collected_ids[2].id
    )
).one_or_none()
proc_params_dict = proc_params.model_dump() if proc_params else None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants