Delete old flow cell before adding re-demultiplexed flow cell #2593

diitaz93 · 2023-10-16T10:59:07Z

Description

Part of the tasks in #2541.
In #2144 the automatic deletion of a flow cell from Housekeeper and StatusDB was removed, having to explicitly delete a flow cell before re-demultiplexing.

This PR re-adds the functionality in a slightly different way.

Changed

Removed Delete_flow_cell_API
Removed the cg demultiplex delete-flow-cell CLI command
No longer checks for active cases before removing stuff (Does one not always want to restart with new data?)

Before post-processing:

The sequencing_metrics are removed from status_db
The sequencing files (FASTQ, SPRING, SPRING-metadata) are removed from housekeeper and disk

How to prepare for test

Ssh to relevant server (depending on type of change)
Use stage: us
Paxa the environment: paxa

Install on stage (example for Hasta):

bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_cg -t cg -b delete-flowcell-post-processing -a

How to test

Finish a flow cell in stage: cg demultiplex finish flow-cell 230908_A01901_0231_BHJWJFDSX7
Check how many files contain the flow cell and fastq tag
Add an extra file to one of the samples in housekeeper with the tags [fastq, <flow_cell_id>, <sample_id>]
Change a sequencing_lane_metrics entry.
Finish the flow cell again.

Expected test outcome

The fastq file count is the same as before (the dummy one was removed)
The sequencing lane metric was changed back to the original
Take a screenshot and attach or copy/paste the output.

Review

Tests executed by
"Merge and deploy" approved by
Thanks for filling in who performed the code review and the test!

This version is a

MAJOR - when you make incompatible API changes
MINOR - when you add functionality in a backwards compatible manner
PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

Document in ...
Deploy this branch on ...
Inform to ...

islean · 2023-10-16T11:09:42Z

Nice! Seems to already have support for filtering on the flow cell tag!

ChrOertlin

Looks good. We should check with @karlnyr if this is the behaviour that we want in CG (i.e. deleting previous flow cell entries)

cg/meta/demultiplex/demux_post_processing.py

diitaz93 · 2023-10-16T11:23:01Z

Looks good. We should check with @karlnyr if this is the behaviour that we want in CG (i.e. deleting previous flow cell entries)

Yes, I added this to the next refinement meeting so that @karlnyr can give me some advice on this

Vince-janv

I like having the delete call here rather than before demultiplexing.

Here are some thoughts though:

Simplify delete_demux_api.delete_flow_cell to only perform what we think is useful
Since the call is moved after demultiplexing, it should not remove any directories.
It should not remove the sample sheet
Do we need to check active samples? (more hassle than worth)

cg/meta/demultiplex/delete_demultiplex_api.py

diitaz93 · 2023-10-18T12:23:38Z

I like having the delete call here rather than before demultiplexing.

Here are some thoughts though:

Simplify delete_demux_api.delete_flow_cell to only perform what we think is useful

Wait for input from prod. This function is also called in the CLI command cg demultiplex delete-flow-cell and receives as parameters all of the desired locations for deletion.

Since the call is moved after demultiplexing, it should not remove any directories.

so demultiplexing_dir=False and run_dir=False

It should not remove the sample sheet

This one is hard, we will have to decouple the sample sheet and Housekeeper deletion if I understand correctly

Due to the cascade setting between metrics and flow cell we do not explicitly need to remove metrics.

I guess this is for when you want to delete the metrics but not the flow cell?

Do we need to check active samples? (more hassle than worth)

We do check active samples when we call delete_flow_cell

… into delete-flowcell-post-processing

sonarqubecloud · 2023-10-26T13:27:24Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

# Conflicts: # cg/meta/demultiplex/housekeeper_storage_functions.py # cg/meta/demultiplex/status_db_storage_functions.py

cg/cli/demultiplex/demux.py

diitaz93

I really like the structure of the new delete functions. I am a bit concerned about the order of the commands store_flow_cell_data and delete_flow_cell_data in finish_flow_cell. Also, I am thinking if it would be useful to leave the CLI command for testing purposes, specially when testing the demultiplex process without invoking the finish command

cg/meta/demultiplex/housekeeper_storage_functions.py

cg/meta/demultiplex/demux_post_processing.py

tests/meta/demultiplex/test_housekeeper_storage_functions.py

cg/meta/demultiplex/demux_post_processing.py

cg/cli/demultiplex/demux.py

cg/meta/demultiplex/delete_demultiplex_api.py

karlnyr

Cool beans 🔥

cg/meta/demultiplex/demux_post_processing.py

cg/meta/demultiplex/status_db_storage_functions.py

tests/cli/demultiplex/test_demultiplex_flowcell.py

tests/store_helpers.py

cg/cli/demultiplex/demux.py

sonarqubecloud · 2023-12-08T08:41:45Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

karlnyr

Clip it and ship it. Time for vacation 🌴

Vince-janv · 2023-12-08T09:47:09Z

Before re-finishing

After re-finishing

Vince-janv · 2023-12-08T09:55:26Z

Finishing from scratch (flow cell not added previously) also worked

Vince-janv · 2023-12-08T10:10:00Z

Deployed in stage and production

diitaz93 added 2 commits October 16, 2023 12:53

Add delete flow cell call inside finish flow cell

