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

Optimize iteration in DatasetInstance model + SA2.0 fix #16776

Merged
merged 3 commits into from
Oct 10, 2023
Merged
Changes from 2 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
12 changes: 6 additions & 6 deletions lib/galaxy/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from collections.abc import Callable
from datetime import timedelta
from enum import Enum
from itertools import chain
from string import Template
from typing import (
Any,
Expand Down Expand Up @@ -4485,13 +4486,12 @@ def set_dbkey(self, value):

def ok_to_edit_metadata(self):
# prevent modifying metadata when dataset is queued or running as input/output
# This code could be more efficient, i.e. by using mappers, but to prevent slowing down loading a History panel, we'll leave the code here for now
sa_session = object_session(self)
for job_to_dataset_association in (
sa_session.query(JobToInputDatasetAssociation).filter_by(dataset_id=self.id).all()
+ sa_session.query(JobToOutputDatasetAssociation).filter_by(dataset_id=self.id).all()
):
if job_to_dataset_association.job.state not in Job.terminal_states:
stmt1 = select(JobToInputDatasetAssociation.job_id).filter_by(dataset_id=self.id)
stmt2 = select(JobToOutputDatasetAssociation.job_id).filter_by(dataset_id=self.id)
for job_id in chain(sa_session.scalars(stmt1), sa_session.scalars(stmt2)):
state = sa_session.scalar(select(Job.state).where(Job.id == job_id))
Copy link
Member

Choose a reason for hiding this comment

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

If we're already doing this, why not join on the job table and filter the state using exists ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. This should be very fast now. Thanks for the suggestion!

Copy link
Member Author

Choose a reason for hiding this comment

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

and the test failures seem relevant...

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. Missed a final select.

if state not in Job.terminal_states:
return False
return True

Expand Down
Loading