launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+416986@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:make-automatic-charmrecipe-builds-more-resilient into launchpad:master.

Commit message:
Make automatic charmrecipe builds more resilient

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/416986
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:make-automatic-charmrecipe-builds-more-resilient into launchpad:master.

diff --git a/lib/lp/charms/model/charmrecipe.py b/lib/lp/charms/model/charmrecipe.py
index 7a112da..7c8c59a 100644
--- a/lib/lp/charms/model/charmrecipe.py
+++ b/lib/lp/charms/model/charmrecipe.py
@@ -628,7 +628,7 @@ class CharmRecipe(StormBase, WebhookTargetMixin):
                 "Scheduling builds of charm recipe %s/%s/%s",
                 self.owner.name, self.project.name, self.name)
         return self.requestBuilds(
-            self.owner, channels=self.auto_build_channels)
+            requester=self.owner, channels=self.auto_build_channels)
 
     def getBuildRequest(self, job_id):
         """See `ICharmRecipe`."""
@@ -1064,7 +1064,13 @@ class CharmRecipeSet:
         recipes = cls._findStaleRecipes()
         build_requests = []
         for recipe in recipes:
-            build_requests.append(recipe.requestAutoBuilds(logger=logger))
+            try:
+                build_request = recipe.requestAutoBuilds(logger=logger)
+            except Exception as e:
+                if logger is not None:
+                    logger.exception(e)
+                continue
+            build_requests.append(build_request)
         return build_requests
 
     def detachFromGitRepository(self, repository):
diff --git a/lib/lp/charms/tests/test_charmrecipe.py b/lib/lp/charms/tests/test_charmrecipe.py
index a788bc6..a5aac5c 100644
--- a/lib/lp/charms/tests/test_charmrecipe.py
+++ b/lib/lp/charms/tests/test_charmrecipe.py
@@ -1279,6 +1279,40 @@ class TestCharmRecipeSet(TestCaseWithFactory):
             expected_log_entries, logger.getLogBuffer().splitlines())
         self.assertFalse(recipe.is_stale)
 
+    def test_makeAutoBuild_skips_for_unexpected_exceptions(self):
+        # scheduling builds need to be unaffected by one erroring
+        recipe = self.factory.makeCharmRecipe(auto_build=True, is_stale=True)
+        logger = BufferLogger()
+        recipe = removeSecurityProxy(recipe)
+        # currently there is no expected way that `makeAutoBuilds` could fail
+        # so we fake it
+        def fake_requestAutoBuilds_with_side_effect(logger=None):
+            raise Exception("something unexpected went wrong")
+        recipe.requestAutoBuilds = fake_requestAutoBuilds_with_side_effect
+
+        build_requests = getUtility(ICharmRecipeSet).makeAutoBuilds(
+            logger=logger)
+
+        self.assertEqual([], build_requests)
+        self.assertEqual(
+            ['ERROR something unexpected went wrong'],
+            logger.getLogBuffer().splitlines()
+        )
+
+    def test_makeAutoBuild_skips_and_no_logger_enabled(self):
+        # This is basically the same test case as
+        # `test_makeAutoBuild_skips_for_unexpected_exceptions`
+        # but we particularly test with no logger enabled.
+        recipe = self.factory.makeCharmRecipe(auto_build=True, is_stale=True)
+        recipe = removeSecurityProxy(recipe)
+        def fake_requestAutoBuilds_with_side_effect(logger=None):
+            raise Exception("something unexpected went wrong")
+        recipe.requestAutoBuilds = fake_requestAutoBuilds_with_side_effect
+
+        build_requests = getUtility(ICharmRecipeSet).makeAutoBuilds()
+
+        self.assertEqual([], build_requests)
+
     def test_makeAutoBuilds_skips_if_requested_recently(self):
         # ICharmRecipeSet.makeAutoBuilds skips recipes that have been built
         # recently.