launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-copypackageppa-flag into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-copypackageppa-flag/+merge/120135

The soyuz.copypackageppa.enabled feature flag has been enabled everywhere for about a month (https://wiki.canonical.com/InformationInfrastructure/OSA/LaunchpadProductionStatus?action=diff&rev1=2859&rev2=2860) with plenty of positive reports.  The only outstanding bug I know about is https://bugs.launchpad.net/launchpad/+bug/1031089, which I should be able to fix soon.  In the meantime, I don't think there's any reason to expect that we'll need to turn the feature flag off again at this point, so we might as well remove it.
-- 
https://code.launchpad.net/~cjwatson/launchpad/remove-copypackageppa-flag/+merge/120135
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-copypackageppa-flag into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
--- lib/lp/soyuz/browser/tests/archive-views.txt	2012-07-20 10:21:18 +0000
+++ lib/lp/soyuz/browser/tests/archive-views.txt	2012-08-17 10:44:24 +0000
@@ -1393,7 +1393,6 @@
     ...     FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
     ...     )
     >>> fixture = FeatureFixture({
-    ...     'soyuz.copypackageppa.enabled': 'on',
     ...     FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS: '0',
     ...     })
     >>> fixture.setUp()

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2012-08-10 17:08:57 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2012-08-17 10:44:24 +0000
@@ -20,7 +20,6 @@
     'CannotUploadToArchive',
     'CannotUploadToPPA',
     'CannotUploadToPocket',
-    'ForbiddenByFeatureFlag',
     'FULL_COMPONENT_SUPPORT',
     'IArchive',
     'IArchiveAppend',
@@ -125,12 +124,6 @@
     """Exception raised when a copy cannot be performed."""
 
 
-@error_status(httplib.FORBIDDEN)
-class ForbiddenByFeatureFlag(Exception):
-    """Exception raised when using a method protected by a feature flag.
-    """
-
-
 @error_status(httplib.BAD_REQUEST)
 class CannotSwitchPrivacy(Exception):
     """Raised when switching the privacy of an archive that has

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-08-14 19:37:38 +0000
+++ lib/lp/soyuz/model/archive.py	2012-08-17 10:44:24 +0000
@@ -86,7 +86,6 @@
     SQLBase,
     sqlvalues,
     )
-from lp.services.features import getFeatureFlag
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.model import (
     LibraryFileAlias,
@@ -132,7 +131,6 @@
     CannotUploadToPPA,
     ComponentNotFound,
     default_name_by_purpose,
-    ForbiddenByFeatureFlag,
     FULL_COMPONENT_SUPPORT,
     IArchive,
     IArchiveSet,
@@ -1671,21 +1669,10 @@
             [source], to_pocket, to_series, include_binaries,
             person=person)
 
-    def _checkCopyPackageFeatureFlags(self):
-        """Prevent copyPackage(s) if these conditions are not met."""
-        if (self.is_ppa and
-            not getFeatureFlag(u"soyuz.copypackageppa.enabled")):
-            # We have no way of giving feedback about failed jobs yet,
-            # so this is disabled for now.
-            raise ForbiddenByFeatureFlag(
-                "Not enabled for copying to PPAs yet.")
-
     def copyPackage(self, source_name, version, from_archive, to_pocket,
                     person, to_series=None, include_binaries=False,
                     sponsored=None, unembargo=False, auto_approve=False):
         """See `IArchive`."""
-        self._checkCopyPackageFeatureFlags()
-
         # Asynchronously copy a package using the job system.
         pocket = self._text_to_pocket(to_pocket)
         series = self._text_to_series(to_series)
@@ -1712,8 +1699,6 @@
                      include_binaries=None, sponsored=None, unembargo=False,
                      auto_approve=False):
         """See `IArchive`."""
-        self._checkCopyPackageFeatureFlags()
-
         sources = self._collectLatestPublishedSources(
             from_archive, from_series, source_names)
 

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2012-08-14 23:27:07 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2012-08-17 10:44:24 +0000
@@ -34,7 +34,6 @@
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.services.database.sqlbase import sqlvalues
-from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.propertycache import clear_property_cache
 from lp.services.webapp.interfaces import (
@@ -60,7 +59,6 @@
     CannotRestrictArchitectures,
     CannotUploadToPocket,
     CannotUploadToPPA,
-    ForbiddenByFeatureFlag,
     IArchiveSet,
     InsufficientUploadRights,
     InvalidPocketForPartnerArchive,
@@ -2160,39 +2158,10 @@
         self.assertEqual('universe', filtered.component.name)
 
 
-class TestSyncSourceFeatureFlag(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
-
-    def test_copyPackage_to_ppa_requires_feature_flag(self):
-        # Ensure feature is off.
-        self.useFixture(FeatureFixture({u"soyuz.copypackageppa.enabled": ''}))
-        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
-        self.assertRaises(
-            ForbiddenByFeatureFlag,
-            archive.copyPackage,
-            None, None, None, None, None)
-
-    def test_copyPackages_to_ppa_requires_feature_flag(self):
-        # Ensure feature is off.
-        self.useFixture(FeatureFixture({u"soyuz.copypackageppa.enabled": ''}))
-        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
-        self.assertRaises(
-            ForbiddenByFeatureFlag,
-            archive.copyPackages,
-            None, None, None, None, None)
-
-
 class TestSyncSource(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
-    def setUp(self):
-        super(TestSyncSource, self).setUp()
-        self.useFixture(FeatureFixture({
-            u"soyuz.copypackageppa.enabled": 'on',
-            }))
-
     def test_security_team_can_copy_to_primary(self):
         # A member of ubuntu-security can use syncSource on any package
         # in the Ubuntu primary archive, regardless of their normal