launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-query-distro-pending-suites into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-query-distro-pending-suites/+merge/101174

== Summary ==

The "pending_suites" action in the lp-query-distro.py script (and the LpQueryDistro script class) made sense when the ftpmaster publisher script was a shell script; pending_suites is used in exactly one place, to discover whether any security suites are pending publication (see r6303.1.2).  However, since it was rewritten in Python, it's just pointless indirection, not to mention inefficient because it means setting up and tearing down a LaunchpadScript instance.  PublishFTPMaster can just talk to the database directly.  We can refactor this.

== Tests ==

bin/test -vvct test_lpquerydistro -t test_publish_ftpmaster

== lint ==

None.
-- 
https://code.launchpad.net/~cjwatson/launchpad/remove-query-distro-pending-suites/+merge/101174
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-query-distro-pending-suites into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2012-02-28 04:24:19 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2012-04-07 12:30:25 +0000
@@ -26,11 +26,13 @@
     LaunchpadScriptFailure,
     )
 from lp.services.utils import file_exists
-from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.enums import (
+    ArchivePurpose,
+    PackagePublishingStatus,
+    )
 from lp.soyuz.scripts.custom_uploads_copier import CustomUploadsCopier
 from lp.soyuz.scripts.processaccepted import ProcessAccepted
 from lp.soyuz.scripts.publishdistro import PublishDistro
-from lp.soyuz.scripts.querydistro import LpQueryDistro
 
 
 def get_publishable_archives(distribution):
@@ -121,13 +123,6 @@
     return {"PATH": '"$PATH":%s' % shell_quote(scripts_dir)}
 
 
-class StoreArgument:
-    """Helper class: receive argument and store it."""
-
-    def __call__(self, argument):
-        self.argument = argument
-
-
 def find_run_parts_dir(distro, parts):
     """Find the requested run-parts directory, if it exists."""
     run_parts_location = config.archivepublisher.run_parts_location
@@ -329,12 +324,20 @@
     def getDirtySuites(self, distribution):
         """Return list of suites that have packages pending publication."""
         self.logger.debug("Querying which suites are pending publication...")
-        query_distro = LpQueryDistro(
-            test_args=['-d', distribution.name, "pending_suites"],
-            logger=self.logger)
-        receiver = StoreArgument()
-        query_distro.runAction(presenter=receiver)
-        return receiver.argument.split()
+
+        pending_suites = set()
+        pending_sources = distribution.main_archive.getPublishedSources(
+            status=PackagePublishingStatus.PENDING)
+        for pub in pending_sources:
+            pending_suites.add((pub.distroseries, pub.pocket))
+
+        pending_binaries = distribution.main_archive.getAllPublishedBinaries(
+            status=PackagePublishingStatus.PENDING)
+        for pub in pending_binaries:
+            pending_suites.add((pub.distroarchseries.distroseries, pub.pocket))
+
+        return [distroseries.name + pocketsuffix[pocket]
+                for distroseries, pocket in pending_suites]
 
     def getDirtySecuritySuites(self, distribution):
         """List security suites with pending publications."""

=== modified file 'lib/lp/soyuz/scripts/querydistro.py'
--- lib/lp/soyuz/scripts/querydistro.py	2012-01-19 03:06:04 +0000
+++ lib/lp/soyuz/scripts/querydistro.py	2012-04-07 12:30:25 +0000
@@ -17,7 +17,6 @@
     build_package_location,
     PackageLocationError,
     )
-from lp.soyuz.enums import PackagePublishingStatus
 
 
 class LpQueryDistro(LaunchpadScript):
@@ -29,7 +28,7 @@
         Also initialize the list 'allowed_arguments'.
         """
         self.allowed_actions = [
-            'current', 'development', 'supported', 'pending_suites', 'archs',
+            'current', 'development', 'supported', 'archs',
             'official_archs', 'nominated_arch_indep', 'pocket_suffixes']
         self.usage = '%%prog <%s>' % ' | '.join(self.allowed_actions)
         LaunchpadScript.__init__(self, *args, **kwargs)
@@ -197,28 +196,6 @@
         return " ".join(supported_series)
 
     @property
-    def pending_suites(self):
-        """Return the suite names containing PENDING publication.
-
-        It check for sources and/or binary publications.
-        """
-        self.checkNoSuiteDefined()
-        pending_suites = set()
-        pending_sources = self.location.archive.getPublishedSources(
-            status=PackagePublishingStatus.PENDING)
-        for pub in pending_sources:
-            pending_suites.add((pub.distroseries, pub.pocket))
-
-        pending_binaries = self.location.archive.getAllPublishedBinaries(
-            status=PackagePublishingStatus.PENDING)
-        for pub in pending_binaries:
-            pending_suites.add(
-                (pub.distroarchseries.distroseries, pub.pocket))
-
-        return " ".join([distroseries.name + pocketsuffix[pocket]
-                         for distroseries, pocket in pending_suites])
-
-    @property
     def archs(self):
         """Return a space-separated list of architecture tags.
 

=== modified file 'lib/lp/soyuz/scripts/tests/test_lpquerydistro.py'
--- lib/lp/soyuz/scripts/tests/test_lpquerydistro.py	2012-01-19 03:09:38 +0000
+++ lib/lp/soyuz/scripts/tests/test_lpquerydistro.py	2012-04-07 12:30:25 +0000
@@ -224,7 +224,6 @@
         self.assertEqual(helper.current, 'warty')
         self.assertEqual(helper.development, 'hoary')
         self.assertEqual(helper.supported, 'hoary warty')
-        self.assertEqual(helper.pending_suites, 'warty')
         self.assertEqual(helper.archs, 'hppa i386')
         self.assertEqual(helper.official_archs, 'i386')
         self.assertEqual(helper.nominated_arch_indep, 'i386')
@@ -253,7 +252,6 @@
         self.assertAttributeRaisesScriptFailure(helper, 'current')
         self.assertAttributeRaisesScriptFailure(helper, 'development')
         self.assertAttributeRaisesScriptFailure(helper, 'supported')
-        self.assertAttributeRaisesScriptFailure(helper, 'pending_suites')
         self.assertEqual(helper.archs, 'hppa i386')
         self.assertEqual(helper.official_archs, 'i386')
         self.assertEqual(helper.nominated_arch_indep, 'i386')