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

Fix database model error #2642

Merged
merged 5 commits into from
Oct 31, 2023
Merged

Fix database model error #2642

merged 5 commits into from
Oct 31, 2023

Conversation

seallard
Copy link
Contributor

@seallard seallard commented Oct 30, 2023

Description

When generating a revision, an error popped up

Alembic: sqlalchemy.exc.CompileError: VARCHAR requires a length on dialect mysql (Family.tickets)

This PR adds a length limit to the Family.tickets field which resolves this error.

Some changes detected when auto generating the revision are fixed in this PR as well. The enum fields are sensitive to ordering, which has changed for family.action, family.data_delivery and flowcell.status. Some of them have chaged from varchar to enum as well.

INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.autogenerate.compare] Detected NOT NULL on column 'application.min_sequencing_depth'
INFO  [alembic.autogenerate.compare] Detected type change from ENUM('analyze', 'running', 'hold') to Enum('analyze', 'hold', 'running') on 'family.action'
INFO  [alembic.autogenerate.compare] Detected type change from VARCHAR(length=64) to Enum('analysis', 'analysis-scout', 'fastq', 'fastq-scout', 'fastq_qc', 'fastq-analysis', 'fastq_qc-analysis', 'fastq-analysis-scout', 'nipt-viewer', 'no-delivery', 'scout', 'statina', name='datadelivery') on 'family.data_delivery'
INFO  [alembic.autogenerate.compare] Detected type change from ENUM('ondisk', 'processing', 'removed', 'requested', 'retrieved') to Enum('ondisk', 'removed', 'requested', 'processing', 'retrieved') on 'flowcell.status'
  Generating /Users/sebastianallard/repos/cg/alembic/versions/2023_10_30_b6f00cc615cf_sync_db.py ...  done

Fixed

  • Fix column error in Family model
  • Sync models with schema

This version is a

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

Implementation Plan

  • Apply migrations to stage database.
  • Verify it is working by retrieving some data.
  • Merge and apply to prod

@seallard seallard requested a review from a team as a code owner October 30, 2023 14:06
@seallard seallard changed the title Sync cg and status db Fix database model error Oct 30, 2023
Copy link
Contributor

@islean islean 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! Think one part of the alembic script will crash though

alembic/versions/2023_10_30_b6f00cc615cf_sync_db.py Outdated Show resolved Hide resolved
cg/store/models.py Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@seallard seallard merged commit 4ab4bd1 into master Oct 31, 2023
8 checks passed
@seallard seallard deleted the fix-alembic branch October 31, 2023 09:07
@seallard
Copy link
Contributor Author

seallard commented Oct 31, 2023

Applied to stage

(cg-env) ➜  cg git:(fix-alembic) ✗ alembic --config ../alembic-stage.ini upgrade head                                     
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 392e49db40fc -> b6f00cc615cf, Sync db

Applied to prod

(cg-env) ➜  cg git:(master) alembic --config ../alembic-prod.ini upgrade head                    
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 392e49db40fc -> b6f00cc615cf, Sync db

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.

2 participants