Merge lp:~cjwatson/launchpad/remove-query-distro-pending-suites into lp:launchpad

[Colin Watson]

Re-land r15073 (LpQueryDistro clean-up), this time with correct load_related handling.

== Summary ==

Fix regression caused by r15073, as outlined in bug 983165.

The bulk-loading suggested by Jeroen in https://code.launchpad.net/~cjwatson/launchpad/remove-query-distro-pending-suites/+merge/101174 broke, partly because I didn't notice that no tests would exercise this and partly because I cavalierly thought no QA was needed. I believe that this fixes it properly.

== Tests ==

bin/test -vvct getDirtySuites

== Demo and Q/A ==

Let's actually do some this time. I think it should be sufficient to upload a modified hello source package to dogfood/ubuntu/oneiric, wait for it to build on i386, and attempt to publish both source and binary.

[Jeroen T. Vermeulen (jtv)]

Looks fine!

One very very small thing:

lib/lp/soyuz/interfaces/publishing.py

 724 distroarchseriesID = Int(
 725 title=_('The DB id for the distroarchseries.'),
 726 required=False, readonly=False)

I highly recommend double quotes for free-form text. It avoids widespread panic and looting as apostrophes destroy civilization.

Jeroen

review: Approve

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2012-04-16 10:04:33 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2012-04-16 15:51:23 +0000
@@ -21,16 +21,20 @@
 from lp.registry.interfaces.pocket import pocketsuffix
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.config import config
+from lp.services.database.bulk import load_related
 from lp.services.scripts.base import (
     LaunchpadCronScript,
     LaunchpadScriptFailure,
     )
 from lp.services.utils import file_exists
-from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.enums import (
+    ArchivePurpose,
+    PackagePublishingStatus,
+    )
+from lp.soyuz.model.distroarchseries import DistroArchSeries
 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 +125,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
@@ -327,14 +324,19 @@
         script.main()
 
     def getDirtySuites(self, distribution):
-        """Return list of suites that have packages pending publication."""
+        """Return set 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()
+
+        archive = distribution.main_archive
+        pending = PackagePublishingStatus.PENDING
+        pending_sources = list(archive.getPublishedSources(status=pending))
+        pending_binaries = list(archive.getAllPublishedBinaries(
+            status=pending))
+        load_related(
+            DistroArchSeries, pending_binaries, ['distroarchseriesID'])
+        return set(
+            pub.distroseries.name + pocketsuffix[pub.pocket]
+            for pub in pending_sources + pending_binaries)
 
     def getDirtySecuritySuites(self, distribution):
         """List security suites with pending publications."""

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2012-04-16 10:04:33 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2012-04-16 15:51:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test publish-ftpmaster cron script."""
@@ -63,9 +63,9 @@
     return file_exists(os.path.join(*path_components))
 
 
-def name_spph_suite(spph):
-    """Return name of `spph`'s suite."""
-    return spph.distroseries.name + pocketsuffix[spph.pocket]
+def name_pph_suite(pph):
+    """Return name of `pph`'s suite."""
+    return pph.distroseries.name + pocketsuffix[pph.pocket]
 
 
 def get_pub_config(distro):
@@ -400,8 +400,8 @@
         distro = spph.distroseries.distribution
         script = self.makeScript(spph.distroseries.distribution)
         script.setUp()
-        self.assertEqual(
-            [name_spph_suite(spph)], script.getDirtySuites(distro))
+        self.assertContentEqual(
+            [name_pph_suite(spph)], script.getDirtySuites(distro))
 
     def test_getDirtySuites_returns_suites_with_pending_publications(self):
         distro = self.makeDistroWithPublishDirectory()
@@ -414,7 +414,7 @@
         script = self.makeScript(distro)
         script.setUp()
         self.assertContentEqual(
-            [name_spph_suite(spph) for spph in spphs],
+            [name_pph_suite(spph) for spph in spphs],
             script.getDirtySuites(distro))
 
     def test_getDirtySuites_ignores_suites_without_pending_publications(self):
@@ -423,7 +423,15 @@
         distro = spph.distroseries.distribution
         script = self.makeScript(spph.distroseries.distribution)
         script.setUp()
-        self.assertEqual([], script.getDirtySuites(distro))
+        self.assertContentEqual([], script.getDirtySuites(distro))
+
+    def test_getDirtySuites_returns_suites_with_pending_binaries(self):
+        bpph = self.factory.makeBinaryPackagePublishingHistory()
+        distro = bpph.distroseries.distribution
+        script = self.makeScript(bpph.distroseries.distribution)
+        script.setUp()
+        self.assertContentEqual(
+            [name_pph_suite(bpph)], script.getDirtySuites(distro))
 
     def test_getDirtySecuritySuites_returns_security_suites(self):
         distro = self.makeDistroWithPublishDirectory()
