launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/get_derived_distros into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #788078 in Launchpad itself: "Allow the Soyuz publisher to optionally publish only non-Ubuntu distros"
  https://bugs.launchpad.net/launchpad/+bug/788078

For more details, see:
https://code.launchpad.net/~jtv/launchpad/get_derived_distros/+merge/68675

= Summary =

I recently added an option to process-accepted: "process all derived distributions (not counting Ubuntu, even if it is technically derived from Ubuntu)."

Now I'm adding a similar option to publish-distro, so the ability to find all derived distributions should be in a reusable place.


== Proposed fix ==

IDistributionSet.getDerivedDistributions().


== Pre-implementation notes ==

The notion "derived distributions" is a bit vague in this case.  It doesn't matter for the foreseeable future; we're currently thinking "Ubuntu-derived distros" but we're not planning to set up any other derived distros (except perhaps Ubuntu as a derivative of Debian).

It's important that Ubuntu not be included in the list of "derived distributions" because it's large and needs to be processed separately.  That's the whole point.


== Implementation details ==

I was unable to find any unit tests for DistributionSet, so I added one.


== Tests ==

{{{
./bin/test -vvc lp.registry.tests.test_distribution -t DistributionSet
./bin/test -vvc lp.soyuz.tests.test_processaccepted
}}}


== Demo and Q/A ==

There isn't much to demo or Q/A, really; existing code has moved a bit.  Just so long as "process-accepted --derived" still runs.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/scripts/processaccepted.py
  lib/lp/soyuz/tests/test_processaccepted.py
  lib/lp/registry/interfaces/distribution.py
  lib/lp/registry/tests/test_distribution.py
  lib/lp/registry/model/distribution.py
-- 
https://code.launchpad.net/~jtv/launchpad/get_derived_distros/+merge/68675
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/get_derived_distros into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2011-07-13 18:18:42 +0000
+++ lib/lp/registry/interfaces/distribution.py	2011-07-21 12:46:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -750,6 +750,13 @@
         :return: A dict as per `IDistribution.getCurrentSourceReleases`
         """
 
+    def getDerivedDistributions():
+        """Find derived distributions.
+
+        :return: An iterable of all derived distributions (not including
+            Ubuntu, even if it is technically derived from Debian).
+        """
+
 
 class NoSuchDistribution(NameLookupFailed):
     """Raised when we try to find a distribution that doesn't exist."""

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-07-14 14:08:04 +0000
+++ lib/lp/registry/model/distribution.py	2011-07-21 12:46:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -157,6 +157,7 @@
     DistributionSourcePackage,
     )
 from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.distroseriesparent import DistroSeriesParent
 from lp.registry.model.hasdrivers import HasDriversMixin
 from lp.registry.model.karma import KarmaContextMixin
 from lp.registry.model.milestone import (
@@ -1993,3 +1994,12 @@
             result[sourcepackage] = DistributionSourcePackageRelease(
                 distro, sp_release)
         return result
+
+    def getDerivedDistributions(self):
+        """See `IDistributionSet`."""
+        ubuntu_id = getUtility(ILaunchpadCelebrities).ubuntu.id
+        return IStore(DistroSeries).find(
+            Distribution,
+            Distribution.id == DistroSeries.distributionID,
+            DistroSeries.id == DistroSeriesParent.derived_series_id,
+            DistroSeries.distributionID != ubuntu_id).config(distinct=True)

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2011-06-17 19:47:36 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-07-21 12:46:33 +0000
@@ -20,10 +20,15 @@
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
+    ZopelessDatabaseLayer,
     )
 from lp.app.errors import NotFoundError
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.errors import NoSuchDistroSeries
-from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distribution import (
+    IDistribution,
+    IDistributionSet,
+    )
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.tests.test_distroseries import (
@@ -37,6 +42,7 @@
     login_person,
     TestCaseWithFactory,
     )
+from lp.testing.matchers import Provides
 from lp.testing.views import create_initialized_view
 
 
@@ -421,3 +427,43 @@
         self.assertNotEqual(distribution.owner, distribution.registrant)
         self.assertEqual(distribution.owner, self.owner)
         self.assertEqual(distribution.registrant, self.registrant)
