launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/queue-recipes-better into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/queue-recipes-better/+merge/47358

Change the way daily builds are queued when makeDailyBuilds() is called. We now check if the recipe has had a build in the last 24 hours, and if not, we proceed as per normal.
-- 
https://code.launchpad.net/~stevenk/launchpad/queue-recipes-better/+merge/47358
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/queue-recipes-better into lp:launchpad.

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2011-01-14 05:46:14 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2011-01-25 05:28:04 +0000
@@ -216,7 +216,7 @@
             raise ValueError('Source package recipe builds disabled.')
         if not archive.is_ppa:
             raise NonPPABuildRequest
-        component = getUtility(IComponentSet)["multiverse"]
+        component = getUtility(IComponentSet)['main']
 
         buildable_distros = get_buildable_distroseries_set(archive.owner)
         if distroseries not in buildable_distros:
@@ -271,14 +271,8 @@
 
     def getLastBuild(self):
         """See `ISourcePackageRecipeBuild`."""
-        store = Store.of(self)
-        result = store.find(
-            (SourcePackageRecipeBuild),
-            SourcePackageRecipeBuild.recipe == self,
-            SourcePackageRecipeBuild.package_build_id == PackageBuild.id,
-            PackageBuild.build_farm_job_id == BuildFarmJob.id)
-        result.order_by(Desc(BuildFarmJob.date_finished))
-        return result.first()
+        return self._getBuilds(
+            True, Desc(BuildFarmJob.date_finished)).first()
 
     def getMedianBuildDuration(self):
         """Return the median duration of builds of this recipe."""

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2011-01-20 22:01:29 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2011-01-25 05:28:04 +0000
@@ -10,7 +10,10 @@
     'SourcePackageRecipeBuild',
     ]
 
-import datetime
+from datetime import (
+    datetime,
+    timedelta,
+    )
 import logging
 import sys
 
@@ -194,6 +197,13 @@
             logger = logging.getLogger()
         builds = []
         for recipe in recipes:
+            # If there is a last build, and it was created less than 24 hours
+            # ago, skip the recipe, *and* don't update staleness.
+            latest_build = recipe.getLastBuild()
+            if latest_build is not None:
+                pb = latest_build.package_build
+                if datetime.now(utc) - pb.date_created < timedelta(hours=24):
+                    continue
             recipe.is_stale = False
             logger.debug(
                 'Recipe %s/%s is stale', recipe.owner.name, recipe.name)
@@ -259,9 +269,9 @@
     def getRecentBuilds(cls, requester, recipe, distroseries, _now=None):
         from lp.buildmaster.model.buildfarmjob import BuildFarmJob
         if _now is None:
-            _now = datetime.datetime.now(utc)
+            _now = datetime.now(utc)
         store = IMasterStore(SourcePackageRecipeBuild)
