launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/recipe-feature-flag into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #720503 recipes should use feature flags exclusively
  https://bugs.launchpad.net/bugs/720503

For more details, see:
https://code.launchpad.net/~thumper/launchpad/recipe-feature-flag/+merge/50092

Use feature flags exclusively for controlling recipes.

Generally replaced the recipes_enabled method with a direct check
of the feature flag.

There is no point checking now in requestBuild.

lib/lp/code/browser/branch.py
 - the link is only shown on the template if the feature flag is set
   so no need to check it twice

-- 
https://code.launchpad.net/~thumper/launchpad/recipe-feature-flag/+merge/50092
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/recipe-feature-flag into lp:launchpad.

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2011-02-02 15:43:31 +0000
+++ lib/lp/code/browser/branch.py	2011-02-17 03:23:59 +0000
@@ -143,13 +143,14 @@
 from lp.code.interfaces.branchnamespace import IBranchNamespacePolicy
 from lp.code.interfaces.branchtarget import IBranchTarget
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
-from lp.code.interfaces.sourcepackagerecipe import recipes_enabled
+from lp.code.interfaces.sourcepackagerecipe import RECIPE_ENABLED_FLAG
 from lp.registry.interfaces.person import (
     IPerson,
     IPersonSet,
     )
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.vocabularies import UserTeamsParticipationPlusSelfVocabulary
+from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.translations.interfaces.translationtemplatesbuild import (
     ITranslationTemplatesBuildSource,
@@ -385,10 +386,8 @@
             '+upgrade', 'Upgrade this branch', icon='edit', enabled=enabled)
 
     def create_recipe(self):
-        if not self.context.private and recipes_enabled():
-            enabled = True
-        else:
-            enabled = False
+        # You can't create a recipe for a private branch.
+        enabled = not self.context.private
         text = 'Create packaging recipe'
         return Link('+new-recipe', text, enabled=enabled, icon='add')
 

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2011-02-15 11:31:56 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2011-02-17 03:23:59 +0000
@@ -90,8 +90,10 @@
     ISourcePackageRecipe,
     ISourcePackageRecipeSource,
     MINIMAL_RECIPE_TEXT,
-    )
+    RECIPE_BETA_FLAG,
+   )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.soyuz.model.archive import Archive
 
@@ -184,10 +186,9 @@
     """Default view of a SourcePackageRecipe."""
 
     def initialize(self):
-        # XXX: rockstar: This should be removed when source package recipes
-        # are put into production. spec=sourcepackagerecipes
         super(SourcePackageRecipeView, self).initialize()
-        self.request.response.addWarningNotification(RECIPE_BETA_MESSAGE)
+        if getFeatureFlag(RECIPE_BETA_FLAG):
+            self.request.response.addWarningNotification(RECIPE_BETA_MESSAGE)
         recipe = self.context
         if recipe.build_daily and recipe.daily_build_archive is None:
             self.request.response.addWarningNotification(
@@ -510,9 +511,8 @@
 
     def initialize(self):
         super(SourcePackageRecipeAddView, self).initialize()
-        # XXX: rockstar: This should be removed when source package recipes
-        # are put into production. spec=sourcepackagerecipes
-        self.request.response.addWarningNotification(RECIPE_BETA_MESSAGE)
+        if getFeatureFlag(RECIPE_BETA_FLAG):
+           self.request.response.addWarningNotification(RECIPE_BETA_MESSAGE)
         widget = self.widgets['use_ppa']
         current_value = widget._getFormValue()
         self.use_ppa_existing = render_radio_widget_part(

=== modified file 'lib/lp/code/browser/sourcepackagerecipelisting.py'
--- lib/lp/code/browser/sourcepackagerecipelisting.py	2011-01-20 23:48:36 +0000
+++ lib/lp/code/browser/sourcepackagerecipelisting.py	2011-02-17 03:23:59 +0000
@@ -19,7 +19,8 @@
     LaunchpadView,
     Link,
     )
-from lp.code.interfaces.sourcepackagerecipe import recipes_enabled
+from lp.code.interfaces.sourcepackagerecipe import RECIPE_ENABLED_FLAG
+from lp.services.features import getFeatureFlag
 
 
 class HasRecipesMenuMixin:
@@ -30,7 +31,7 @@
         enabled = False
         if self.context.getRecipes().count():
             enabled = True
-        if not recipes_enabled():
+        if not getFeatureFlag(RECIPE_ENABLED_FLAG):
             enabled = False
         return Link(
             '+recipes', text, icon='info', enabled=enabled, site='code')

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py	2011-02-14 01:48:57 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py	2011-02-17 03:23:59 +0000
@@ -14,7 +14,8 @@
     'ISourcePackageRecipeData',
     'ISourcePackageRecipeSource',
     'MINIMAL_RECIPE_TEXT',
-    'recipes_enabled',
+    'RECIPE_BETA_FLAG',
+    'RECIPE_ENABLED_FLAG',
     ]
 
 
