Merge lp:~julian-edwards/launchpad/publish-copy-archives-bug-520520-getPubConfig-proc-accept into lp:launchpad

[Julian Edwards]

[Julian Edwards (julian-edwards)]

= Summary =
Make process-accepted.py cope with rebuild archives

== Proposed fix ==
As part of publishing rebuild archives, we need to make process-accepted work
for them because when builds get uploaded, they are not auto-accepted.

== Pre-implementation notes ==
This is a simple fix, but the existing tests are a total shambles. Therefore
I decided to convert the script into a LaunchpadScript which makes it easier
to unit test. This change forms 95% of the diff.

== Implementation details ==
There's a new test file which does the Right Thing in regards to unit testing.
(i.e. it doesn't use Popen like some of the existing tests)

The test calls the script object and passes in the new option --copy-archives
and makes sure that it only accepts those, not anything in the main archive.

I also had to change SoyuzTestPublisher so that it creates packages in a pre-
accepted state.

There's also some bits of lint fixed.

== Tests ==
bin/test -cvv test_processaccepted

== Demo and Q/A ==
Nothing for this branch, the final feature will be QA'ed.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/tests/test_publishing.py
  lib/lp/soyuz/scripts/processaccepted.py
  lib/lp/soyuz/tests/test_processaccepted.py
  scripts/process-accepted.py

[Graham Binns (gmb)]

Hi Julian,

Couple of minor nitpicks, but the branch looks good.

> === modified file 'lib/lp/soyuz/tests/test_publishing.py'
> --- lib/lp/soyuz/tests/test_publishing.py 2009-10-09 11:13:30 +0000
> +++ lib/lp/soyuz/tests/test_publishing.py 2010-02-23 11:20:39 +0000
> @@ -35,6 +35,7 @@
> from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
> from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
> from lp.soyuz.interfaces.build import BuildStatus
> +from lp.soyuz.interfaces.queue import PackageUploadStatus
> from lp.soyuz.interfaces.publishing import (
> PackagePublishingPriority, PackagePublishingStatus)
> from canonical.launchpad.scripts import FakeLogger
> @@ -130,12 +131,20 @@
> def addPackageUpload(self, archive, distroseries,
> pocket=PackagePublishingPocket.RELEASE,
> changes_file_name="foo_666_source.changes",
> - changes_file_content="fake changes file content"):
> + changes_file_content="fake changes file content",
> + upload_status=PackageUploadStatus.DONE):
> signing_key = self.person.gpgkeys[0]
> package_upload = distroseries.createQueueEntry(
> pocket, changes_file_name, changes_file_content, archive,
> signing_key)
> - package_upload.setDone()
> +
> + status_to_method = {
> + PackageUploadStatus.DONE: 'setDone',
> + PackageUploadStatus.ACCEPTED: 'setAccepted'
> + }

Minor nit here: our coding guidelines say that the closing brace should
be indented to the same level as the dict contents. Also, the last item
in the dict should have a comma at the end of it.

> + method = getattr(package_upload, status_to_method[upload_status])
> + method()
> +
> return package_upload
>
> def getPubSource(self, sourcename=None, version='666', component='main',
> @@ -152,8 +161,13 @@
> dsc_binaries='foo-bin', build_conflicts=None,
> build_conflicts_indep=None,
> dsc_maintainer_rfc822='Foo Bar <email address hidden>',
> - maintainer=None, creator=None, date_uploaded=UTC_NOW):
> - """Return a mock source publishing record."""
> + maintainer=None, creator=None, date_uploaded=UTC_NOW,
> + spr_only=False):
> + """Return a mock source publishing record.
> +
> + if spr_only is specified, the source is not published and the
> + sourcepackagerelease object is returned instead.
> + """
> if sourcename is None:
> sourcename = self.default_package_name
> spn = getUtility(ISourcePackageNameSet).getOrCreateByName(sourcename)
> @@ -194,12 +208,20 @@
> archive=archive, dateuploaded=date_uploaded)
>
> changes_file_name = "%s_%s_source.changes" % (sourcename, version)
> + if spr_only:
> + upload_status = PackageUploadStatus.ACCEPTED
> + else:
> + upload_status = PackageUploadStatus.DONE
> package_upload = self.addPackageUpload(
> ...

review: Approve (code)

=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py	2009-08-28 06:39:38 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py	2010-02-23 11:57:23 +0000
@@ -8,6 +8,7 @@
     'close_bugs',
     'close_bugs_for_queue_item',
     'close_bugs_for_sourcepublication',
+    'ProcessAccepted',
     ]
 
 from zope.component import getUtility
