Skip to content

Commit

Permalink
frontend: use ORM attributes in queries
Browse files Browse the repository at this point in the history
See #3130

This is a straightforward case. We cannot join or select using
attribute names represented as strings.

    sqlalchemy.exc.ArgumentError: Strings are not accepted for
    attribute names in loader options; please use class-bound
    attributes directly.

This one is a bit harder to understand.

    sqlalchemy.exc.ArgumentError: expected ORM mapped attribute for
    loader strategy argument

Easier to show on an example. When selecting e.g. `BuildChroot` and
wanting to join the build information, we cannot do
`join(models.Build)` but instead, we need to
`join(models.BuildChroot.build)`.
  • Loading branch information
FrostyX authored and praiskup committed Mar 7, 2024
1 parent cc71d22 commit 7a236d1
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 26 deletions.
10 changes: 8 additions & 2 deletions frontend/coprs_frontend/commands/rawhide_to_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ def rawhide_to_release_function(rawhide_chroot, dest_chroot, retry_forked):
.join(models.CoprChroot)
.filter(models.Copr.follow_fedora_branching == True)
.filter(models.CoprChroot.mock_chroot == mock_rawhide_chroot)
.options(joinedload('copr_chroots').joinedload('mock_chroot'))
.options(
joinedload(models.Copr.copr_chroots)
.joinedload(models.CoprChroot.mock_chroot)
)
)

mock_chroot.comment = mock_rawhide_chroot.comment
Expand Down Expand Up @@ -87,7 +90,10 @@ def rawhide_to_release_function(rawhide_chroot, dest_chroot, retry_forked):

fork_builds = (
db.session.query(models.Build)
.options(joinedload('build_chroots').joinedload('mock_chroot'))
.options(
joinedload(models.Build.build_chroots)
.joinedload(models.BuildChroot.mock_chroot)
)
.filter(models.Build.id.in_(latest_pkg_builds_in_rawhide))
).all()

Expand Down
105 changes: 82 additions & 23 deletions frontend/coprs_frontend/coprs/logic/builds_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,24 +320,45 @@ def get_pending_srpm_build_tasks(cls, background=None, data_type=None):

if data_type in ["for_backend", "overview"]:

load_build_fields = ["is_background", "submitted_by", "batch_id",
"user_id", "ssh_public_keys", "source_status"]
load_build_fields = [
models.Build.is_background,
models.Build.submitted_by,
models.Build.batch_id,
models.Build.user_id,
models.Build.ssh_public_keys,
models.Build.source_status,
]
if data_type == "for_backend":
# The custom method allows us to set the chroot for SRPM builds
load_build_fields += ["source_type", "source_json"]
load_build_fields += [
models.Build.source_type,
models.Build.source_json,
]

query = query.options(
load_only(*load_build_fields),
# from copr project info we only need the project name
joinedload('copr').load_only("user_id", "group_id", "name")
joinedload(models.Build.copr).load_only(
models.Copr.user_id,
models.Copr.group_id,
models.Copr.name,
)
.options(
joinedload('user').load_only("username"),
joinedload("group").load_only("name")
joinedload(models.Copr.user).load_only(
models.User.username,
),
joinedload(models.Copr.group).load_only(
models.Group.name,
)
),
# submitter
joinedload('user').load_only("username"),
joinedload(models.Build.user).load_only(
models.User.username,
),
# preload also batch info
joinedload('batch').load_only("blocked_by_id"),
joinedload(models.Build.batch).load_only(
models.Batch.blocked_by_id,
),
)
if background is not None:
query = query.filter(models.Build.is_background == (true() if background else false()))
Expand Down Expand Up @@ -371,21 +392,48 @@ def get_pending_build_tasks(cls, background=None, data_type=None):

if data_type in ["for_backend", "overview"]:
query = query.options(
load_only("build_id", "tags_raw"),
joinedload('build').load_only("id", "is_background", "submitted_by", "batch_id", "ssh_public_keys", "source_status")
load_only(
models.BuildChroot.build_id,
models.BuildChroot.tags_raw,
),
joinedload(models.BuildChroot.build).load_only(
models.Build.id,
models.Build.is_background,
models.Build.submitted_by,
models.Build.batch_id,
models.Build.ssh_public_keys,
models.Build.source_status,
)
.options(
# from copr project info we only need the project name
joinedload('copr').load_only("user_id", "group_id", "name")
joinedload(models.Build.copr).load_only(
models.Copr.user_id,
models.Copr.group_id,
models.Copr.name,
)
.options(
joinedload('user').load_only("username"),
joinedload("group").load_only("name")
joinedload(models.Copr.user).load_only(
models.User.username,
),
joinedload(models.Copr.group).load_only(
models.Group.name,
)
),
# submitter
joinedload('user').load_only("username"),
joinedload(models.Build.user).load_only(
models.User.username,
),
# preload also batch info
joinedload('batch').load_only("blocked_by_id"),
joinedload(models.Build.batch).load_only(
models.Batch.blocked_by_id,
),
),
joinedload(models.BuildChroot.mock_chroot).load_only(
models.MockChroot.os_version,
models.MockChroot.os_release,
models.MockChroot.arch,
models.MockChroot.tags_raw,
),
joinedload('mock_chroot').load_only("os_version", "os_release", "arch", "tags_raw"),
)

if background is not None:
Expand Down Expand Up @@ -425,7 +473,8 @@ def get_copr_builds_list(cls, copr, dirname=None):
if dirname:
copr_dir = coprs_logic.CoprDirsLogic.get_by_copr(copr, dirname)
query = query.filter(models.Build.copr_dir_id==copr_dir.id)
query = query.options(selectinload('build_chroots'), selectinload('package'))
query = query.options(selectinload(models.Build.build_chroots),
selectinload(models.Build.package))
return query

@classmethod
Expand Down Expand Up @@ -1523,13 +1572,23 @@ def package_build_chroots_query(cls, copr_dir, mock_chroot_ids):
"""
return (
models.BuildChroot.query
.join(models.Build)
.join(models.Package)
.join(models.BuildChroot.build)
.join(models.Build.package)
.options(
load_only("build_id", "status", "mock_chroot_id", "result_dir"),
contains_eager("build").load_only("package_id", "copr_dir_id",
"pkg_version")
.contains_eager("package").load_only("name"),
load_only(
models.BuildChroot.build_id,
models.BuildChroot.status,
models.BuildChroot.mock_chroot_id,
models.BuildChroot.result_dir,
),
contains_eager(models.BuildChroot.build).load_only(
models.Build.package_id,
models.Build.copr_dir_id,
models.Build.pkg_version,
)
.contains_eager(models.Build.package).load_only(
models.Package.name,
),
)
.filter(models.BuildChroot.mock_chroot_id.in_(mock_chroot_ids))
.filter(models.Build.copr_dir==copr_dir)
Expand Down
2 changes: 1 addition & 1 deletion frontend/coprs_frontend/coprs/logic/packages_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def get_packages_with_latest_builds_for_dir(
packages_map = {package.id: package for package in packages}

builds = (models.Build.query.filter(models.Build.id.in_(builds_ids))
.options(selectinload('build_chroots'))
.options(selectinload(models.Build.build_chroots))
.yield_per(1000))

for build in builds:
Expand Down

0 comments on commit 7a236d1

Please sign in to comment.