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

Towards SQLAlchemy 2.0: use model attributes instead of strings #15340

Merged
merged 16 commits into from
Jan 22, 2023
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
4 changes: 3 additions & 1 deletion lib/galaxy/managers/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,9 @@ def _get_collection_contents_qry(self, parent_id, limit=None, offset=None):
DCE = model.DatasetCollectionElement
qry = Query(DCE).filter(DCE.dataset_collection_id == parent_id)
qry = qry.order_by(DCE.element_index)
qry = qry.options(joinedload("child_collection"), joinedload("hda"))
qry = qry.options(
joinedload(model.DatasetCollectionElement.child_collection), joinedload(model.DatasetCollectionElement.hda)
)
if limit is not None:
qry = qry.limit(int(limit))
if offset is not None:
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/managers/history_contents.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ def _contained_id_map(self, id_list):
.query(component_class)
.filter(component_class.id.in_(id_list))
.options(undefer(component_class._metadata))
.options(joinedload("dataset.actions")) # TODO: use class attr after moving Dataset to declarative mapping.
.options(joinedload(component_class.dataset).joinedload(model.Dataset.actions))
.options(joinedload(component_class.tags))
.options(joinedload(component_class.annotations)) # type: ignore[attr-defined]
)
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/managers/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def get_session_from_session_key(self, session_key: str):
self.model.GalaxySession.table.c.is_valid == true(),
)
)
.options(joinedload("user"))
.options(joinedload(self.model.GalaxySession.user))
.first()
)
return galaxy_session
2 changes: 1 addition & 1 deletion lib/galaxy/managers/sharable.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def _query_shared_with(self, user, eagerloads=True, **kwargs):
Return a query for this model already filtered to models shared
with a particular user.
"""
query = self.session().query(self.model_class).join("users_shared_with")
query = self.session().query(self.model_class).join(self.model_class.users_shared_with)
if eagerloads is False:
query = query.enable_eagerloads(False)
# TODO: as filter in FilterParser also
Expand Down
20 changes: 11 additions & 9 deletions lib/galaxy/managers/workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,10 @@ def index_query(
query = query.outerjoin(model.StoredWorkflow.users_shared_with)
query = query.outerjoin(model.StoredWorkflow.tags)

latest_workflow_load = joinedload("latest_workflow")
latest_workflow_load = joinedload(model.StoredWorkflow.latest_workflow)
if not payload.skip_step_counts:
latest_workflow_load = latest_workflow_load.undefer("step_count")
latest_workflow_load = latest_workflow_load.lazyload("steps")
latest_workflow_load = latest_workflow_load.lazyload(model.Workflow.steps)

query = query.options(joinedload(model.StoredWorkflow.annotations))
query = query.options(latest_workflow_load)
Expand Down Expand Up @@ -268,9 +268,11 @@ def get_stored_workflow(self, trans, workflow_id, by_stored_id=True) -> StoredWo
)
)
stored_workflow = workflow_query.options(
joinedload("annotations"),
joinedload("tags"),
subqueryload("latest_workflow").joinedload("steps").joinedload("*"),
joinedload(trans.app.model.StoredWorkflow.annotations),
joinedload(trans.app.model.StoredWorkflow.tags),
subqueryload(trans.app.model.StoredWorkflow.latest_workflow)
.joinedload(trans.app.model.Workflow.steps)
Comment on lines +271 to +274
Copy link
Member

Choose a reason for hiding this comment

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

You can use model.* instead of trans.app, right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, absolutely! There were a few other similar cases - I thought it might be better to group all those changes in a "cleanup" PR after I'm done with all the relevant SA 2.0 updates to Query/select() not to mix them with strictly functional stuff in case something breaks and we need to bisect lots of commits, etc.

.joinedload("*"),
).first()
if stored_workflow is None:
if not by_stored_id:
Expand Down Expand Up @@ -366,10 +368,10 @@ def get_invocation(self, trans, decoded_invocation_id, eager=False) -> model.Wor
if eager:
q = q.options(
subqueryload(model.WorkflowInvocation.steps)
.joinedload("implicit_collection_jobs")
.joinedload("jobs")
.joinedload("job")
.joinedload("input_datasets")
.joinedload(model.WorkflowInvocationStep.implicit_collection_jobs)
.joinedload(model.ImplicitCollectionJobs.jobs)
.joinedload(model.ImplicitCollectionJobsJobAssociation.job)
.joinedload(model.Job.input_datasets)
)
workflow_invocation = q.get(decoded_invocation_id)
if not workflow_invocation:
Expand Down
13 changes: 8 additions & 5 deletions lib/galaxy/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2962,10 +2962,10 @@ def active_dataset_and_roles_query(self):
.filter(not_(HistoryDatasetAssociation.deleted))
.order_by(HistoryDatasetAssociation.table.c.hid.asc())
.options(
joinedload("dataset"),
joinedload("dataset.actions"),
joinedload("dataset.actions.role"),
joinedload("tags"),
joinedload(HistoryDatasetAssociation.dataset)
.joinedload(Dataset.actions)
.joinedload(DatasetPermissions.role),
joinedload(HistoryDatasetAssociation.tags),
)
)

Expand Down Expand Up @@ -2993,7 +2993,10 @@ def active_visible_dataset_collections(self):
.filter(not_(HistoryDatasetCollectionAssociation.deleted))
.filter(HistoryDatasetCollectionAssociation.visible)
.order_by(HistoryDatasetCollectionAssociation.table.c.hid.asc())
.options(joinedload("collection"), joinedload("tags"))
.options(
joinedload(HistoryDatasetCollectionAssociation.collection),
joinedload(HistoryDatasetCollectionAssociation.tags),
)
)
self._active_visible_dataset_collections = query.all()
return self._active_visible_dataset_collections
Expand Down
5 changes: 4 additions & 1 deletion lib/galaxy/model/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -1507,12 +1507,15 @@ def check_folder_contents(self, user, roles, folder, hidden_folder_ids=""):
return True, ""
action = self.permitted_actions.DATASET_ACCESS

raise
lddas = (
self.sa_session.query(self.model.LibraryDatasetDatasetAssociation)
.join("library_dataset")
.filter(self.model.LibraryDataset.folder == folder)
.join("dataset")
.options(joinedload("dataset").joinedload("actions"))
.options(
joinedload(self.model.LibraryDatasetDatasetAssociation.dataset).joinedload(self.model.Dataset.actions)
)
.all()
)

Expand Down
3 changes: 2 additions & 1 deletion lib/galaxy/tools/actions/upload_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,11 @@ def active_folders(trans, folder):
# Stolen from galaxy.web.controllers.library_common (importing from which causes a circular issues).
# Much faster way of retrieving all active sub-folders within a given folder than the
# performance of the mapper. This query also eagerloads the permissions on each folder.
raise
return (
trans.sa_session.query(LibraryFolder)
.filter_by(parent=folder, deleted=False)
.options(joinedload("actions"))
.options(joinedload(LibraryFolder.actions))
.order_by(LibraryFolder.table.c.name)
.all()
)
2 changes: 1 addition & 1 deletion lib/galaxy/webapps/galaxy/controllers/history.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def get_value(self, trans, grid, history):
]

def build_initial_query(self, trans, **kwargs):
return trans.sa_session.query(self.model_class).join("users_shared_with")
return trans.sa_session.query(self.model_class).join(self.model_class.users_shared_with)

def apply_query_filter(self, trans, query, **kwargs):
return query.filter(model.HistoryUserShareAssociation.user == trans.user)
Expand Down
6 changes: 5 additions & 1 deletion lib/galaxy/webapps/galaxy/controllers/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ def build_initial_query(self, trans, **kwargs):
trans.sa_session.query(self.model_class)
.join("user")
.filter(model.User.deleted == false())
.options(joinedload("user").load_only("username"), joinedload("annotations"), undefer("average_rating"))
.options(
joinedload(self.model_class.user).load_only("username"),
joinedload(self.model_class.annotations),
undefer("average_rating"),
)
)

def apply_query_filter(self, trans, query, **kwargs):
Expand Down
8 changes: 6 additions & 2 deletions lib/galaxy/webapps/galaxy/controllers/visualization.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,12 @@ def build_initial_query(self, trans, **kwargs):
# See optimization description comments and TODO for tags in matching public histories query.
return (
trans.sa_session.query(self.model_class)
.join("user")
.options(joinedload("user").load_only("username"), joinedload("annotations"), undefer("average_rating"))
.join(self.model_class.user)
.options(
joinedload(self.model_class.user).load_only("username"),
joinedload(self.model_class.annotations),
undefer("average_rating"),
)
)

def apply_query_filter(self, trans, query, **kwargs):
Expand Down
12 changes: 6 additions & 6 deletions lib/galaxy/webapps/galaxy/controllers/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ def build_initial_query(self, trans, **kwargs):
# of its steps to be eagerly loaded.
return (
trans.sa_session.query(self.model_class)
.join("user")
.join(self.model_class.user)
.options(
lazyload("latest_workflow"),
joinedload("user").load_only("username"),
joinedload("annotations"),
undefer("average_rating"),
lazyload(self.model_class.latest_workflow),
joinedload(self.model_class.user).load_only(model.User.username),
joinedload(self.model_class.annotations),
undefer(self.model_class.average_rating),
)
)

Expand Down Expand Up @@ -487,7 +487,7 @@ def editor(self, trans, id=None, workflow_id=None, version=None):
trans.sa_session.query(model.StoredWorkflow)
.filter_by(user=trans.user, deleted=False, hidden=False)
.order_by(desc(model.StoredWorkflow.table.c.update_time))
.options(joinedload("latest_workflow").joinedload("steps"))
.options(joinedload(model.StoredWorkflow.latest_workflow).joinedload(model.Workflow.steps))
.all()
)
if version is None:
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/webapps/reports/controllers/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def deleted_histories(self, trans, **kwd):
model.History.update_time < cutoff_time,
)
)
.options(joinedload("datasets"))
.options(joinedload(model.History.datasets))
)

for history in histories:
Expand Down
11 changes: 6 additions & 5 deletions lib/tool_shed/webapp/api/repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
repository_util,
tool_util,
)
from tool_shed.webapp import model
from tool_shed.webapp.search.repo_search import RepoSearch

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -118,7 +119,7 @@ def get_ordered_installable_revisions(self, trans, name=None, owner=None, **kwd)
if owner is None:
owner = kwd.get("owner", None)
tsr_id = kwd.get("tsr_id", None)
eagerload_columns = ["downloadable_revisions"]
eagerload_columns = [model.Repository.downloadable_revisions]
if None not in [name, owner]:
# Get the repository information.
repository = repository_util.get_repository_by_name_and_owner(
Expand Down Expand Up @@ -217,7 +218,7 @@ def get_repository_revision_install_info(self, trans, name, owner, changeset_rev
if name and owner and changeset_revision:
# Get the repository information.
repository = repository_util.get_repository_by_name_and_owner(
self.app, name, owner, eagerload_columns=["downloadable_revisions"]
self.app, name, owner, eagerload_columns=[model.Repository.downloadable_revisions]
)
if repository is None:
log.debug(f"Cannot locate repository {name} owned by {owner}")
Expand Down Expand Up @@ -284,7 +285,7 @@ def get_installable_revisions(self, trans, **kwd):
tsr_id = kwd.get("tsr_id", None)
if tsr_id is not None:
repository = repository_util.get_repository_in_tool_shed(
self.app, tsr_id, eagerload_columns=["downloadable_revisions"]
self.app, tsr_id, eagerload_columns=[model.Repository.downloadable_revisions]
)
else:
error_message = "Error in the Tool Shed repositories API in get_ordered_installable_revisions: "
Expand Down Expand Up @@ -737,7 +738,7 @@ def updates(self, trans, **kwd):
changeset_revision = kwd.get("changeset_revision", None)
hexlify_this = util.asbool(kwd.get("hexlify", True))
repository = repository_util.get_repository_by_name_and_owner(
trans.app, name, owner, eagerload_columns=["downloadable_revisions"]
trans.app, name, owner, eagerload_columns=[model.Repository.downloadable_revisions]
)
if repository and repository.downloadable_revisions:
repository_metadata = metadata_util.get_repository_metadata_by_changeset_revision(
Expand Down Expand Up @@ -837,7 +838,7 @@ def metadata(self, trans, id, **kwd):
downloadable_only = util.asbool(kwd.get("downloadable_only", "True"))
all_metadata = {}
repository = repository_util.get_repository_in_tool_shed(
self.app, id, eagerload_columns=["downloadable_revisions"]
self.app, id, eagerload_columns=[model.Repository.downloadable_revisions]
)
for changeset, changehash in metadata_util.get_metadata_revisions(
self.app, repository, sort_revisions=True, downloadable=downloadable_only
Expand Down