9dbd52c

Merge branch 'master' into delete-flowcell-post-processing

807d7a3

diitaz93 self-assigned this Oct 16, 2023

ChrOertlin approved these changes Oct 16, 2023

View reviewed changes

cg/meta/demultiplex/demux_post_processing.py Outdated Show resolved Hide resolved

Vince-janv reviewed Oct 17, 2023

View reviewed changes

cg/meta/demultiplex/delete_demultiplex_api.py Outdated Show resolved Hide resolved

diitaz93 added 3 commits October 17, 2023 13:46

Merge branch 'master' into delete-flowcell-post-processing

5422495

Merge branch 'master' into delete-flowcell-post-processing

973cd53

Rename function in deletedemuxAPI

2915b23

diitaz93 and others added 7 commits October 19, 2023 11:41

Merge branch 'master' into delete-flowcell-post-processing

ae8190a

Merge branch 'master' into delete-flowcell-post-processing

7c7c654

Add removal of output dir to demultiplexing command

1341501

Add deletion of flow cell before re-adding it in post-processing

8c031b8

Merge remote-tracking branch 'origin/delete-flowcell-post-processing'…

31dc23f

… into delete-flowcell-post-processing

Moved code to other PR

c4f9a2d

Merge branch 'master' into delete-flowcell-post-processing

7c4db16

karlnyr self-requested a review October 23, 2023 11:45

Merge branch 'master' into delete-flowcell-post-processing

24c6dfd

diitaz93 requested a review from ChrOertlin October 26, 2023 13:40

Vince-janv and others added 6 commits December 6, 2023 15:22

Merge branch 'master' into delete-flowcell-post-processing

4f81572

# Conflicts: # cg/meta/demultiplex/housekeeper_storage_functions.py # cg/meta/demultiplex/status_db_storage_functions.py

Add tests

637dd4a

Removed old delete functionality

0bb74a7

Doc-string

5ae6444

Merge branch 'master' into delete-flowcell-post-processing

c919959

Merge branch 'master' into delete-flowcell-post-processing

c161fbe

Vince-janv marked this pull request as ready for review December 7, 2023 10:52

Vince-janv requested a review from a team as a code owner December 7, 2023 10:52

Vince-janv reviewed Dec 7, 2023

View reviewed changes

cg/cli/demultiplex/demux.py Show resolved Hide resolved

diitaz93 commented Dec 7, 2023

View reviewed changes

karlnyr reviewed Dec 7, 2023

View reviewed changes

cg/meta/demultiplex/delete_demultiplex_api.py Show resolved Hide resolved

karlnyr reviewed Dec 7, 2023

View reviewed changes

Vince-janv added 4 commits December 7, 2023 15:31

Addressed review

0c0bef8

Add commit in function

94a7395

Remove removal of logs

7fbe77f

Merge branch 'master' into delete-flowcell-post-processing

13397ae

karlnyr approved these changes Dec 8, 2023

View reviewed changes

Vince-janv merged commit e53a094 into master Dec 8, 2023
9 checks passed

Vince-janv deleted the delete-flowcell-post-processing branch December 8, 2023 09:56

diitaz93 mentioned this pull request Dec 8, 2023

Re-implement DeleteDemuxAPI #2541

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete old flow cell before adding re-demultiplexed flow cell #2593

Delete old flow cell before adding re-demultiplexed flow cell #2593

diitaz93 commented Oct 16, 2023 •

edited by Vince-janv

Loading

islean commented Oct 16, 2023

ChrOertlin left a comment

diitaz93 commented Oct 16, 2023

Vince-janv left a comment •

edited

Loading

diitaz93 commented Oct 18, 2023 •

edited

Loading

sonarqubecloud bot commented Oct 26, 2023

diitaz93 left a comment

karlnyr left a comment

sonarqubecloud bot commented Dec 8, 2023

karlnyr left a comment

Vince-janv commented Dec 8, 2023

Vince-janv commented Dec 8, 2023

Vince-janv commented Dec 8, 2023

Delete old flow cell before adding re-demultiplexed flow cell #2593

Delete old flow cell before adding re-demultiplexed flow cell #2593

Conversation

diitaz93 commented Oct 16, 2023 • edited by Vince-janv Loading

Description

Changed

Before post-processing:

How to prepare for test

How to test

Expected test outcome

Review

This version is a

Implementation Plan

islean commented Oct 16, 2023

ChrOertlin left a comment

Choose a reason for hiding this comment

diitaz93 commented Oct 16, 2023

Vince-janv left a comment • edited Loading

Choose a reason for hiding this comment

diitaz93 commented Oct 18, 2023 • edited Loading

sonarqubecloud bot commented Oct 26, 2023

diitaz93 left a comment

Choose a reason for hiding this comment

karlnyr left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 8, 2023

karlnyr left a comment

Choose a reason for hiding this comment

Vince-janv commented Dec 8, 2023

Before re-finishing

After re-finishing

Vince-janv commented Dec 8, 2023

Finishing from scratch (flow cell not added previously) also worked

Vince-janv commented Dec 8, 2023

diitaz93 commented Oct 16, 2023 •

edited by Vince-janv

Loading

Vince-janv left a comment •

edited

Loading

diitaz93 commented Oct 18, 2023 •

edited

Loading