+
+
+class DistributionSet(TestCaseWithFactory):
+    """Test case for `IDistributionSet`."""
+
+    layer = ZopelessDatabaseLayer
+
+    def test_implements_interface(self):
+        self.assertThat(
+            getUtility(IDistributionSet), Provides(IDistributionSet))
+
+    def test_getDerivedDistributions_finds_derived_distro(self):
+        dsp = self.factory.makeDistroSeriesParent()
+        derived_distro = dsp.derived_series.distribution
+        distroset = getUtility(IDistributionSet)
+        self.assertIn(derived_distro, distroset.getDerivedDistributions())
+
+    def test_getDerivedDistributions_ignores_nonderived_distros(self):
+        distroset = getUtility(IDistributionSet)
+        nonderived_distro = self.factory.makeDistribution()
+        self.assertNotIn(
+            nonderived_distro, distroset.getDerivedDistributions())
+
+    def test_getDerivedDistributions_ignores_ubuntu_even_if_derived(self):
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        self.factory.makeDistroSeriesParent(
+            derived_series=ubuntu.currentseries)
+        distroset = getUtility(IDistributionSet)
+        self.assertNotIn(ubuntu, distroset.getDerivedDistributions())
+
+    def test_getDerivedDistribution_finds_each_distro_just_once(self):
+        # Derived distros are not duplicated in the output of
+        # getDerivedDistributions, even if they have multiple parents and
+        # multiple derived series.
+        dsp = self.factory.makeDistroSeriesParent()
+        distro = dsp.derived_series.distribution
+        other_series = self.factory.makeDistroSeries(distribution=distro)
+        self.factory.makeDistroSeriesParent(derived_series=other_series)
+        distroset = getUtility(IDistributionSet)
+        self.assertEqual(1, len(list(distroset.getDerivedDistributions())))

=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py	2011-07-14 16:51:59 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py	2011-07-21 12:46:33 +0000
@@ -19,7 +19,6 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.webapp.errorlog import (
     ErrorReportingUtility,
     ScriptRequest,
@@ -31,7 +30,6 @@
 from lp.bugs.interfaces.bugtask import BugTaskStatus
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.model.distroseriesparent import DistroSeriesParent
 from lp.services.scripts.base import (
     LaunchpadCronScript,
     LaunchpadScriptFailure,
@@ -287,19 +285,6 @@
         else:
             self.txn.commit()
 
-    def findDerivedDistros(self):
-        """Find Ubuntu-derived distributions."""
-        # Avoid circular imports.
-        from lp.registry.model.distribution import Distribution
-        from lp.registry.model.distroseries import DistroSeries
-
-        ubuntu_id = getUtility(ILaunchpadCelebrities).ubuntu.id
-        return IStore(DistroSeries).find(
-            Distribution,
-            Distribution.id == DistroSeries.distributionID,
-            DistroSeries.id == DistroSeriesParent.derived_series_id,
-            DistroSeries.distributionID != ubuntu_id).config(distinct=True)
-
     def findNamedDistro(self, distro_name):
         """Find the `Distribution` called `distro_name`."""
         self.logger.debug("Finding distribution %s.", distro_name)
@@ -312,7 +297,7 @@
     def findTargetDistros(self):
         """Find the distribution(s) to process, based on arguments."""
         if self.options.derived:
-            return self.findDerivedDistros()
+            return getUtility(IDistributionSet).getDerivedDistributions()
         else:
             return [self.findNamedDistro(self.args[0])]
 

=== modified file 'lib/lp/soyuz/tests/test_processaccepted.py'
--- lib/lp/soyuz/tests/test_processaccepted.py	2011-07-14 16:54:41 +0000
+++ lib/lp/soyuz/tests/test_processaccepted.py	2011-07-21 12:46:33 +0000
@@ -4,7 +4,6 @@
 """Test process-accepted.py"""
 
 from cStringIO import StringIO
-from zope.component import getUtility
 
 from canonical.launchpad.interfaces.lpstorm import IStore
 from debian.deb822 import Changes
@@ -14,7 +13,6 @@
 from canonical.config import config
 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
 from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.log.logger import BufferLogger
 from lp.services.scripts.base import LaunchpadScriptFailure
@@ -223,32 +221,7 @@
         dsp = self.factory.makeDistroSeriesParent()
         script = ProcessAccepted(test_args=['--derived'])
         self.assertIn(
-            dsp.derived_series.distribution, script.findDerivedDistros())
-
-    def test_findDerivedDistros_for_derived_ignores_non_derived_distros(self):
-        distro = self.factory.makeDistribution()
-        script = ProcessAccepted(test_args=['--derived'])
-        self.assertNotIn(distro, script.findDerivedDistros())
-
-    def test_findDerivedDistros_ignores_ubuntu_even_if_derived(self):
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        self.factory.makeDistroSeriesParent(
-            derived_series=ubuntu.currentseries)
-        script = ProcessAccepted(test_args=['--derived'])
-        self.assertNotIn(ubuntu, script.findDerivedDistros())
-
-    def test_findDerivedDistros_finds_each_distro_just_once(self):
-        # Derived distros are not duplicated in the output of
-        # findDerivedDistros, even if they have multiple parents and
-        # multiple derived series.
-        dsp = self.factory.makeDistroSeriesParent()
-        distro = dsp.derived_series.distribution
-        other_series = self.factory.makeDistroSeries(distribution=distro)
-        self.factory.makeDistroSeriesParent(derived_series=other_series)
-        script = ProcessAccepted(test_args=['--derived'])
-        derived_distros = list(script.findDerivedDistros())
-        self.assertEqual(
-            1, derived_distros.count(dsp.derived_series.distribution))
+            dsp.derived_series.distribution, script.findTargetDistros())
 
 
 class TestBugsFromChangesFile(TestCaseWithFactory):