launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-recipe-quota into lp:launchpad.

Commit message:
Remove the hardcoded five-a-day recipe build quota.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-recipe-quota/+merge/257845

Remove the hardcoded five-a-day recipe build quota.  It's an occasional support burden and is generally more trouble than it's worth now that the PPA build farm is much more capable.  We have other ways to deal with abuse if that becomes necessary.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-recipe-quota into lp:launchpad.

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2014-11-16 09:49:59 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2015-04-30 02:20:23 +0000
@@ -89,7 +89,6 @@
     BuildAlreadyPending,
     NoSuchBranch,
     PrivateBranchRecipe,
-    TooManyBuilds,
     TooNewRecipeFormat,
     )
 from lp.code.interfaces.branchtarget import IBranchTarget
@@ -390,15 +389,6 @@
                 "You need to specify at least one distro series for which "
                 "to build.")
             return
-        over_quota_distroseries = []
-        for distroseries in data['distroseries']:
-            if self.context.isOverQuota(self.user, distroseries):
-                over_quota_distroseries.append(str(distroseries))
-        if len(over_quota_distroseries) > 0:
-            self.setFieldError(
-                'distroseries',
-                "You have exceeded today's quota for %s." %
-                ', '.join(over_quota_distroseries))
 
     def requestBuild(self, data):
         """User action for requesting a number of builds.
@@ -527,7 +517,7 @@
         recipe = self.context
         try:
             builds = recipe.performDailyBuild()
-        except (TooManyBuilds, ArchiveDisabled) as e:
+        except ArchiveDisabled as e:
             self.request.response.addErrorNotification(str(e))
             self.next_url = canonical_url(recipe)
             return

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2015-04-20 09:48:57 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2015-04-30 02:20:23 +0000
@@ -1384,24 +1384,6 @@
             set([2505]),
             set(build.buildqueue_record.lastscore for build in builds))
 
-    def test_request_daily_builds_action_over_quota(self):
-        recipe = self.factory.makeSourcePackageRecipe(
-            owner=self.chef, daily_build_archive=self.ppa,
-            name=u'julia', is_stale=True, build_daily=True)
-        # Create some previous builds.
-        series = list(recipe.distroseries)[0]
-        for x in xrange(5):
-            build = recipe.requestBuild(
-                self.ppa, self.chef, series, PackagePublishingPocket.RELEASE)
-            build.updateStatus(BuildStatus.FULLYBUILT)
-        harness = LaunchpadFormHarness(
-            recipe, SourcePackageRecipeRequestDailyBuildView)
-        harness.submit('build', {})
-        self.assertEqual(
-            "You have exceeded your quota for recipe chef/julia "
-            "for distroseries ubuntu warty",
-            harness.view.request.notifications[0].message)
-
     def test_request_daily_builds_disabled_archive(self):
         # Requesting a daily build from a disabled archive is a user error.
         recipe = self.factory.makeSourcePackageRecipe(
@@ -1509,24 +1491,8 @@
             self.factory.makePerson(), supports_virtualized=True)
         return woody
 
-    def test_request_build_rejects_over_quota(self):
-        """Over-quota build requests cause validation failures."""
-        woody = self._makeWoodyDistroSeries()
-        recipe = self.makeRecipe()
-        for x in range(5):
-            build = recipe.requestBuild(
-                self.ppa, self.chef, woody, PackagePublishingPocket.RELEASE)
-            build.updateStatus(BuildStatus.FULLYBUILT)
-
-        browser = self.getViewBrowser(recipe, '+request-builds')
-        browser.getControl('Woody').click()
-        browser.getControl('Request builds').click()
-        self.assertIn(
-            html_escape("You have exceeded today's quota for ubuntu woody."),
-            extract_text(find_main_content(browser.contents)))
-
     def test_request_builds_rejects_duplicate(self):
-        """Over-quota build requests cause validation failures."""
+        """Duplicate build requests cause validation failures."""
         woody = self._makeWoodyDistroSeries()
         recipe = self.makeRecipe()
         recipe.requestBuild(

=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py	2015-04-29 12:55:48 +0000
+++ lib/lp/code/errors.py	2015-04-30 02:20:23 +0000
@@ -46,7 +46,6 @@
     'PrivateBranchRecipe',
     'ReviewNotPending',
     'StaleLastMirrored',
-    'TooManyBuilds',
     'TooNewRecipeFormat',
     'UnknownBranchTypeError',
     'UpdatePreviewDiffNotReady',
@@ -465,16 +464,6 @@
         Exception.__init__(self, msg)
 
 
-class TooManyBuilds(RecipeBuildException):
-    """A build was requested that exceeded the quota."""
-
-    def __init__(self, recipe, distroseries):
-        RecipeBuildException.__init__(
-            self, recipe, distroseries,
-            'You have exceeded your quota for recipe %(recipe)s for'
-            ' distroseries %(distroseries)s')
-
-
 class BuildAlreadyPending(RecipeBuildException):
     """A build was requested when an identical build was already pending."""
 

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py	2014-07-09 03:08:57 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py	2015-04-30 02:20:23 +0000
@@ -135,13 +135,6 @@
             title=_("The the most recent build of this recipe."),
             readonly=True))
 
-    def isOverQuota(requester, distroseries):
-        """True if the recipe/requester/distroseries combo is >= quota.
-
-        :param requester: The Person requesting a build.
-        :param distroseries: The distroseries to build for.
-        """
-
     @call_with(requester=REQUEST_USER)
     @operation_parameters(
         archive=Reference(schema=IArchive),

=== modified file 'lib/lp/code/interfaces/webservice.py'
--- lib/lp/code/interfaces/webservice.py	2015-04-19 12:56:32 +0000
+++ lib/lp/code/interfaces/webservice.py	2015-04-30 02:20:23 +0000
@@ -33,7 +33,6 @@
     'IPreviewDiff',
     'ISourcePackageRecipe',
     'ISourcePackageRecipeBuild',
-    'TooManyBuilds',
     ]
 
 # The exceptions are imported so that they can produce the special
@@ -46,7 +45,6 @@
     BuildAlreadyPending,
     CodeImportAlreadyRunning,
     CodeImportNotInReviewedState,
-    TooManyBuilds,
     )
 from lp.code.interfaces.branch import (
     IBranch,

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2015-02-24 13:46:46 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2015-04-30 02:20:23 +0000
@@ -40,7 +40,6 @@
 from lp.code.errors import (
     BuildAlreadyPending,
     BuildNotAllowedForDistro,
-    TooManyBuilds,
     )
 from lp.code.interfaces.sourcepackagerecipe import (
     ISourcePackageRecipe,
@@ -254,11 +253,6 @@
         store.remove(self._recipe_data)
         store.remove(self)
 
-    def isOverQuota(self, requester, distroseries):
-        """See `ISourcePackageRecipe`."""
-        return SourcePackageRecipeBuild.getRecentBuilds(
-            requester, self, distroseries).count() >= 5
-
     def containsUnbuildableSeries(self, archive):
         buildable_distros = set(
             BuildableDistroSeries.findSeries(archive.owner))
@@ -280,8 +274,6 @@
             pocket)
         if reject_reason is not None:
             raise reject_reason
-        if self.isOverQuota(requester, distroseries):
-            raise TooManyBuilds(self, distroseries)
         pending = IStore(self).find(SourcePackageRecipeBuild,
             SourcePackageRecipeBuild.recipe_id == self.id,
             SourcePackageRecipeBuild.distroseries_id == distroseries.id,

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2013-11-28 05:13:37 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2015-04-30 02:20:23 +0000
@@ -8,10 +8,7 @@
     'SourcePackageRecipeBuild',
     ]
 
-from datetime import (
-    datetime,
-    timedelta,
-    )
+from datetime import timedelta
 import logging
 
 from psycopg2 import ProgrammingError
@@ -318,16 +315,6 @@
                 bfj.id for bfj in build_farm_jobs))
         return DecoratedResultSet(rows, pre_iter_hook=cls.preloadBuildsData)
 
-    @classmethod
-    def getRecentBuilds(cls, requester, recipe, distroseries, _now=None):
-        if _now is None:
-            _now = datetime.now(pytz.UTC)
-        store = IMasterStore(SourcePackageRecipeBuild)
-        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,
-            cls.date_created > old_threshold)
-
     def estimateDuration(self):
         """See `IPackageBuild`."""
         median = self.recipe.getMedianBuildDuration()

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2014-09-01 03:45:08 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2015-04-30 02:20:23 +0000
@@ -32,7 +32,6 @@
 from lp.code.errors import (
     BuildAlreadyPending,
     PrivateBranchRecipe,
-    TooManyBuilds,
     TooNewRecipeFormat,
     )
 from lp.code.interfaces.sourcepackagerecipe import (
@@ -406,24 +405,6 @@
         queue_record.score()
         self.assertEqual(2705, queue_record.lastscore)
 
-    def test_requestBuildRejectsOverQuota(self):
-        """Build requests that exceed quota raise an exception."""
-        requester = self.factory.makePerson(name='requester')
-        recipe = self.factory.makeSourcePackageRecipe(
-            name=u'myrecipe', owner=requester)
-        series = list(recipe.distroseries)[0]
-        archive = self.factory.makeArchive(owner=requester)
-
-        def request_build():
-            build = recipe.requestBuild(archive, requester, series,
-                    PackagePublishingPocket.RELEASE)
-            build.updateStatus(BuildStatus.FULLYBUILT)
-        [request_build() for num in range(5)]
-        e = self.assertRaises(TooManyBuilds, request_build)
-        self.assertIn(
-            'You have exceeded your quota for recipe requester/myrecipe',
-            str(e))
-
     def test_requestBuildRejectRepeats(self):
         """Reject build requests that are identical to pending builds."""
         recipe = self.factory.makeSourcePackageRecipe()
@@ -1120,26 +1101,6 @@
         self.assertIn(
             'An identical build of this recipe is already pending.', str(e))
 
-    def test_requestBuildRejectOverQuota(self):
-        """Build requests are rejected if they exceed quota."""
-        person = self.factory.makePerson()
-        archives = [self.factory.makeArchive(owner=person) for x in range(6)]
-        distroseries = self.factory.makeSourcePackageRecipeDistroseries()
-
-        recipe, user, launchpad = self.makeRecipe(person)
-        distroseries = ws_object(launchpad, distroseries)
-        for archive in archives[:-1]:
-            archive = ws_object(launchpad, archive)
-            recipe.requestBuild(
-                archive=archive, distroseries=distroseries,
-                pocket=PackagePublishingPocket.RELEASE.title)
-
-        archive = ws_object(launchpad, archives[-1])
-        e = self.assertRaises(Exception, recipe.requestBuild,
-            archive=archive, distroseries=distroseries,
-            pocket=PackagePublishingPocket.RELEASE.title)
-        self.assertIn('You have exceeded your quota', str(e))
-
     def test_requestBuildRejectUnsupportedDistroSeries(self):
         """Build requests are rejected if they have a bad distroseries."""
         person = self.factory.makePerson()

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2015-04-20 09:48:57 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2015-04-30 02:20:23 +0000
@@ -406,40 +406,6 @@
             "DEBUG  - cannot build against Warty (4.10).",
             logger.getLogBuffer())
 
-    def test_getRecentBuilds(self):
-        """Recent builds match the same person, series and receipe.
-
-        Builds do not match if they are older than 24 hours, or have a
-        different requester, series or recipe.
-        """
-        requester = self.factory.makePerson()
-        recipe = self.factory.makeSourcePackageRecipe()
-        series = self.factory.makeDistroSeries()
-        removeSecurityProxy(series).nominatedarchindep = (
-            self.factory.makeDistroArchSeries(distroseries=series))
-        now = self.factory.getUniqueDate()
-        build = self.factory.makeSourcePackageRecipeBuild(recipe=recipe,
-            requester=requester)
-        self.factory.makeSourcePackageRecipeBuild(
-            recipe=recipe, distroseries=series)
-        self.factory.makeSourcePackageRecipeBuild(
-            requester=requester, distroseries=series)
-
-        def get_recent():
-            Store.of(build).flush()
-            return SourcePackageRecipeBuild.getRecentBuilds(
-                requester, recipe, series, _now=now)
-        self.assertContentEqual([], get_recent())
-        yesterday = now - timedelta(days=1)
-        self.factory.makeSourcePackageRecipeBuild(
-            recipe=recipe, distroseries=series, requester=requester,
-            date_created=yesterday)
-        self.assertContentEqual([], get_recent())
-        more_recent_build = self.factory.makeSourcePackageRecipeBuild(
-            recipe=recipe, distroseries=series, requester=requester,
-            date_created=yesterday + timedelta(seconds=1))
-        self.assertContentEqual([more_recent_build], get_recent())
-
     def test_destroySelf(self):
         # ISourcePackageRecipeBuild should make sure to remove jobs and build
         # queue entries and then invalidate itself.