From 6eebe9217ee958150c2a4e8ed4e9dc12f5b57fa6 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: May 09 2020 19:59:56 +0000
Subject: frontend: don't queue build chroots with incomplete info


The use of outerjoin() was wrong, there should be join() from the
beginning (548a9a05e1f2fd1294a4b3d0e4b6691a85779fdf, and probably even
before).

That's because we can not process build_chroot without knowing what
Package we actually build and what CoprDir we build into.  The
BuildChroot.build_id is naturally also required, but we have non-null
constraint there.

Fixes: #1321
Relates: #1269

---

diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py
index 358a206..38fa658 100644
--- a/frontend/coprs_frontend/coprs/logic/builds_logic.py
+++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py
@@ -276,22 +276,31 @@ class BuildsLogic(object):
 
     @classmethod
     def get_pending_build_tasks(cls, background=None):
-        query = (models.BuildChroot.query
-                    .outerjoin(models.Build)
-                    .outerjoin(models.CoprDir)
-                    .outerjoin(models.Package, models.Package.id == models.Build.package_id)
-                    .options(joinedload('build').joinedload('copr_dir'),
-                             joinedload('build').joinedload('package'))
-                .filter(models.Build.canceled == false())
-                .filter(or_(
-                    models.BuildChroot.status == StatusEnum("pending"),
-                    and_(
-                        models.BuildChroot.status == StatusEnum("running"),
-                        models.BuildChroot.started_on < int(time.time() - 1.1 * app.config["MAX_BUILD_TIMEOUT"]),
-                        models.BuildChroot.ended_on.is_(None)
-                    )
-                ))
-                .order_by(models.Build.is_background.asc(), models.Build.id.asc()))
+        """
+        Get list of BuildChroot objects that are to be (re)processed.
+        """
+
+        # Also add too-long running tasks, those are probably staled.
+        # TODO: Is this still needed?
+        restart_older = int(time.time() - 1.1 * app.config["MAX_BUILD_TIMEOUT"])
+
+        query = (
+            models.BuildChroot.query
+            .join(models.Build)
+            .join(models.CoprDir)
+            # TODO: BuildChroot objects should be self-standing.  The thing is
+            # that this is racy -- Package reference provides some build
+            # configuration which can be changed in the middle of the
+            # BuildChroot processing.
+            .join(models.Package, models.Package.id == models.Build.package_id)
+            .options(joinedload('build').joinedload('copr_dir'),
+                     joinedload('build').joinedload('package'))
+            .filter(models.Build.canceled == false())
+            .filter(or_(models.BuildChroot.status == StatusEnum("pending"),
+                        and_(models.BuildChroot.status == StatusEnum("running"),
+                             models.BuildChroot.started_on < restart_older,
+                             models.BuildChroot.ended_on.is_(None))))
+            .order_by(models.Build.is_background.asc(), models.Build.id.asc()))
         if background is not None:
             query = query.filter(models.Build.is_background == (true() if background else false()))
         return query