@@ -17,9 +18,12 @@
 from lp.bugs.interfaces.bugtask import BugTaskStatus
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.webapp.interfaces import NotFoundError
-from lp.soyuz.interfaces.archive import ArchivePurpose
+from lp.soyuz.interfaces.archive import ArchivePurpose, IArchiveSet
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.soyuz.interfaces.queue import IPackageUploadSet
+from lp.soyuz.interfaces.queue import IPackageUploadSet, PackageUploadStatus
+
+from lp.services.scripts.base import LaunchpadScript
 
 
 def get_bugs_from_changes_file(changes_file):
@@ -160,3 +164,109 @@
                 owner=janitor,
                 subject=bug.followup_subject(),
                 content=content)
+
+
+class ProcessAccepted(LaunchpadScript):
+    """Queue/Accepted processor.
+
+    Given a distribution to run on, obtains all the queue items for the
+    distribution and then gets on and deals with any accepted items, preparing
+    them for publishing as appropriate.
+    """
+
+    def add_my_options(self):
+        """Command line options for this script."""
+        self.parser.add_option(
+            "-n", "--dry-run", action="store_true",
+            dest="dryrun", metavar="DRY_RUN", default=False,
+            help="Whether to treat this as a dry-run or not.")
+
+        self.parser.add_option(
+            "--ppa", action="store_true",
+            dest="ppa", metavar="PPA", default=False,
+            help="Run only over PPA archives.")
+
+        self.parser.add_option(
+            "--copy-archives", action="store_true",
+            dest="copy_archives", metavar="COPY_ARCHIVES",
+            default=False, help="Run only over COPY archives.")
+
+    @property
+    def lockfilename(self):
+        """Override LaunchpadScript's lock file name."""
+        return "/var/lock/launchpad-upload-queue.lock"
+
+    def main(self):
+        """Entry point for a LaunchpadScript."""
+        if len(self.args) != 1:
+            self.logger.error(
+                "Need to be given exactly one non-option argument. "
+                "Namely the distribution to process.")
+            return 1
+
+        if self.options.ppa and self.options.copy_archives:
+            self.logger.error(
+                "Specify only one of copy archives or ppa archives.")
+            return 1
+
+        distro_name = self.args[0]
+
+        processed_queue_ids = []
+        try:
+            self.logger.debug("Finding distribution %s." % distro_name)
+            distribution = getUtility(IDistributionSet).getByName(distro_name)
+
+            # target_archives is a tuple of (archive, description).
+            if self.options.ppa:
+                target_archives = [
+                    (archive, archive.archive_url)
+                    for archive in distribution.getPendingAcceptancePPAs()]
+            elif self.options.copy_archives:
+                copy_archives = getUtility(
+                    IArchiveSet).getArchivesForDistribution(
+                        distribution, purposes=[ArchivePurpose.COPY])
+                target_archives = [
+                    (archive, archive.displayname)
+                    for archive in copy_archives]
+            else:
+                target_archives = [
+                    (archive, archive.purpose.title)
+                    for archive in distribution.all_distro_archives]
+
+            for archive, description in target_archives:
+                for distroseries in distribution.series:
+
+                    self.logger.debug("Processing queue for %s %s" % (
+                        distroseries.name, description))
+
+                    queue_items = distroseries.getQueueItems(
+                        PackageUploadStatus.ACCEPTED, archive=archive)
+                    for queue_item in queue_items:
+                        try:
+                            queue_item.realiseUpload(self.logger)
+                        except:
+                            self.logger.error(
+                                "Failure processing queue_item %d" % (
+                                    queue_item.id), exc_info=True)
+                            raise
+                        else:
+                            processed_queue_ids.append(queue_item.id)
+
+            if not self.options.dryrun:
+                self.txn.commit()
+            else:
+                self.logger.debug("Dry Run mode.")
+
+            if not self.options.ppa and not self.options.copy_archives:
+                self.logger.debug("Closing bugs.")
+                close_bugs(processed_queue_ids)
+
+            if not self.options.dryrun:
+                self.txn.commit()
+
+        finally:
+            self.logger.debug("Rolling back any remaining transactions.")
+            self.txn.abort()
+
+        return 0
+

