remove obsolete build.started_on, build.ended_od

This cause problem when user wanted to remove project, which build has build.ended_on set to null.
fedora-copr · Mar 4, 2016 · 867aeb8 · 867aeb8
1 parent 8b06868
commit 867aeb8
Show file tree

Hide file tree

Showing 15 changed files with 60 additions and 39 deletions.
diff --git a/...d/coprs_frontend/alembic/versions/13af46c70227_remove_build_ended_on_build_started_on_.py b/...d/coprs_frontend/alembic/versions/13af46c70227_remove_build_ended_on_build_started_on_.py
@@ -0,0 +1,28 @@
+"""Remove build.ended_on, build.started_on columns
+
+Revision ID: 13af46c70227
+Revises: 22c5f7a954ce
+Create Date: 2016-03-04 09:14:03.088234
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '13af46c70227'
+down_revision = '22c5f7a954ce'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.drop_index('build_ended_on_canceled_started_on', table_name='build')
+    op.drop_column('build', 'ended_on')
+    op.drop_column('build', 'started_on')
+    op.create_index('build_canceled', 'build', ['canceled'], unique=False)
+
+
+def downgrade():
+    op.drop_index('build_canceled', table_name='build')
+    op.add_column('build', sa.Column('started_on', sa.INTEGER(), autoincrement=False, nullable=True))
+    op.add_column('build', sa.Column('ended_on', sa.INTEGER(), autoincrement=False, nullable=True))
+    op.create_index('build_ended_on_canceled_started_on', 'build', ['ended_on', 'canceled', 'started_on'], unique=False)
diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py
@@ -51,8 +51,8 @@ def get_recent_tasks(cls, user=None, limit=None):
             limit = 100
 
         query = models.Build.query \
-            .filter(models.Build.ended_on.isnot(None)) \
-            .order_by(models.Build.ended_on.desc())
+            .join(models.BuildChroot).filter(models.BuildChroot.ended_on.isnot(None)) \
+            .order_by(models.BuildChroot.ended_on.desc())
 
         if user is not None:
             query = query.filter(models.Build.user_id == user.id)
@@ -209,11 +209,12 @@ def get_importing(cls):
         """
         query = (models.Build.query.join(models.Build.copr)
                  .join(models.User)
+                 .join(models.BuildChroot)
                  .options(db.contains_eager(models.Build.copr))
                  .options(db.contains_eager("copr.owner"))
-                 .filter((models.Build.started_on == None)
-                         | (models.Build.started_on < int(time.time() - 7200)))
-                 .filter(models.Build.ended_on == None)
+                 .filter((models.BuildChroot.started_on == None)
+                         | (models.BuildChroot.started_on < int(time.time() - 7200)))
+                 .filter(models.BuildChroot.ended_on == None)
                  .filter(models.Build.canceled == False)
                  .order_by(models.Build.submitted_on.asc()))
         return query
@@ -681,9 +682,6 @@ def update_state_from_dict(cls, build, upd_dict):
             if value:
                 setattr(build, attr, value)
 
-        if build.max_ended_on is not None:
-            build.ended_on = build.max_ended_on
-
         db.session.add(build)
 
     @classmethod
@@ -736,8 +734,8 @@ def last_modified(cls, copr):
             builds.join(models.BuildChroot)
             .filter((models.BuildChroot.status == helpers.StatusEnum("succeeded"))
                     | (models.BuildChroot.status == helpers.StatusEnum("skipped")))
-            .filter(models.Build.ended_on.isnot(None))
-            .order_by(models.Build.ended_on.desc())
+            .filter(models.BuildChroot.ended_on.isnot(None))
+            .order_by(models.BuildChroot.ended_on.desc())
         ).first()
         if last_build:
             return last_build.ended_on
@@ -749,9 +747,9 @@ def filter_is_finished(cls, query, is_finished):
         # todo: check that ended_on is set correctly for all cases
         # e.g.: failed dist-git import, cancellation
         if is_finished:
-            return query.filter(models.Build.ended_on.isnot(None))
+            return query.join(models.BuildChroot).filter(models.BuildChroot.ended_on.isnot(None))
         else:
-            return query.filter(models.Build.ended_on.is_(None))
+            return query.join(models.BuildChroot).filter(models.BuildChroot.ended_on.is_(None))
 
     @classmethod
     def filter_by_group_name(cls, query, group_name):

diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py
@@ -429,10 +429,6 @@ class Build(db.Model, helpers.Serializer):
     # the three below represent time of important events for this build
     # as returned by int(time.time())
     submitted_on = db.Column(db.Integer, nullable=False)
