-
Notifications
You must be signed in to change notification settings - Fork 32
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(models): add missing foreign key to workflow_uuid of Job (#214) #214
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
102 changes: 102 additions & 0 deletions
102
reana_db/alembic/versions/20240531_0959_86435bb00714_foreign_key_for_workflow_uuid_of_job.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
"""Foreign key for workflow_uuid of Job. | ||
|
||
Revision ID: 86435bb00714 | ||
Revises: eb5309f3d8ee | ||
Create Date: 2024-05-31 09:59:18.951074 | ||
|
||
""" | ||
|
||
from alembic import op | ||
import sqlalchemy as sa | ||
from sqlalchemy.dialects import postgresql | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = "86435bb00714" | ||
down_revision = "eb5309f3d8ee" | ||
branch_labels = None | ||
depends_on = None | ||
|
||
|
||
def upgrade(): | ||
"""Upgrade to 86435bb00714.""" | ||
job_table = sa.sql.table( | ||
"job", | ||
sa.sql.column("id_"), | ||
sa.sql.column("workflow_uuid"), | ||
schema="__reana", | ||
) | ||
job_cache_table = sa.sql.table( | ||
"job_cache", | ||
sa.sql.column("job_id"), | ||
schema="__reana", | ||
) | ||
workflow_table = sa.sql.table( | ||
"workflow", | ||
sa.sql.column("id_"), | ||
schema="__reana", | ||
) | ||
|
||
# 1. delete jobs which refer to non-existing workflows | ||
# 1a. disable foreign key checks, so that we can delete jobs even if they | ||
# are still referenced in the `job_cache` table | ||
op.execute( | ||
sa.text( | ||
"ALTER TABLE __reana.job_cache " | ||
"ALTER CONSTRAINT fk_job_cache_job_id_job DEFERRABLE" | ||
) | ||
) | ||
op.execute(sa.text("SET CONSTRAINTS __reana.fk_job_cache_job_id_job DEFERRED")) | ||
# 1b. delete jobs from `job` table | ||
op.execute( | ||
job_table.delete().where( | ||
job_table.c.workflow_uuid.notin_(sa.select([workflow_table.c.id_])) | ||
) | ||
) | ||
# 1c. delete rows in `job_cache` that reference deleted jobs | ||
op.execute( | ||
job_cache_table.delete().where( | ||
job_cache_table.c.job_id.notin_(sa.select([job_table.c.id_])) | ||
) | ||
) | ||
# 1d. enable foreign key checks | ||
op.execute(sa.text("SET CONSTRAINTS __reana.fk_job_cache_job_id_job IMMEDIATE")) | ||
op.execute( | ||
sa.text( | ||
"ALTER TABLE __reana.job_cache " | ||
"ALTER CONSTRAINT fk_job_cache_job_id_job NOT DEFERRABLE" | ||
) | ||
) | ||
|
||
# 2. alter column to make it non-nullable | ||
op.alter_column( | ||
"job", | ||
"workflow_uuid", | ||
existing_type=postgresql.UUID(), | ||
nullable=False, | ||
schema="__reana", | ||
) | ||
|
||
# 3. create foreign key constraint | ||
op.create_foreign_key( | ||
"fk_job_workflow_uuid_workflow", | ||
"job", | ||
"workflow", | ||
["workflow_uuid"], | ||
["id_"], | ||
source_schema="__reana", | ||
referent_schema="__reana", | ||
) | ||
|
||
|
||
def downgrade(): | ||
"""Downgrade to eb5309f3d8ee.""" | ||
op.drop_constraint( | ||
"fk_job_workflow_uuid_workflow", "job", schema="__reana", type_="foreignkey" | ||
) | ||
op.alter_column( | ||
"job", | ||
"workflow_uuid", | ||
existing_type=postgresql.UUID(), | ||
nullable=True, | ||
schema="__reana", | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, shall we do a check first whether there would be any rows deleted by this upgrade, and if not then we are good, and if yes shall we warn the user beforehand about the deletion in order to ask for the permission to do so?
(FWIW I think it is perfectly OK, practically, to also remove the rows in this hard-way, since if a job is not attached to a workflow, then chances are it is not exposed to users at all, and so it's ready for the deletion. I'm just wondering, theoretically, whether this isn't a good occasion to introduce an interactive please-confirm-action upgrade technique for DB recipes. Could serve as a nice example for the future?)