@@ -437,7 +445,7 @@
         script = self.makeScript(distro)
         script.setUp()
         self.assertContentEqual(
-            [name_spph_suite(spph) for spph in spphs],
+            [name_pph_suite(spph) for spph in spphs],
             script.getDirtySecuritySuites(distro))
 
     def test_getDirtySecuritySuites_ignores_non_security_suites(self):

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2012-01-09 13:40:48 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2012-04-16 15:51:23 +0000
@@ -721,6 +721,9 @@
         required=False, readonly=False)
     binarypackagerelease = Attribute(
         "The binary package release being published")
+    distroarchseriesID = Int(
+        title=_("The DB id for the distroarchseries."),
+        required=False, readonly=False)
     distroarchseries = exported(
         Reference(
             # Really IDistroArchSeries (fixed in

=== modified file 'lib/lp/soyuz/scripts/querydistro.py'
--- lib/lp/soyuz/scripts/querydistro.py	2012-04-16 10:04:33 +0000
+++ lib/lp/soyuz/scripts/querydistro.py	2012-04-16 15:51:23 +0000
@@ -7,7 +7,6 @@
 
 __all__ = ['LpQueryDistro']
 
-from lp.registry.interfaces.pocket import pocketsuffix
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.scripts.base import (
     LaunchpadScript,
@@ -17,7 +16,6 @@
     build_package_location,
     PackageLocationError,
     )
-from lp.soyuz.enums import PackagePublishingStatus
 
 
 class LpQueryDistro(LaunchpadScript):
@@ -28,9 +26,7 @@
 
         Also initialize the list 'allowed_arguments'.
         """
-        self.allowed_actions = [
-            'current', 'development', 'supported', 'pending_suites', 'archs',
-            'official_archs', 'nominated_arch_indep', 'pocket_suffixes']
+        self.allowed_actions = ['development', 'supported', 'archs']
         self.usage = '%%prog <%s>' % ' | '.join(self.allowed_actions)
         LaunchpadScript.__init__(self, *args, **kwargs)
 
@@ -122,23 +118,6 @@
                 "Action does not accept defined suite.")
 
     @property
-    def current(self):
-        """Return the name of the CURRENT distroseries.
-
-        It is restricted for the context distribution.
-
-        It may raise LaunchpadScriptFailure if a suite was passed on the
-        command-line or if not CURRENT distroseries was found.
-        """
-        self.checkNoSuiteDefined()
-        series = self.location.distribution.getSeriesByStatus(
-            SeriesStatus.CURRENT)
-        if not series:
-            raise LaunchpadScriptFailure("No CURRENT series.")
-
-        return series[0].name
-
-    @property
     def development(self):
         """Return the name of the DEVELOPMENT distroseries.
 
@@ -197,28 +176,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.
 
@@ -226,35 +183,3 @@
         """
         architectures = self.location.distroseries.architectures
         return " ".join(arch.architecturetag for arch in architectures)
-
-    @property
-    def official_archs(self):
-        """Return a space-separated list of official architecture tags.
-
-        It is restricted to the context distribution and suite.
-        """
-        architectures = self.location.distroseries.architectures
-        return " ".join(arch.architecturetag
-                        for arch in architectures
-                        if arch.official)
-
-    @property
-    def nominated_arch_indep(self):
-        """Return the nominated arch indep architecture tag.
-
-        It is restricted to the context distribution and suite.
-        """
-        series = self.location.distroseries
-        return series.nominatedarchindep.architecturetag
-
-    @property
-    def pocket_suffixes(self):
-        """Return a space-separated list of non-empty pocket suffixes.
-
-        The RELEASE pocket (whose suffix is the empty string) is omitted.
-
-        The returned space-separated string is ordered alphabetically.
-        """
-        sorted_non_empty_suffixes = sorted(
-            suffix for suffix in pocketsuffix.values() if suffix != '')
-        return " ".join(sorted_non_empty_suffixes)

=== modified file 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py'
--- lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py	2012-04-16 10:04:33 +0000
+++ lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py	2012-04-16 15:51:23 +0000
@@ -8,8 +8,8 @@
 
 
 def error_and_exit():
-    sys.stderr.write("ERROR: I'm a mock, I only support 'development' "
-                        "and 'supported' as argument\n")
+    sys.stderr.write("ERROR: I'm a mock, I only support 'supported' as "
+                     "argument\n")
     sys.exit(1)
 
 
@@ -18,12 +18,8 @@
     # test for it and error if it looks wrong
     if len(args) == 2:
         distro = args[1]
-        if distro == "development":
-                return "natty"
-        elif distro == "supported":
-                return "hardy jaunty karmic lucid maverick"
-    elif len(args) == 4 and args[1] == '-s' and args[3] == 'archs':
-        return "i386 amd64 powerpc armel"
+        if distro == "supported":
+            return "hardy jaunty karmic lucid maverick"
     error_and_exit()
 
 

=== modified file 'lib/lp/soyuz/scripts/tests/test_lpquerydistro.py'
--- lib/lp/soyuz/scripts/tests/test_lpquerydistro.py	2012-04-16 10:04:33 +0000
+++ lib/lp/soyuz/scripts/tests/test_lpquerydistro.py	2012-04-16 15:51:23 +0000
@@ -50,15 +50,15 @@
         Check that:
          * return code is ZERO,
          * standard error is empty
-         * standard output contains only the 'current distroseries' name
+         * standard output contains only the 'development distroseries' name
         """
         returncode, out, err = self.runLpQueryDistro(
-            extra_args=['current'])
+            extra_args=['development'])
 
         self.assertEqual(
             0, returncode, "\nScript Failed:%s\nStdout:\n%s\nStderr\n%s\n"
             % (returncode, out, err))
-        self.assertEqual(out.strip(), 'warty')
+        self.assertEqual(out.strip(), 'hoary')
         self.assertEqual(err.strip(), '')
 
     def testMissingAction(self):
@@ -108,7 +108,7 @@
          * standard error contains additional information about the failure.
         """
         returncode, out, err = self.runLpQueryDistro(
-            extra_args=['-s', 'hoary', 'current'])
+            extra_args=['-s', 'warty', 'development'])
 
         self.assertEqual(
             1, returncode,
@@ -140,15 +140,6 @@
         """
         self.test_output = '%s' % args
 
-    def testSuccessfullyAction(self):
-        """Check if the 'current' action is executed sucessfully."""
-        helper = self.getLpQueryDistro(test_args=['current'])
-        helper.runAction(presenter=self.presenter)
-        warty = self.ubuntu['warty']
-        self.assertEqual(warty.status.name, 'CURRENT')
-        self.assertEqual(helper.location.distribution.name, u'ubuntu')
-        self.assertEqual(self.test_output, u'warty')
-
     def testDevelopmentAndFrozenDistroSeries(self):
         """The 'development' action should cope with FROZEN distroseries."""
         helper = self.getLpQueryDistro(test_args=['development'])
@@ -185,7 +176,8 @@
         Some actions do not allow passing 'suite'.
         See testActionswithDefinedSuite for further information.
         """
-        helper = self.getLpQueryDistro(test_args=['-s', 'hoary', 'current'])
+        helper = self.getLpQueryDistro(
+            test_args=['-s', 'warty', 'development'])
         self.assertRaises(LaunchpadScriptFailure,
                           helper.runAction, self.presenter)
 
@@ -213,23 +205,16 @@
     def testActionsWithUndefinedSuite(self):
         """Check the actions supposed to work with undefined suite.
 
-        Only 'current', 'development' and 'supported' work with undefined
-        suite.
-        The other actions ('archs', 'official_arch', 'nominated_arch_indep')
-        will assume the CURRENT distroseries in context.
+        Only 'development' and 'supported' work with undefined suite.
+        The other actions ('archs') will assume the CURRENT distroseries in
+        context.
         """
         helper = self.getLpQueryDistro(test_args=[])
         helper._buildLocation()
 
-        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')
-        self.assertEqual(helper.pocket_suffixes,
-                         '-backports -proposed -security -updates')
 
     def assertAttributeRaisesScriptFailure(self, obj, attr_name):
         """Asserts if accessing the given attribute name fails.
@@ -242,20 +227,13 @@
     def testActionsWithDefinedSuite(self):
         """Opposite of  testActionsWithUndefinedSuite.
 
-        Only some actions ('archs', 'official_arch', 'nominated_arch_indep',
-        and pocket_suffixes) work with defined suite, the other actions
-        ('current', 'development' and 'supported') will raise
+        Only some actions ('archs') work with defined suite, the other
+        actions ('development' and 'supported') will raise
         LaunchpadScriptError if the suite is defined.
         """
         helper = self.getLpQueryDistro(test_args=['-s', 'warty'])
         helper._buildLocation()
 
-        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')
-        self.assertEqual(helper.pocket_suffixes,
-                         '-backports -proposed -security -updates')

=== modified file 'scripts/ftpmaster-tools/lp-query-distro.py'
--- scripts/ftpmaster-tools/lp-query-distro.py	2012-04-16 10:04:33 +0000
+++ scripts/ftpmaster-tools/lp-query-distro.py	2012-04-16 15:51:23 +0000
@@ -9,15 +9,10 @@
    It should provide an easy way to retrieve current information from
    Launchpad when using plain shell scripts, for example:
 
-   * CURRENT distroseries name: `./ubuntu-helper.py -d ubuntu current`
    * DEVELOPMENT distroseries name:
        `./ubuntu-helper.py -d ubuntu development`
    * Distroseries architectures:
        `./lp-query-distro.py -d ubuntu -s feisty archs`
-   * Distroseries official architectures:
-       `./lp-query-distro.py -d ubuntu -s feisty official_archs`
-   * Distroseries nominated-arch-indep:
-       `./lp-query-distro.py -d ubuntu -s feisty nominated_arch_indep`
 
    Standard Output will carry the successfully executed information and
    exit_code will be ZERO.