launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17321
[Merge] lp:~wgrant/launchpad/process-accepted-cleanup into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/process-accepted-cleanup into lp:launchpad.
Commit message:
Refactor process-accepted's target archive calculation so we can eventually unify them all.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/process-accepted-cleanup/+merge/230201
Refactor process-accepted's target archive calculation so we can eventually unify them all.
--
https://code.launchpad.net/~wgrant/launchpad/process-accepted-cleanup/+merge/230201
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/process-accepted-cleanup into lp:launchpad.
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2014-07-18 07:43:06 +0000
+++ lib/lp/soyuz/model/queue.py 2014-08-09 09:23:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -439,26 +439,6 @@
'Unable to reject queue item due to status.')
self.status = PassthroughStatusValue(PackageUploadStatus.REJECTED)
- def _closeBugs(self, changesfile_path, logger=None):
- """Close bugs for a just-accepted source.
-
- :param changesfile_path: path to the context changesfile.
- :param logger: optional context Logger object (used on DEBUG level);
-
- It does not close bugs for PPA sources.
- """
- from lp.soyuz.scripts.processaccepted import close_bugs_for_queue_item
-
- if self.isPPA():
- debug(logger, "Not closing bugs for PPA source.")
- return
-
- debug(logger, "Closing bugs.")
- changesfile_object = open(changesfile_path, 'r')
- close_bugs_for_queue_item(
- self, changesfile_object=changesfile_object)
- changesfile_object.close()
-
def _validateBuildsForSource(self, sourcepackagerelease, builds):
"""Check if the sourcepackagerelease generates at least one build.
@@ -507,6 +487,8 @@
def acceptFromUploader(self, changesfile_path, logger=None):
"""See `IPackageUpload`."""
+ from lp.soyuz.scripts.processaccepted import close_bugs_for_queue_item
+
debug(logger, "Setting it to ACCEPTED")
self.setAccepted()
@@ -521,7 +503,9 @@
[pub_source] = self.realiseUpload()
builds = pub_source.createMissingBuilds(logger=logger)
self._validateBuildsForSource(pub_source.sourcepackagerelease, builds)
- self._closeBugs(changesfile_path, logger)
+ with open(changesfile_path, 'r') as changesfile_object:
+ close_bugs_for_queue_item(
+ self, changesfile_object=changesfile_object)
self._giveKarma()
def _acceptSyncFromQueue(self):
=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py 2014-08-05 10:34:11 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py 2014-08-09 09:23:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Helper functions for the process-accepted.py script."""
@@ -39,7 +39,6 @@
from lp.soyuz.interfaces.processacceptedbugsjob import (
IProcessAcceptedBugsJobSource,
)
-from lp.soyuz.interfaces.queue import IPackageUploadSet
from lp.soyuz.model.processacceptedbugsjob import (
close_bug_ids_for_sourcepackagerelease,
)
@@ -181,72 +180,6 @@
job_source.create(distroseries, source_release, bug_ids_to_close)
-class TargetPolicy:
- """Policy describing what kinds of archives to operate on."""
-
- def __init__(self, logger):
- self.logger = logger
-
- def getTargetArchives(self, distribution):
- """Get target archives of the right sort for `distribution`."""
- raise NotImplemented("getTargetArchives")
-
- def describeArchive(self, archive):
- """Return textual description for `archive` in this script run."""
- raise NotImplemented("describeArchive")
-
- def postprocessSuccesses(self, queue_ids):
- """Optionally, post-process successfully processed queue items.
-
- :param queue_ids: An iterable of `PackageUpload` ids that were
- successfully processed.
- """
-
-
-class PPATargetPolicy(TargetPolicy):
- """Target policy for PPA archives."""
-
- def getTargetArchives(self, distribution):
- """See `TargetPolicy`."""
- return distribution.getPendingAcceptancePPAs()
-
- def describeArchive(self, archive):
- """See `TargetPolicy`."""
- return archive.archive_url
-
-
-class CopyArchiveTargetPolicy(TargetPolicy):
- """Target policy for copy archives."""
-
- def getTargetArchives(self, distribution):
- """See `TargetPolicy`."""
- return getUtility(IArchiveSet).getArchivesForDistribution(
- distribution, purposes=[ArchivePurpose.COPY])
-
- def describeArchive(self, archive):
- """See `TargetPolicy`."""
- return archive.displayname
-
-
-class DistroTargetPolicy(TargetPolicy):
- """Target policy for distro archives."""
-
- def getTargetArchives(self, distribution):
- """See `TargetPolicy`."""
- return distribution.all_distro_archives
-
- def describeArchive(self, archive):
- """See `TargetPolicy`."""
- return archive.purpose.title
-
- def postprocessSuccesses(self, queue_ids):
- """See `TargetPolicy`."""
- self.logger.debug("Closing bugs.")
- for queue_id in queue_ids:
- queue_item = getUtility(IPackageUploadSet).get(queue_id)
- close_bugs_for_queue_item(queue_item)
-
-
class ProcessAccepted(PublisherScript):
"""Queue/Accepted processor.
@@ -281,15 +214,15 @@
raise OptionValueError(
"Can't combine --derived with a distribution name.")
- def makeTargetPolicy(self):
- """Pick and instantiate a `TargetPolicy` based on given options."""
+ def getTargetArchives(self, distribution):
+ """Find archives to target based on given options."""
if self.options.ppa:
- policy_class = PPATargetPolicy
+ return distribution.getPendingAcceptancePPAs()
elif self.options.copy_archives:
- policy_class = CopyArchiveTargetPolicy
+ return getUtility(IArchiveSet).getArchivesForDistribution(
+ distribution, purposes=[ArchivePurpose.COPY])
else:
- policy_class = DistroTargetPolicy
- return policy_class(self.logger)
+ return distribution.all_distro_archives
def processQueueItem(self, queue_item):
"""Attempt to process `queue_item`.
@@ -315,22 +248,20 @@
"Successfully processed queue item %d", queue_item.id)
return True
- def processForDistro(self, distribution, target_policy):
+ def processForDistro(self, distribution):
"""Process all queue items for a distribution.
Commits between items.
:param distribution: The `Distribution` to process queue items for.
- :param target_policy: The applicable `TargetPolicy`.
:return: A list of all successfully processed items' ids.
"""
processed_queue_ids = []
- for archive in target_policy.getTargetArchives(distribution):
- description = target_policy.describeArchive(archive)
+ for archive in self.getTargetArchives(distribution):
for distroseries in distribution.series:
self.logger.debug("Processing queue for %s %s" % (
- distroseries.name, description))
+ archive.reference, distroseries.name))
queue_items = distroseries.getPackageUploads(
status=PackageUploadStatus.ACCEPTED, archive=archive)
@@ -341,21 +272,18 @@
# on-disk archive, so the partial state must
# make it to the DB.
self.txn.commit()
+ close_bugs_for_queue_item(queue_item)
+ self.txn.commit()
return processed_queue_ids
def main(self):
"""Entry point for a LaunchpadScript."""
self.validateArguments()
- target_policy = self.makeTargetPolicy()
try:
for distro in self.findDistros():
- queue_ids = self.processForDistro(distro, target_policy)
- self.txn.commit()
- target_policy.postprocessSuccesses(queue_ids)
- self.txn.commit()
-
+ self.processForDistro(distro)
+ self.txn.commit()
finally:
self.logger.debug("Rolling back any remaining transactions.")
self.txn.abort()
-
return 0
=== modified file 'lib/lp/soyuz/tests/test_processaccepted.py'
--- lib/lp/soyuz/tests/test_processaccepted.py 2014-08-05 10:34:11 +0000
+++ lib/lp/soyuz/tests/test_processaccepted.py 2014-08-09 09:23:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test process-accepted.py"""
@@ -157,9 +157,11 @@
upload for upload in uploads
if upload.package_upload.status ==
PackageUploadStatus.DONE])
- self.assertEqual(
- min(len(uploads), inner_self.commit_count),
- done_count)
+ # We actually commit twice for each upload: once for the
+ # queue item itself, and again to close its bugs.
+ self.assertIn(
+ min(len(uploads) * 2, inner_self.commit_count),
+ (done_count * 2, (done_count * 2) - 1))
script = self.getScript([])
switch_dbuser(self.dbuser)
Follow ups