-        old_threshold = _now - datetime.timedelta(days=1)
+        old_threshold = _now - timedelta(days=1)
         return store.find(cls, cls.distroseries_id == distroseries.id,
             cls.requester_id == requester.id, cls.recipe_id == recipe.id,
             BuildFarmJob.date_created > old_threshold,
@@ -282,7 +292,7 @@
         median = self.recipe.getMedianBuildDuration()
         if median is not None:
             return median
-        return datetime.timedelta(minutes=10)
+        return timedelta(minutes=10)
 
     def verifySuccessfulUpload(self):
         return self.source_package_release is not None

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2011-01-20 22:01:29 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2011-01-25 05:28:04 +0000
@@ -5,9 +5,12 @@
 
 __metaclass__ = type
 
-import datetime
+from datetime import (
+    datetime,
+    timedelta,
+    )
+from pytz import utc
 import re
-import unittest
 
 from storm.locals import Store
 import transaction
@@ -177,16 +180,13 @@
         # If there are no successful builds, estimate 10 minutes.
         spb = self.makeSourcePackageRecipeBuild()
         cur_date = self.factory.getUniqueDate()
-        self.assertEqual(
-            datetime.timedelta(minutes=10), spb.estimateDuration())
+        self.assertEqual(timedelta(minutes=10), spb.estimateDuration())
         for minutes in [20, 5, 1]:
             build = removeSecurityProxy(
                 self.factory.makeSourcePackageRecipeBuild(recipe=spb.recipe))
             build.date_started = cur_date
-            build.date_finished = (
-                cur_date + datetime.timedelta(minutes=minutes))
-        self.assertEqual(
-            datetime.timedelta(minutes=5), spb.estimateDuration())
+            build.date_finished = cur_date + timedelta(minutes=minutes)
+        self.assertEqual(timedelta(minutes=5), spb.estimateDuration())
 
     def test_getFileByName(self):
         """getFileByName returns the logs when requested by name."""
@@ -269,20 +269,40 @@
         SourcePackageRecipeBuild.makeDailyBuilds()[0]
         self.assertFalse(recipe.is_stale)
 
-    def test_makeDailyBuilds_skips_pending(self):
-        """When creating daily builds, skip ones that are already pending."""
+    def test_makeDailyBuilds_skips_if_built_in_last_24_hours(self):
+        # The makeDailyBuilds() call exits early if the recipe has been built
+        # in the last 24 hours.
         recipe = self.factory.makeSourcePackageRecipe(
             build_daily=True, is_stale=True)
-        first_distroseries = list(recipe.distroseries)[0]
         recipe.requestBuild(
-            recipe.daily_build_archive, recipe.owner, first_distroseries,
-            PackagePublishingPocket.RELEASE)
-        second_distroseries = \
-            self.factory.makeSourcePackageRecipeDistroseries("hoary")
-        recipe.distroseries.add(second_distroseries)
-        builds = SourcePackageRecipeBuild.makeDailyBuilds()
-        self.assertEqual(
-            [second_distroseries], [build.distroseries for build in builds])
+            recipe.daily_build_archive, recipe.owner,
+            list(recipe.distroseries)[0], PackagePublishingPocket.RELEASE)
+        daily_builds = SourcePackageRecipeBuild.makeDailyBuilds()
+        self.assertEqual([], daily_builds)
+
+    def test_makeDailyBuilds_skips_non_stale_builds(self):
+        # makeDailyBuilds() won't create a build if the recipe isn't stale.
+        self.factory.makeSourcePackageRecipe(
+            build_daily=True, is_stale=False)
+        daily_builds = SourcePackageRecipeBuild.makeDailyBuilds()
+        self.assertEqual([], daily_builds)
+
+    def test_makeDailyBuilds_with_an_older_build(self):
+        # If a previous build is more than 24 hours old, and the recipe is
+        # stale, we'll fire another off.
+        recipe = self.factory.makeSourcePackageRecipe(
+            build_daily=True, is_stale=True)
+        build = recipe.requestBuild(
+            recipe.daily_build_archive, recipe.owner,
+            list(recipe.distroseries)[0], PackagePublishingPocket.RELEASE)
+        nb = removeSecurityProxy(build)
+        nb.date_created = datetime.now(utc) - timedelta(hours=24, seconds=1)
+        # The build also needs to be completed
+        nb.status = BuildStatus.FULLYBUILT
+        daily_builds = SourcePackageRecipeBuild.makeDailyBuilds()
+        self.assertEquals(1, len(daily_builds))
+        actual_title = [b.title for b in daily_builds]
+        self.assertEquals([build.title], actual_title)
 
     def test_getRecentBuilds(self):
         """Recent builds match the same person, series and receipe.
@@ -306,12 +326,12 @@
             return SourcePackageRecipeBuild.getRecentBuilds(
                 requester, recipe, series, _now=now)
         self.assertContentEqual([], get_recent())
-        yesterday = now - datetime.timedelta(days=1)
+        yesterday = now - timedelta(days=1)
         recent_build = self.factory.makeSourcePackageRecipeBuild(
             recipe=recipe, distroseries=series, requester=requester,
             date_created=yesterday)
         self.assertContentEqual([], get_recent())
-        a_second = datetime.timedelta(seconds=1)
+        a_second = timedelta(seconds=1)
         removeSecurityProxy(recent_build).date_created += a_second
         self.assertContentEqual([recent_build], get_recent())
 
@@ -363,7 +383,6 @@
         # getSpecificJob returns the SourcePackageRecipeBuild
         sprb = self.makeSourcePackageRecipeBuild()
         Store.of(sprb).flush()
-        build = sprb.build_farm_job
         job = sprb.build_farm_job.getSpecificJob()
         self.assertEqual(sprb, job)
 
@@ -443,7 +462,7 @@
         build = self.factory.makeSourcePackageRecipeBuild(
             distroseries=distroseries,
             status=BuildStatus.FULLYBUILT,
-            duration=datetime.timedelta(minutes=5))
+            duration=timedelta(minutes=5))
         build.queueBuild(build)
         return build
 
@@ -456,7 +475,3 @@
 class TestHandleStatusForSPRBuild(
     MakeSPRecipeBuildMixin, TestHandleStatusMixin, TrialTestCase):
     """IPackageBuild.handleStatus works with SPRecipe builds."""
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)