launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/no-ppa-ff into lp:launchpad.

Commit message:
Remove the soyuz.ppa.distroful_urls feature flag. The distroful URL is now always canonical.

Requested reviews:
  William Grant (wgrant): code

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-ppa-ff/+merge/228054

Remove the soyuz.ppa.distroful_urls feature flag. The distroful URL is now always canonical.
-- 
https://code.launchpad.net/~wgrant/launchpad/no-ppa-ff/+merge/228054
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2014-07-07 02:30:03 +0000
+++ lib/lp/registry/browser/person.py	2014-07-24 08:15:27 +0000
@@ -192,7 +192,6 @@
 from lp.registry.model.person import get_recipients
 from lp.services.config import config
 from lp.services.database.sqlbase import flush_database_updates
-from lp.services.features import getFeatureFlag
 from lp.services.feeds.browser import FeedsMixin
 from lp.services.geoip.interfaces import IRequestPreferredLanguages
 from lp.services.gpg.interfaces import (
@@ -431,15 +430,14 @@
         #    The distribution is assumed to be "ubuntu" and the PPA "ppa".
         #
         # Only the first is canonical, with the others redirecting to it.
-        distroful_urls = bool(getFeatureFlag('soyuz.ppa.distroful_urls'))
         bits = list(reversed(self.request.getTraversalStack()[-2:]))
         attempts = []
         if len(bits) == 2:
             attempts.append(
-                (bits[0], bits[1], 2, redirect_allowed and not distroful_urls))
+                (bits[0], bits[1], 2, False))
         if len(bits) >= 1:
             attempts.append(
-                ("ubuntu", bits[0], 1, redirect_allowed and distroful_urls))
+                ("ubuntu", bits[0], 1, redirect_allowed))
         attempts.append(("ubuntu", "ppa", 0, True))
 
         # Go through the attempts in order.

=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py	2014-07-07 02:25:33 +0000
+++ lib/lp/registry/browser/tests/test_person.py	2014-07-24 08:15:27 +0000
@@ -41,7 +41,6 @@
 from lp.registry.model.milestone import milestone_sort_key
 from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
 from lp.services.config import config
-from lp.services.features.testing import FeatureFixture
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
 from lp.services.log.logger import FakeLogger
@@ -105,16 +104,8 @@
         archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
         in_suf = '/~%s/+archive/%s/%s' % (
             archive.owner.name, archive.distribution.name, archive.name)
-        old_suf = '/~%s/+archive/%s' % (archive.owner.name, archive.name)
-        # A feature flag makes the distroful URLs canonical.
-        with FeatureFixture({'soyuz.ppa.distroful_urls': 'on'}):
-            self.assertEqual(archive, test_traverse(in_suf)[0])
-            self.assertEqual(archive, test_traverse('/api/devel' + in_suf)[0])
-            self.assertEqual(archive, test_traverse('/api/1.0' + in_suf)[0])
-        # Without the flag set requests will redirect to the old URL,
-        # unless a request is on the 1.0 API.
-        self.assertRedirect(in_suf, old_suf)
-        self.assertRedirect('/api/devel' + in_suf, '/api/devel' + old_suf)
+        self.assertEqual(archive, test_traverse(in_suf)[0])
+        self.assertEqual(archive, test_traverse('/api/devel' + in_suf)[0])
         self.assertEqual(archive, test_traverse('/api/1.0' + in_suf)[0])
 
     def test_traverse_archive_distroless(self):
@@ -123,16 +114,12 @@
         in_suf = '/~%s/+archive/%s' % (archive.owner.name, archive.name)
         out_suf = '/~%s/+archive/%s/%s' % (
             archive.owner.name, archive.distribution.name, archive.name)
-        with FeatureFixture({'soyuz.ppa.distroful_urls': 'on'}):
-            self.assertRedirect(in_suf, out_suf)
-            self.assertRedirect('/api/devel' + in_suf, '/api/devel' + out_suf)
-            # 1.0 API requests don't redirect, since some manually construct
-            # URLs and don't cope with redirects (most notably the Python 2
-            # implementation of apt-add-repository).
-            self.assertEqual(archive, test_traverse('/api/1.0' + out_suf)[0])
-        self.assertEqual(archive, test_traverse(in_suf)[0])
-        self.assertEqual(archive, test_traverse('/api/devel' + in_suf)[0])
-        self.assertEqual(archive, test_traverse('/api/1.0' + in_suf)[0])
+        self.assertRedirect(in_suf, out_suf)
+        self.assertRedirect('/api/devel' + in_suf, '/api/devel' + out_suf)
+        # 1.0 API requests don't redirect, since some manually construct
+        # URLs and don't cope with redirects (most notably the Python 2
+        # implementation of apt-add-repository).
+        self.assertEqual(archive, test_traverse('/api/1.0' + out_suf)[0])
 
     def test_traverse_archive_distroless_implies_ubuntu(self):
         # The distroless PPA redirect only finds Ubuntu PPAs, since
@@ -153,10 +140,9 @@
         in_suf = '/~%s/+archive' % archive.owner.name
         out_suf = '/~%s/+archive/%s/%s' % (
             archive.owner.name, archive.distribution.name, archive.name)
-        with FeatureFixture({'soyuz.ppa.distroful_urls': 'on'}):
-            self.assertRedirect(in_suf, out_suf)
-            self.assertRedirect('/api/devel' + in_suf, '/api/devel' + out_suf)
-            self.assertRedirect('/api/1.0' + in_suf, '/api/1.0' + out_suf)
+        self.assertRedirect(in_suf, out_suf)
+        self.assertRedirect('/api/devel' + in_suf, '/api/devel' + out_suf)
+        self.assertRedirect('/api/1.0' + in_suf, '/api/1.0' + out_suf)
 
 
 class PersonViewOpenidIdentityUrlTestCase(TestCaseWithFactory):

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2014-07-09 03:08:57 +0000
+++ lib/lp/soyuz/browser/archive.py	2014-07-24 08:15:27 +0000
@@ -95,7 +95,6 @@
     get_user_agent_distroseries,
     )
 from lp.services.database.bulk import load_related
-from lp.services.features import getFeatureFlag
 from lp.services.helpers import english_list
 from lp.services.job.model.job import Job
 from lp.services.librarian.browser import FileNavigationMixin
@@ -230,11 +229,8 @@
 
     @property
     def path(self):
-        if getFeatureFlag('soyuz.ppa.distroful_urls'):
-            return u"+archive/%s/%s" % (
-                self.context.distribution.name, self.context.name)
-        else:
-            return u"+archive/%s" % self.context.name
+        return u"+archive/%s/%s" % (
+            self.context.distribution.name, self.context.name)
 
 
 class ArchiveNavigation(Navigation, FileNavigationMixin):