=== added file 'lib/lp/soyuz/tests/test_processaccepted.py'
--- lib/lp/soyuz/tests/test_processaccepted.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_processaccepted.py	2010-02-23 11:57:23 +0000
@@ -0,0 +1,85 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test process-accepted.py"""
+
+from zope.component import getUtility
+
+from canonical.config import config
+from canonical.launchpad.scripts import QuietFakeLogger
+from canonical.testing import LaunchpadZopelessLayer
+
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.soyuz.interfaces.archive import ArchivePurpose
+from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+from lp.soyuz.scripts.processaccepted import ProcessAccepted
+from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+from lp.testing import TestCaseWithFactory
+
+
+class TestProcessAccepted(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+    dbuser = config.uploadqueue.dbuser
+
+    def setUp(self):
+        """Create the Soyuz test publisher."""
+        TestCaseWithFactory.setUp(self)
+        self.stp = SoyuzTestPublisher()
+        self.stp.prepareBreezyAutotest()
+        self.test_package_name = "accept-test"
+
+    def getScript(self, test_args=None):
+        """Return a ProcessAccepted instance."""
+        if test_args is None:
+            test_args = []
+        test_args.append(self.stp.ubuntutest.name)
+        script = ProcessAccepted("process accepted", test_args=test_args)
+        script.logger = QuietFakeLogger()
+        script.txn = self.layer.txn
+        return script
+
+    def createWaitingAcceptancePackage(self, archive=None):
+        """Create some pending publications."""
+        if archive is None:
+            archive = getUtility(
+                IDistributionSet).getByName('ubuntu').main_archive
+        return self.stp.getPubSource(
+            archive=archive, sourcename=self.test_package_name, spr_only=True)
+
+    def testAcceptCopyArchives(self):
+        """Test that publications in a copy archive are accepted properly."""
+        # Upload some pending packages in a copy archive.
+        copy_archive = self.factory.makeArchive(
+            distribution=self.stp.ubuntutest, purpose=ArchivePurpose.COPY)
+        copy_source = self.createWaitingAcceptancePackage(
+            archive=copy_archive)
+        # Also upload some stuff in the main archive.
+        main_source = self.createWaitingAcceptancePackage()
+
+        # Before accepting, the package should not be published at all.
+        published_copy = copy_archive.getPublishedSources(
+            name=self.test_package_name)
+        # Using .count() until Storm fixes __nonzero__ on SQLObj result
+        # sets, then we can use bool() which is far more efficient than
+        # counting.
+        self.assertEqual(published_copy.count(), 0)
+
+        # Accept the packages.
+        script = self.getScript(['--copy-archives'])
+        self.layer.txn.commit()
+        self.layer.switchDbUser(self.dbuser)
+        script.main()
+
+        # Packages in main archive should not be accepted and published.
+        published_main = self.stp.ubuntutest.main_archive.getPublishedSources(
+            name=self.test_package_name)
+        self.assertEqual(published_main.count(), 0)
+
+        # Check the copy archive source was accepted.
+        [published_copy] = copy_archive.getPublishedSources(
+            name=self.test_package_name)
+        self.assertEqual(
+            published_copy.status, PackagePublishingStatus.PENDING)
+        self.assertEqual(copy_source, published_copy.sourcepackagerelease)
+

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2009-10-09 11:13:30 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2010-02-23 11:57:23 +0000
@@ -35,6 +35,7 @@
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
 from lp.soyuz.interfaces.build import BuildStatus
+from lp.soyuz.interfaces.queue import PackageUploadStatus
 from lp.soyuz.interfaces.publishing import (
     PackagePublishingPriority, PackagePublishingStatus)
 from canonical.launchpad.scripts import FakeLogger
@@ -130,12 +131,20 @@
     def addPackageUpload(self, archive, distroseries,
                          pocket=PackagePublishingPocket.RELEASE,
                          changes_file_name="foo_666_source.changes",
-                         changes_file_content="fake changes file content"):
+                         changes_file_content="fake changes file content",
+                         upload_status=PackageUploadStatus.DONE):
         signing_key =  self.person.gpgkeys[0]
         package_upload = distroseries.createQueueEntry(
             pocket, changes_file_name, changes_file_content, archive,
             signing_key)
-        package_upload.setDone()
+
+        status_to_method = {
+            PackageUploadStatus.DONE: 'setDone',
+            PackageUploadStatus.ACCEPTED: 'setAccepted',
+            }
+        method = getattr(package_upload, status_to_method[upload_status])
+        method()
+
         return package_upload
 
     def getPubSource(self, sourcename=None, version='666', component='main',
@@ -152,8 +161,13 @@
                      dsc_binaries='foo-bin', build_conflicts=None,
                      build_conflicts_indep=None,
                      dsc_maintainer_rfc822='Foo Bar <[e-mail]>',
-                     maintainer=None, creator=None, date_uploaded=UTC_NOW):
-        """Return a mock source publishing record."""
+                     maintainer=None, creator=None, date_uploaded=UTC_NOW,
+                     spr_only=False):
+        """Return a mock source publishing record.
+
+        if spr_only is specified, the source is not published and the
+        sourcepackagerelease object is returned instead.
+        """
         if sourcename is None:
             sourcename = self.default_package_name
         spn = getUtility(ISourcePackageNameSet).getOrCreateByName(sourcename)
@@ -194,12 +208,20 @@
             archive=archive, dateuploaded=date_uploaded)
 
         changes_file_name = "%s_%s_source.changes" % (sourcename, version)
+        if spr_only:
+            upload_status = PackageUploadStatus.ACCEPTED
+        else:
+            upload_status = PackageUploadStatus.DONE
         package_upload = self.addPackageUpload(
             archive, distroseries, pocket,
             changes_file_name=changes_file_name,
-            changes_file_content=changes_file_content)
+            changes_file_content=changes_file_content,
+            upload_status=upload_status)
         package_upload.addSource(spr)
 
+        if spr_only:
+            return spr
+
         if filename is None:
             filename = "%s_%s.dsc" % (sourcename, version)
         alias = self.addMockFile(
@@ -389,7 +411,8 @@
         """File with given name fragment in directory tree starting at top."""
         for root, dirs, files in os.walk(top, topdown=False):
             for name in files:
-                if name.endswith('.changes') and name.find(name_fragment) > -1:
+                if (name.endswith('.changes') and 
+                    name.find(name_fragment) > -1):
                     return os.path.join(root, name)
         return None
 

=== modified file 'scripts/process-accepted.py'
--- scripts/process-accepted.py	2009-11-17 02:33:27 +0000
+++ scripts/process-accepted.py	2010-02-23 11:57:23 +0000
@@ -3,6 +3,9 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+# Stop pylint complaining about the _pythonpath relative import.
+# pylint: disable-msg=W0403
+
 """Queue/Accepted processor
 
 Given a distribution to run on, obtains all the queue items for the
@@ -12,110 +15,13 @@
 
 import _pythonpath
 
-import sys
-from optparse import OptionParser
-
-from zope.component import getUtility
-
 from canonical.config import config
 from canonical.database.sqlbase import ISOLATION_LEVEL_READ_COMMITTED
-from lp.soyuz.interfaces.queue import PackageUploadStatus
-from canonical.launchpad.scripts import (
-    execute_zcml_for_scripts, logger, logger_options)
-from lp.soyuz.scripts.processaccepted import close_bugs
-from canonical.lp import initZopeless
-
-from contrib.glock import GlobalLock
-
-def main():
-    # Prevent circular imports.
-    from lp.registry.interfaces.distribution import IDistributionSet
-
-    # Parse command-line arguments
-    parser = OptionParser()
-    logger_options(parser)
-
-    parser.add_option("-n", "--dry-run", action="store_true",
-                      dest="dryrun", metavar="DRY_RUN", default=False,
-                      help="Whether to treat this as a dry-run or not.")
-
-    parser.add_option("--ppa", action="store_true",
-                      dest="ppa", metavar="PPA", default=False,
-                      help="Run only over PPA archives.")
-
-    (options, args) = parser.parse_args()
-
-    log = logger(options, "process-accepted")
-
-    if len(args) != 1:
-        log.error("Need to be given exactly one non-option argument. "
-                  "Namely the distribution to process.")
-        return 1
-
-    distro_name = args[0]
-
-    log.debug("Acquiring lock")
-    lock = GlobalLock('/var/lock/launchpad-upload-queue.lock')
-    lock.acquire(blocking=True)
-
-    log.debug("Initialising connection.")
-
-    execute_zcml_for_scripts()
-    ztm = initZopeless(dbuser=config.uploadqueue.dbuser,
-                       isolation=ISOLATION_LEVEL_READ_COMMITTED)
-
-    processed_queue_ids = []
-    try:
-        log.debug("Finding distribution %s." % distro_name)
-        distribution = getUtility(IDistributionSet).getByName(distro_name)
-
-        # target_archives is a tuple of (archive, description).
-        if options.ppa:
-            target_archives = [
-                (archive, archive.archive_url)
-                for archive in distribution.getPendingAcceptancePPAs()]
-        else:
-            target_archives = [
-                (archive, archive.purpose.title)
-                for archive in distribution.all_distro_archives]
-
-        for archive, description in target_archives:
-            for distroseries in distribution.series:
-
-                log.debug("Processing queue for %s %s" % (
-                        distroseries.name, description))
-
-                queue_items = distroseries.getQueueItems(
-                    PackageUploadStatus.ACCEPTED, archive=archive)
-                for queue_item in queue_items:
-                    try:
-                        queue_item.realiseUpload(log)
-                    except:
-                        log.error("Failure processing queue_item %d"
-                                  % (queue_item.id), exc_info=True)
-                        raise
-                    else:
-                        processed_queue_ids.append(queue_item.id)
-
-        if not options.dryrun:
-            ztm.commit()
-        else:
-            log.debug("Dry Run mode.")
-
-        log.debug("Closing bugs.")
-        close_bugs(processed_queue_ids)
-
-        if not options.dryrun:
-            ztm.commit()
-
-    finally:
-        log.debug("Rolling back any remaining transactions.")
-        ztm.abort()
-        log.debug("Releasing lock")
-        lock.release()
-
-    return 0
+from lp.soyuz.scripts.processaccepted import ProcessAccepted
+
 
 if __name__ == '__main__':
-    sys.exit(main())
+    script = ProcessAccepted(
+        "process-accepted", dbuser=config.uploadqueue.dbuser)
+    script.lock_and_run(isolation=ISOLATION_LEVEL_READ_COMMITTED)