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(models): add missing foreign key to workflow_uuid of Job (#214) #214

Merged
merged 1 commit into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"))
Copy link
Member

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?)

# 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",
)
7 changes: 6 additions & 1 deletion reana_db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ class Workflow(Base, Timestamp, QuotaBase):
retention_rules = relationship(
"WorkspaceRetentionRule", backref="workflow", lazy="dynamic"
)
jobs = relationship("Job", backref="workflow", lazy="dynamic")

__table_args__ = (
UniqueConstraint(
Expand Down Expand Up @@ -823,7 +824,11 @@ class Job(Base, Timestamp):

id_ = Column(UUIDType, primary_key=True, default=generate_uuid)
backend_job_id = Column(String(256))
workflow_uuid = Column(UUIDType)
workflow_uuid = Column(
UUIDType,
ForeignKey("__reana.workflow.id_"),
nullable=False,
)
status = Column(Enum(JobStatus), default=JobStatus.created)
compute_backend = Column(String(30))
cvmfs_mounts = Column(Text)
Expand Down
Loading