Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add IlluminaFlowCellDevice model #3239

Merged
merged 9 commits into from
May 20, 2024
Merged

Conversation

diitaz93
Copy link
Contributor

@diitaz93 diitaz93 commented May 16, 2024

Description

Closes https://github.com/Clinical-Genomics/add-new-tech/issues/2

Added

  • IlluminaFlowCellDevice model
  • Alembic migration

How to prepare for test

  • Ssh to relevant server (depending on type of change)

How to test

  • Do alembic -c $ALEMBIC_CG_STAGE upgrade fe23de4ed528
  • Verify that the new alembic version is fe23de4ed528
  • Verify that the new table ìllumina_flow_cell` exists
  • Create a new entry in the run_device table of type ILLUMINA
  • Create a new entry in the table illumina_flow_cell with the same id as the previous entry and mode 10B.
  • Query the new table filtering by flow cells with mode == "10B"
  • Do alembic -c $ALEMBIC_CG_STAGE downgrade 6e6c36d5157b

Expected test outcome

  • Check that the filtering returns a flow cell and no errors are raised
Screenshot 2024-05-20 at 09 34 09

Review

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

This version is a

  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

  • Deployed to stage:
Logging deploy ...
Getting deployer... done.
Getting last commit message and SHA... done.
Getting version of deploy scripts... /home/js.diazboada
done.
Log deploy... done.
cg, version 60.7.25
[js.diazboada@hasta:~] [S_base] $ up
  • Deployed to production:
Log deploy... done.
cg, version 60.7.25
remote: Enumerating objects: 615, done.
remote: Counting objects: 100% (615/615), done.
remote: Compressing objects: 100% (252/252), done.
remote: Total 615 (delta 452), reused 454 (delta 362), pack-reused 0
Receiving objects: 100% (615/615), 184.00 KiB | 0 bytes/s, done.
Resolving deltas: 100% (452/452), completed with 64 local objects.
From https://github.com/Clinical-Genomics/cg
   b511844..b624356  master     -> origin/master
   52e9972..27c2ff6  fix_dry_run_conflict -> origin/fix_dry_run_conflict
 * [new branch]      patch_taxprofiler_versions -> origin/patch_taxprofiler_versions
   fdb5e1a..4f27930  raredisease_store -> origin/raredisease_store
   4056ffa..92def7a  remove-FFPE-from-loqusdb -> origin/remove-FFPE-from-loqusdb
 * [new tag]         v60.7.25   -> v60.7.25
Already on 'master'
Your branch is behind 'origin/master' by 2 commits, and can be fast-forwarded.
  (use "git pull" to update your local branch)
Updating b511844..b624356
Fast-forward
 .bumpversion.cfg                                                   |  2 +-
 alembic/versions/2024_05_17_fe23de4ed528_add_illumina_flow_cell.py | 31 +++++++++++++++++++++++++++++++
 cg/__init__.py                                                     |  2 +-
 cg/constants/demultiplexing.py                                     |  2 +-
 cg/store/models.py                                                 | 13 +++++++++++++
 pyproject.toml                                                     |  2 +-
 6 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 alembic/versions/2024_05_17_fe23de4ed528_add_illumina_flow_cell.py
/home/js.diazboada
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 6e6c36d5157b -> fe23de4ed528, add_illumina_flow_cell
[js.diazboada@hasta:~] [P_base] $ 

@diitaz93 diitaz93 self-assigned this May 16, 2024
@diitaz93 diitaz93 added Project Task Part of a quarterly project alembic migration Perform a change in database labels May 16, 2024
@diitaz93
Copy link
Contributor Author

Apparently what we called "flow cell model" (10B, 25B) is officially called "type", which is the field we use for polymorphism. I changed it to flow_cell_type, any other suggestions? @Vince-janv @ChrOertlin

Copy link
Contributor

@Vince-janv Vince-janv left a comment

Choose a reason for hiding this comment

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

I think short and descriptive names for the fields are better. We already have a clear name for the Model, so we don't need to duplicate the information in the field name

cg/store/models.py Outdated Show resolved Hide resolved
cg/store/models.py Outdated Show resolved Hide resolved
cg/store/models.py Outdated Show resolved Hide resolved
cg/constants/demultiplexing.py Outdated Show resolved Hide resolved
cg/store/models.py Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented May 17, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@diitaz93 diitaz93 marked this pull request as ready for review May 20, 2024 07:07
@diitaz93 diitaz93 requested a review from a team as a code owner May 20, 2024 07:07
Copy link
Contributor

@ChrOertlin ChrOertlin left a comment

Choose a reason for hiding this comment

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

Looks good

@diitaz93 diitaz93 merged commit bb01f24 into master May 20, 2024
9 checks passed
@diitaz93 diitaz93 deleted the add-illumina-flow-cell-model branch May 20, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alembic migration Perform a change in database Project Task Part of a quarterly project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants