launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/disabled-archive-recipe-oops into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #751958 in Launchpad itself: "recipe request daily build script triggers OOPS on disabled archive"
  https://bugs.launchpad.net/launchpad/+bug/751958

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/disabled-archive-recipe-oops/+merge/57421

== Implementation ==

A simple change to stop the make_daily_builds script throw an exception (and hence oopsing) when there are issues with the recipe's daily build archive. Like is done for the case when there is already a build pending, the exception is caught, a message is logged, and the script continues.

== Tests ==

Add some tests to lp/code/model/tests/test_sourcepackagerecipebuild.py
  test_makeDailyBuilds_skips_builds_already_queued
  test_makeDailyBuilds_skips_disabled_archive
  test_makeDailyBuilds_skips_archive_with_no_permission

The first test above should have been written earlier when the functionality was first done but appears to have been missed out. So I added that and also tests for cases where daily builds were done on recipes with archive issues.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/model/sourcepackagerecipebuild.py
  lib/lp/code/model/tests/test_sourcepackagerecipebuild.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/disabled-archive-recipe-oops/+merge/57421
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/disabled-archive-recipe-oops into lp:launchpad.

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2011-02-16 13:28:59 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2011-04-13 02:15:48 +0000
@@ -67,6 +67,7 @@
 from lp.services.job.model.job import Job
 from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
 from lp.soyuz.model.buildfarmbuildjob import BuildFarmBuildJob
+from lp.soyuz.interfaces.archive import CannotUploadToArchive
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 
@@ -213,6 +214,12 @@
                     logger.debug(
                         ' - build already pending for %s', series_name)
                     continue
+                except CannotUploadToArchive, e:
+                    # This will catch all PPA related issues -
+                    # disabled, security, wrong pocket etc
+                    logger.debug(
+                        ' - daily build failed for %s: %s',
+                        series_name, str(e))
                 except ProgrammingError:
                     raise
                 except:

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2011-02-02 10:24:13 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2011-04-13 02:15:48 +0000
@@ -287,6 +287,68 @@
         daily_builds = SourcePackageRecipeBuild.makeDailyBuilds()
         self.assertEqual([], daily_builds)
 
+    def test_makeDailyBuilds_skips_builds_already_queued(self):
+        # If the recipe already has an identical build pending,
+        # makeDailyBuilds() won't create a build.
+        owner = self.factory.makePerson(name='eric')
+        recipe = self.factory.makeSourcePackageRecipe(
+            owner=owner, name=u'funky-recipe', build_daily=True,
+            is_stale=True)
+        series = list(recipe.distroseries)[0]
+        existing_build = recipe.requestBuild(
+            recipe.daily_build_archive, recipe.owner, series,
+            PackagePublishingPocket.RELEASE)
+        removeSecurityProxy(existing_build).date_created = (
+            datetime.now(utc) - timedelta(hours=24, seconds=1))
+        removeSecurityProxy(recipe).is_stale = True
+
+        logger = BufferLogger()
+        daily_builds = SourcePackageRecipeBuild.makeDailyBuilds(logger)
+        self.assertEqual([], daily_builds)
+        self.assertEqual(
+            'DEBUG Recipe eric/funky-recipe is stale\n'
+            'DEBUG  - build already pending for Warty (4.10)\n',
+            logger.getLogBuffer())
+
+    def test_makeDailyBuilds_skips_disabled_archive(self):
+        # If the recipe's daily build archive is disabled, makeDailyBuilds()
+        # won't create a build.
+        owner = self.factory.makePerson(name='eric')
+        recipe = self.factory.makeSourcePackageRecipe(
+            owner=owner, name=u'funky-recipe', build_daily=True,
+            is_stale=True)
+        archive = self.factory.makeArchive(owner=recipe.owner, name="ppa")
+        removeSecurityProxy(recipe).daily_build_archive = archive
+        removeSecurityProxy(archive).disable()
+
+        logger = BufferLogger()
+        daily_builds = SourcePackageRecipeBuild.makeDailyBuilds(logger)
+        self.assertEqual([], daily_builds)
+        self.assertEqual(
+            'DEBUG Recipe eric/funky-recipe is stale\n'
+            'DEBUG  - daily build failed for Warty (4.10): ' +
+            'PPA for Eric is disabled.\n',
+            logger.getLogBuffer())
+
+    def test_makeDailyBuilds_skips_archive_with_no_permission(self):
+        # If the recipe's daily build archive cannot be uploaded to due to
+        # insufficient permissions, makeDailyBuilds() won't create a build.
+        owner = self.factory.makePerson(name='eric')
+        recipe = self.factory.makeSourcePackageRecipe(
+            owner=owner, name=u'funky-recipe', build_daily=True,
+            is_stale=True)
+        archive = self.factory.makeArchive(name="ppa")
+        removeSecurityProxy(recipe).daily_build_archive = archive
+
+        logger = BufferLogger()
+        daily_builds = SourcePackageRecipeBuild.makeDailyBuilds(logger)
+        self.assertEqual([], daily_builds)
+        self.assertEqual(
+            'DEBUG Recipe eric/funky-recipe is stale\n'
+            'DEBUG  - daily build failed for Warty (4.10): ' +
+            'Signer has no upload rights to this PPA.\n',
+            logger.getLogBuffer())
+
     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.