launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/supports_mirrors-model into lp:launchpad.

Commit message:
Introduce a new Distribution.supports_mirrors flag to replace the mirror subsystem's usage of the hardcoded Distribution.full_functionality. supports_mirrors is still hardcoded until the DB schema is finished.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/supports_mirrors-model/+merge/225771

A number of features are gated on Distribution.full_functionality, which is hardcoded to only be true for Ubuntu. We need PPAs and official package functionality for the RTM distro, but not distribution mirrors, so let's split full_functionality up into a few flags that separately enable bits of functionality without hardcoding distro names.

This branch introduces a new Distribution.supports_mirrors flag and switches all the mirror enablement checks to use that instead of full_functionality. The new flag just delegates to full_functionality until we get the DB side of things in place.
-- 
https://code.launchpad.net/~wgrant/launchpad/supports_mirrors-model/+merge/225771
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/supports_mirrors-model into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2014-02-26 04:47:33 +0000
+++ lib/lp/registry/browser/distribution.py	2014-07-07 05:08:51 +0000
@@ -252,7 +252,7 @@
     def _userCanSeeNonPublicMirrorListings(self):
         """Does the user have rights to see non-public mirrors listings?"""
         user = getUtility(ILaunchBag).user
-        return (self.distribution.full_functionality
+        return (self.distribution.supports_mirrors
                 and user is not None
                 and user.inTeam(self.distribution.mirror_admin))
 
@@ -352,7 +352,7 @@
 
     def newmirror(self):
         text = 'Register a new mirror'
-        enabled = self.context.full_functionality
+        enabled = self.context.supports_mirrors
         return Link('+newmirror', text, enabled=enabled, icon='add')
 
     def top_contributors(self):
@@ -370,7 +370,7 @@
     def _userCanSeeNonPublicMirrorListings(self):
         """Does the user have rights to see non-public mirrors listings?"""
         user = getUtility(ILaunchBag).user
-        return (self.context.full_functionality
+        return (self.context.supports_mirrors
                 and user is not None
                 and user.inTeam(self.context.mirror_admin))
 
@@ -398,7 +398,7 @@
     @enabled_with_permission('launchpad.Edit')
     def mirror_admin(self):
         text = 'Change mirror admins'
-        enabled = self.context.full_functionality
+        enabled = self.context.supports_mirrors
         return Link('+selectmirroradmins', text, enabled=enabled, icon='edit')
 
     def search(self):
@@ -1050,7 +1050,7 @@
 
     def render(self):
         request = self.request
-        if not self.context.full_functionality:
+        if not self.context.supports_mirrors:
             request.response.setStatus(404)
             return u''
         ip_address = ipaddress_from_request(request)

=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
--- lib/lp/registry/browser/tests/distributionmirror-views.txt	2012-12-10 13:43:47 +0000
+++ lib/lp/registry/browser/tests/distributionmirror-views.txt	2014-07-07 05:08:51 +0000
@@ -4,14 +4,14 @@
 Registering a mirror
 --------------------
 
-Only full functionality distributions may have mirrors.
-IDerivativeDistributions cannot use the form.
+Distributions must have mirror support enabled in order to have mirrors.
+Other distributions cannot use the form.
 
     >>> from lp.testing.pages import (
     ...     extract_text, find_tag_by_id)
 
     >>> distribution = factory.makeDistribution(name='youbuntu')
-    >>> distribution.full_functionality
+    >>> distribution.supports_mirrors
     False
 
     >>> ignored = login_person(distribution.owner)
@@ -21,8 +21,7 @@
     >>> print extract_text(content)
     This functionality is not yet available...
 
-Ubuntu is an IBaseDistributions and is full functionality. It can have
-mirrors.
+Ubuntu has mirror support enabled, so it can have mirrors.
 
     >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2013-08-01 14:43:03 +0000
+++ lib/lp/registry/interfaces/distribution.py	2014-07-07 05:08:51 +0000
@@ -316,6 +316,10 @@
         "this distribution. Currently only Ubuntu and some derivatives "
         "get the full functionality of LP")
 
+    supports_mirrors = Attribute(
+        "Whether we enable mirror management functionality for this "
+        "distribution")
+
     translation_focus = Choice(
         title=_("Translation focus"),
         description=_(

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2014-01-07 01:45:40 +0000
+++ lib/lp/registry/model/distribution.py	2014-07-07 05:08:51 +0000
@@ -546,6 +546,10 @@
         return False
 
     @property
+    def supports_mirrors(self):
+        return self.full_functionality
+
+    @property
     def drivers(self):
         """See `IDistribution`."""
         if self.driver is not None:
@@ -699,7 +703,7 @@
         # the full functionality of Launchpad enabled. This is Ubuntu and
         # commercial derivatives that have been specifically given this
         # ability
-        if not self.full_functionality:
+        if not self.supports_mirrors:
             return None
 
         urls = {'http_base_url': http_base_url,

=== modified file 'lib/lp/registry/scripts/distributionmirror_prober.py'
--- lib/lp/registry/scripts/distributionmirror_prober.py	2013-05-01 22:26:29 +0000
+++ lib/lp/registry/scripts/distributionmirror_prober.py	2014-07-07 05:08:51 +0000
@@ -848,10 +848,10 @@
             # XXX: salgado 2006-05-26:
             # Some people registered mirrors on distros other than Ubuntu back
             # in the old times, so now we need to do this small hack here.
-            if not mirror.distribution.full_functionality:
+            if not mirror.distribution.supports_mirrors:
                 self.logger.debug(
                     "Mirror '%s' of distribution '%s' can't be probed --we "
-                    "only probe Ubuntu mirrors."
+                    "only probe distros that support mirrors.."
                     % (mirror.name, mirror.distribution.name))
                 continue
 

=== modified file 'lib/lp/registry/templates/distribution-mirrors.pt'
--- lib/lp/registry/templates/distribution-mirrors.pt	2011-03-04 00:08:20 +0000
+++ lib/lp/registry/templates/distribution-mirrors.pt	2014-07-07 05:08:51 +0000
@@ -37,7 +37,7 @@
       <p>
         We don't know of any <span tal:replace="view/heading">Archive
         Mirrors</span> for this distribution.
-        <span tal:condition="context/full_functionality">
+        <span tal:condition="context/supports_mirrors">
           If you know of one, please
           <a tal:attributes="href context/fmt:url/+newmirror">register it</a>.
         </span>

=== modified file 'lib/lp/registry/templates/distribution-newmirror.pt'
--- lib/lp/registry/templates/distribution-newmirror.pt	2009-08-25 18:50:54 +0000
+++ lib/lp/registry/templates/distribution-newmirror.pt	2014-07-07 05:08:51 +0000
@@ -7,7 +7,7 @@
   i18n:domain="launchpad">
   <body>
     <div metal:fill-slot="main">
-      <tal:full-functionality tal:condition="context/full_functionality">
+      <tal:full-functionality tal:condition="context/supports_mirrors">
         <div metal:use-macro="context/@@launchpad_form/form">
           <p id="full-functionality" metal:fill-slot="extra_info">
             To register a new mirror you have to provide at least an HTTP or FTP
@@ -25,7 +25,7 @@
         </p>
       </tal:full-functionality>
 
-      <tal:no-full-functionality tal:condition="not: context/full_functionality">
+      <tal:no-full-functionality tal:condition="not: context/supports_mirrors">
         <p id="not-full-functionality" class="informational message">
           This functionality is not yet available for all distributions.
         </p>