launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/stop-publishing-obsolete-series into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #905281 in Launchpad itself: "Publisher spends time considering series that can't possibly be relevant"
  https://bugs.launchpad.net/launchpad/+bug/905281

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/stop-publishing-obsolete-series/+merge/86967

== Summary ==

About 15 seconds of the primary archive publisher's runtime is spent considering obsolete distroseries.  It's not a huge amount, but it would be nice to shave this off.

== Proposed fix ==

Exclude OBSOLETE and FUTURE series when publishing PRIMARY and PARTNER archives.  The archive purpose restriction is because apparently OEM still has PPAs for obsolete series, as noted by William Grant pre-implementation.

== Tests ==

bin/test -vvct archivepublisher.tests.test_publisher.TestPublisher

== Demo and Q/A ==

Run the publisher on mawson.  Given the current state of the dogfood database, the publisher log under "Step A: Publishing packages" should show it considering rusty, precise, oneiric, natty, maverick, lucid, newdog, hardy, testagain, and dozing, but not q-series, karmic, jaunty, intrepid, gutsy, feisty, edgy, dapper, breezy, hoary, or warty.

== lint ==

None.
-- 
https://code.launchpad.net/~cjwatson/launchpad/stop-publishing-obsolete-series/+merge/86967
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/stop-publishing-obsolete-series into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2011-10-18 09:13:51 +0000
+++ lib/lp/archivepublisher/publishing.py	2011-12-27 17:37:27 +0000
@@ -41,6 +41,7 @@
     RepositoryIndexFile,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.utils import file_exists
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -236,7 +237,28 @@
         """
         self.log.debug("* Step A: Publishing packages")
 
-        for distroseries in self.distro.series:
+        if self.archive.purpose in (
+            ArchivePurpose.PRIMARY,
+            ArchivePurpose.PARTNER,
+            ):
+            # For PRIMARY and PARTNER archives, skip OBSOLETE and FUTURE
+            # series.  We will never want to publish anything in them, so it
+            # isn't worth thinking about whether they have pending
+            # publications.
+            consider_series = [
+                series
+                for series in self.distro.series
+                if series.status not in (
+                    SeriesStatus.OBSOLETE,
+                    SeriesStatus.FUTURE,
+                    )]
+        else:
+            # Other archives may have reasons to continue building at least
+            # for OBSOLETE series.  For example, a PPA may be continuing to
+            # provide custom builds for users who haven't upgraded yet.
+            consider_series = self.distro.series
+
+        for distroseries in consider_series:
             for pocket in self.archive.getPockets():
                 allowed = (
                     not self.allowed_suites or

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2011-12-09 01:35:17 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2011-12-27 17:37:27 +0000
@@ -57,6 +57,7 @@
 from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.tests.test_publishing import TestNativePublishingBase
 from lp.testing import TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
 from lp.testing.gpgkeys import gpgkeysdir
 from lp.testing.keyserver import KeyServerTac
 
@@ -431,6 +432,60 @@
         # remove locally created dir
         shutil.rmtree(test_pool_dir)
 
+    def testPublishingSkipsObsoletePrimarySeries(self):
+        """Publisher skips OBSOLETE series in PRIMARY archives."""
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+        # Remove security proxy so that the publisher can call our fake
+        # method.
+        publisher.distro = removeSecurityProxy(publisher.distro)
+
+        naked_breezy_autotest = publisher.distro['breezy-autotest']
+        naked_breezy_autotest.status = SeriesStatus.OBSOLETE
+        naked_breezy_autotest.publish = FakeMethod(result=set())
+
+        publisher.A_publish(False)
+
+        self.assertEqual(0, naked_breezy_autotest.publish.call_count)
+
+    def testPublishingSkipsFuturePrimarySeries(self):
+        """Publisher skips FUTURE series in PRIMARY archives."""
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+        # Remove security proxy so that the publisher can call our fake
+        # method.
+        publisher.distro = removeSecurityProxy(publisher.distro)
+
+        naked_breezy_autotest = publisher.distro['breezy-autotest']
+        naked_breezy_autotest.status = SeriesStatus.FUTURE
+        naked_breezy_autotest.publish = FakeMethod(result=set())
+
+        publisher.A_publish(False)
+
+        self.assertEqual(0, naked_breezy_autotest.publish.call_count)
+
+    def testPublishingConsidersObsoletePPASeries(self):
+        """Publisher does not skip OBSOLETE series in PPA archives."""
+        ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team')
+        test_archive = getUtility(IArchiveSet).new(
+            distribution=self.ubuntutest, owner=ubuntu_team,
+            purpose=ArchivePurpose.PPA)
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool, test_archive)
+        # Remove security proxy so that the publisher can call our fake
+        # method.
+        publisher.distro = removeSecurityProxy(publisher.distro)
+
+        naked_breezy_autotest = publisher.distro['breezy-autotest']
+        naked_breezy_autotest.status = SeriesStatus.OBSOLETE
+        naked_breezy_autotest.publish = FakeMethod(result=set())
+
+        publisher.A_publish(False)
+
+        self.assertEqual(1, naked_breezy_autotest.publish.call_count)
+
     def testPublisherBuilderFunctions(self):
         """Publisher can be initialized via provided helper function.