@@ -53,7 +54,6 @@
 from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.role import IHasOwner
-from lp.services import features
 from lp.services.fields import (
     Description,
     PersonChoice,
@@ -62,6 +62,9 @@
 from lp.soyuz.interfaces.archive import IArchive
 
 
+RECIPE_ENABLED_FLAG = u'code.recipes_enabled'
+RECIPE_BETA_FLAG = u'code.recipes.beta'
+
 MINIMAL_RECIPE_TEXT = dedent(u'''\
     # bzr-builder format 0.3 deb-version {debupstream}-0~{revno}
     %s
@@ -212,13 +215,3 @@
 
     def exists(owner, name):
         """Check to see if a recipe by the same name and owner exists."""
-
-
-def recipes_enabled():
-    """Return True if recipes are enabled."""
-    # Features win:
-    if features.getFeatureFlag(u'code.recipes_enabled'):
-        return True
-    if not config.build_from_branch.enabled:
-        return False
-    return True

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2011-02-03 03:39:14 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2011-02-17 03:23:59 +0000
@@ -56,7 +56,6 @@
     ISourcePackageRecipe,
     ISourcePackageRecipeData,
     ISourcePackageRecipeSource,
-    recipes_enabled,
     )
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuildSource,
@@ -236,8 +235,6 @@
     def requestBuild(self, archive, requester, distroseries, pocket,
                      manual=False):
         """See `ISourcePackageRecipe`."""
-        if not recipes_enabled():
-            raise ValueError('Source package recipe builds disabled.')
         if not archive.is_ppa:
             raise NonPPABuildRequest
 

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2011-02-13 22:07:27 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2011-02-17 03:23:59 +0000
@@ -39,7 +39,6 @@
     ISourcePackageRecipe,
     ISourcePackageRecipeSource,
     MINIMAL_RECIPE_TEXT,
-    recipes_enabled,
     )
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuild,
@@ -72,7 +71,6 @@
     login,
     login_person,
     person_logged_in,
-    set_feature_flag,
     TestCaseWithFactory,
     ws_object,
     )
@@ -385,15 +383,6 @@
             ValueError, recipe.requestBuild, ppa, ppa.owner, distroseries,
             PackagePublishingPocket.RELEASE)
 
-    def test_recipes_enabled_config(self):
-        self.pushConfig('build_from_branch', enabled=False)
-        self.assertFalse(recipes_enabled())
-
-    def test_recipes_enabled_flag(self):
-        self.pushConfig('build_from_branch', enabled=False)
-        set_feature_flag(u'code.recipes_enabled', u'on')
-        self.assertTrue(recipes_enabled())
-
     def test_requestBuildRejectsOverQuota(self):
         """Build requests that exceed quota raise an exception."""
         requester = self.factory.makePerson(name='requester')

=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt	2010-11-08 14:11:57 +0000
+++ lib/lp/code/templates/branch-index.pt	2011-02-17 03:23:59 +0000
@@ -111,7 +111,7 @@
            replace="structure context/@@++branch-pending-merges" />
       <tal:branch-recipes
            replace="structure context/@@++branch-recipes"
-           condition="modules/lp.code.interfaces.sourcepackagerecipe/recipes_enabled"
+           condition="features/code.recipes_enabled"
       />
       <tal:related-bugs-specs
            replace="structure context/@@++branch-related-bugs-specs" />

=== modified file 'lib/lp/code/templates/branch-recipes.pt'
--- lib/lp/code/templates/branch-recipes.pt	2010-10-31 22:27:36 +0000
+++ lib/lp/code/templates/branch-recipes.pt	2011-02-17 03:23:59 +0000
@@ -25,7 +25,8 @@
       tal:condition="link/enabled"
       tal:replace="structure link/render"
       />
-    <img src="/@@/beta" alt="beta" title="This feature is in beta" />
+    <img src="/@@/beta" alt="beta" title="This feature is in beta"
+         tal:condition="features/code.recipes.beta"/>
   </div>
 
 </div>

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-02-14 18:21:01 +0000
+++ lib/lp/services/features/flags.py	2011-02-17 03:23:59 +0000
@@ -24,6 +24,10 @@
      '[on|off]',
      'enable recipes',
      'off'),
+    ('code.recipes.beta',
+     '[true|false]',
+     'True if recipes are still in beta',
+     'false'),
     ('hard_timeout',
      'float',
      'sets the hard timeout in seconds',