-    # obsolete, see started_on property of each build_chroot
-    started_on = db.Column(db.Integer)
-    # means that ALL chroots are finished
-    ended_on = db.Column(db.Integer)
     # url of the build results
     results = db.Column(db.Text)
     # memory requirements for backend builder
@@ -496,6 +492,10 @@ def result_dir_name(self):
     def source_json_dict(self):
         return json.loads(self.source_json)
 
+    @property
+    def started_on(self):
+        return self.min_started_on()
+
     @property
     def min_started_on(self):
         mb_list = [chroot.started_on for chroot in
@@ -505,6 +505,10 @@ def min_started_on(self):
         else:
             return None
 
+    @property
+    def ended_on(self):
+        return self.max_ended_on()
+
     @property
     def max_ended_on(self):
         if not self.build_chroots:
@@ -634,11 +638,11 @@ def deletable(self):
         """
 
         # build failed due to import error
-        if self.state == "failed" and self.started_on is None:
+        if self.state == "failed" and self.min_started_on is None:
             return True
 
         # build failed and all chroots are finished
-        if self.state == "failed" and self.ended_on is not None:
+        if self.state == "failed" and self.max_ended_on is not None:
             return True
 
         return self.state in ["succeeded", "canceled", "skipped"]

diff --git a/frontend/coprs_frontend/coprs/rest_api/schemas.py b/frontend/coprs_frontend/coprs/rest_api/schemas.py
@@ -159,8 +159,6 @@ class BuildSchema(Schema):
     repos = SpaceSeparatedList(dump_only=True)
 
     submitted_on = fields.Int(dump_only=True)
-    started_on = fields.Int(dump_only=True)
-    ended_on = fields.Int(dump_only=True)
 
     # timeout = fields.Int(dump_only=True)  # currently has no use
     enable_net = fields.Bool(dump_only=True)

diff --git a/frontend/coprs_frontend/coprs/templates/_helpers.html b/frontend/coprs_frontend/coprs/templates/_helpers.html
@@ -294,7 +294,7 @@ <h4 class="list-group-item-heading">{{build.pkgs | pkg_name}}</h4>
           </p>
           <p class="list-group-item-text">
             <small>Finished:</small>
-            {{build.ended_on|time_ago()}} ago
+            {{build.max_ended_on|time_ago()}} ago
           </p>
         </a>
       {% endfor %}

diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/overview.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/overview.html
@@ -120,7 +120,7 @@ <h4 class="list-group-item-heading">{{latest_build.pkgs | pkg_name}}</h4>
           </p>
           <p class="list-group-item-text">
             <small> Finished: </small>
-            {{latest_build.ended_on|time_ago()}} ago
+            {{latest_build.max_ended_on|time_ago()}} ago
           </p>
         </a>
       </div>

diff --git a/frontend/coprs_frontend/coprs/templates/recent/all.html b/frontend/coprs_frontend/coprs/templates/recent/all.html
@@ -37,7 +37,7 @@ <h2> All Projects </h2>
           {{ build.pkg_version}}
         </td>
         <td>
-          {{ build.ended_on|time_ago() }} ago
+          {{ build.max_ended_on|time_ago() }} ago
         </td>
         {% if build.canceled %}
         <td class="build-canceled">canceled</td>

diff --git a/frontend/coprs_frontend/coprs/templates/recent/my.html b/frontend/coprs_frontend/coprs/templates/recent/my.html
@@ -37,7 +37,7 @@ <h2> My Projects </h2>
         {{ build.pkg_version}}
       </td>
       <td>
-        {{ build.ended_on|time_ago() }} ago
+        {{ build.max_ended_on|time_ago() }} ago
       </td>
       {% if build.canceled %}
       <td class="build-canceled">canceled</td>

diff --git a/frontend/coprs_frontend/coprs/views/api_ns/api_general.py b/frontend/coprs_frontend/coprs/views/api_ns/api_general.py
@@ -417,7 +417,7 @@ def build_detail(build_id):
         "chroots": chroots,
         "submitted_on": build.submitted_on,
         "started_on": build.min_started_on,
-        "ended_on": build.ended_on,
+        "ended_on": build.max_ended_on,
         "src_pkg": build.pkgs,
         "submitted_by": build.user.name,
         "results_by_chroot": results_by_chroot

diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py
@@ -249,7 +249,6 @@ def f_builds(self):
                 if build is self.b1 or build is self.b2:
                     buildchroot.started_on = 139086644000
                     buildchroot.ended_on = 149086644000
-                    build.ended_on = 149086644000
 
 
                 build_chroots.append(buildchroot)
@@ -265,7 +264,7 @@ def f_build_few_chroots(self):
         self.b_few_chroots = models.Build(
             id=2345,
             copr=self.c1, user=self.u1,
-            submitted_on=50, started_on=139086644000,
+            submitted_on=50,
             pkgs="http://example.com/copr-keygen-1.58-1.fc20.src.rpm",
             pkg_version="1.58"
         )
@@ -299,7 +298,7 @@ def f_build_many_chroots(self):
         self.b_many_chroots = models.Build(
             id=12347,
             copr=self.c1, user=self.u1,
-            submitted_on=50, started_on=139086644000,
+            submitted_on=50,
             pkgs="http://example.com/copr-keygen-1.58-1.fc20.src.rpm",
             pkg_version="1.58"
         )

diff --git a/frontend/coprs_frontend/tests/test_api/test_build_r.py b/frontend/coprs_frontend/tests/test_api/test_build_r.py
@@ -52,8 +52,8 @@ def test_build_collection_ok_finished(
         obj_b = json.loads(r_b.data.decode("utf-8"))
 
         builds = BuildsLogic.get_multiple().all()
-        expected_ids_a = set([b.id for b in builds if b.ended_on is not None])
-        expected_ids_b = set([b.id for b in builds if b.ended_on is None])
+        expected_ids_a = set([b.id for b in builds if b.max_ended_on is not None])
+        expected_ids_b = set([b.id for b in builds if b.max_ended_on is None])
 
         assert expected_ids_a == self.extract_build_ids(obj_a)
         assert expected_ids_b == self.extract_build_ids(obj_b)
@@ -459,7 +459,6 @@ def test_build_put_wrong_user(
             bc.status = StatusEnum("pending")
             bc.ended_on = None
 
-        self.b1.ended_on = None
         self.db.session.add_all(self.b1_bc)
         self.db.session.add(self.b1)
 
@@ -501,7 +500,6 @@ def test_build_put_cancel(
             bc.status = StatusEnum("pending")
             bc.ended_on = None
 
-        self.b1.ended_on = None
         self.db.session.add_all(self.b1_bc)
         self.db.session.add(self.b1)
 
@@ -527,7 +525,6 @@ def test_build_put_cancel_wrong_state(
             self, f_users, f_coprs,
             f_mock_chroots, f_builds,f_users_api, ):
 
-        self.b1.ended_on = None
         old_state = self.b1.state
         self.db.session.add_all(self.b1_bc)
         self.db.session.add(self.b1)

diff --git a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py
@@ -122,7 +122,6 @@ def test_delete_build_exceptions(
         for bc in self.b4_bc:
             bc.status = StatusEnum("succeeded")
             bc.ended_on = time.time()
-        self.b4.ended_on = time.time()
         self.u1.admin = False
 
         self.db.session.add_all(self.b4_bc)
@@ -142,7 +141,6 @@ def test_delete_build_as_admin(
             self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):
 
         self.b4.pkgs = "http://example.com/copr-keygen-1.58-1.fc20.src.rpm"
-        self.b4.ended_on = time.time()
         expected_dir = self.b4.result_dir_name
         for bc in self.b4_bc:
             bc.status = StatusEnum("succeeded")

diff --git a/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py b/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
@@ -111,7 +111,6 @@ def test_update_build_ended(self, f_users, f_coprs, f_mock_chroots,
     def test_update_more_existent_and_non_existent_builds(
             self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):
 
-        self.b1.started_on = None
         self.db.session.add(self.b1)
         self.db.session.commit()
 

diff --git a/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py b/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
@@ -202,7 +202,7 @@ def test_copr_build_basic_build_repeat(
         self.b_few_chroots = models.Build(
             id=2345,
             copr=self.c1, user=self.u1,
-            submitted_on=50, started_on=139086644000,
+            submitted_on=50,
             pkgs="http://example.com/copr-keygen-1.58-1.fc20.src.rpm",
             pkg_version="1.58"
         )

diff --git a/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py b/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
@@ -620,12 +620,12 @@ def f_custom_builds(self):
         """ Custom builds are used in order not to break the default ones """
         self.b5 = self.models.Build(
             copr=self.c1, user=self.u1, submitted_on=9,
-            ended_on=200, results="https://bar.baz")
+            results="https://bar.baz")
         self.b6 = self.models.Build(
             copr=self.c1, user=self.u1, submitted_on=11)
         self.b7 = self.models.Build(
             copr=self.c1, user=self.u1, submitted_on=10,
-            ended_on=150, results="https://bar.baz")
+            results="https://bar.baz")
         self.mc1 = self.models.MockChroot(
             os_release="fedora", os_version="18", arch="x86_64")
         self.cc1 = self.models.CoprChroot(mock_chroot=self.mc1, copr=self.c1)