launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03976
[Merge] lp:~jtv/launchpad/bug-394645 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-394645 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #394645 in Launchpad itself: "IDistroSeries.getQueueItems deprecated by IPackageUploadSet.getPackageUploads"
https://bugs.launchpad.net/launchpad/+bug/394645
Bug #537335 in Launchpad itself: "DistroSeries.getQueueItems is undocumented and fails to return ordered results"
https://bugs.launchpad.net/launchpad/+bug/537335
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-394645/+merge/64787
= Summary =
The Soyuz +queue pages and the queue.py script collect queue items to display by calling DistroSeries.getQueueItems. This method is deprecated in favour of DistroSeries.getPackageUploads, which forwards to PackageUploadSet.getAll. The latter is essentially a Storm rewrite (getQueueItems composed SQL text directly), but has also been modernized to recognize PackageUploads (which is what a "queue item" really is) that have PackageCopyJobs attached but none of the more traditional sources, builds, or custom files.
As a result, this will make "copy-job uploads" show up on the DistroSeries:+queue pages and in the output of scripts/ftpmaster-tools/queue.py. It also fixes the problem of indeterministic sorting in getQueueItems.
== Proposed fix ==
Easy, really: in previous branches I updated getPackageUploads to be a drop-in replacement for getQueueItems. There are also tons of getQueueItems calls in tests (plus deliberate sorting of its results where output needed to be in a deterministic order), which I will clean up in a separate branch.
== Pre-implementation notes ==
This is the last of the current high-priority tickets for the red squad's distro-derivation work. After this I suppose we can promote a new batch of "regular" tickets to be of relatively high priority.
Julian and I agreed that the result ordering in getAll, by descending id, is probably the best choice.
== Implementation details ==
You'll note that getAll's "status" parameter can be a single status DBItem, but also a list of statuses. I documented this and made use of it to speed up the checking code that verifies before deriving one distroseries from another that the parent has no pending uploads of certain statuses. This code is entirely ordering-independent so there's no more need for it to loop over the list of statuses and query each separately.
== Tests ==
I cast a wide net for tests, and ran them through EC2:
{{{
./bin/test -vvc lp.registry -t queue
./bin/test -vvc lp.soyuz
}}}
== Demo and Q/A ==
The queue script and +queue pages should work as before (although ordering and batching should be a bit more consistent now). In addition though, a PackageUpload without any source, build, or custom file attached but with a PackageCopyJob will show up which wasn't the case before.
In local testing I created one of these by running "factory.makeCopyJobPackageUpload()" in "make harness" and committing. The script will display the upload if pointed at the right pocket: use script options -d <upload.distroseries.distribution.name> and -s <upload.distroseries.name>-backports. (The factory picks the backports pocket by default, but the script shows the release pocket by default).
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/scripts/initialize_distroseries.py
lib/lp/soyuz/model/queue.py
lib/lp/soyuz/interfaces/queue.py
lib/lp/soyuz/browser/queue.py
lib/lp/soyuz/scripts/processaccepted.py
lib/lp/registry/interfaces/distroseries.py
lib/lp/soyuz/scripts/queue.py
--
https://code.launchpad.net/~jtv/launchpad/bug-394645/+merge/64787
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-394645 into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py 2011-06-15 02:37:35 +0000
+++ lib/lp/registry/interfaces/distroseries.py 2011-06-16 07:51:26 +0000
@@ -537,7 +537,8 @@
:param created_since_date: If specified, only returns items uploaded
since the timestamp supplied.
- :param status: Filter results by this `PackageUploadStatus`
+ :param status: Filter results by this `PackageUploadStatus`, or list
+ of `PackageUploadStatus`es.
:param archive: Filter results for this `IArchive`
:param pocket: Filter results by this `PackagePublishingPocket`
:param custom_type: Filter results by this `PackageUploadCustomFormat`
=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py 2011-06-13 11:51:46 +0000
+++ lib/lp/soyuz/browser/queue.py 2011-06-16 07:51:26 +0000
@@ -89,19 +89,14 @@
self.filtered_options = []
for state in valid_states:
- if state == self.state:
- selected = True
- else:
- selected = False
+ selected = (state == self.state)
self.filtered_options.append(
- dict(name=state.title, value=state.value, selected=selected)
- )
+ dict(name=state.title, value=state.value, selected=selected))
- # request context queue items according the selected state
- queue_items = self.context.getQueueItems(
+ queue_items = self.context.getPackageUploads(
status=self.state, name=self.name_filter)
- self.batchnav = BatchNavigator(queue_items, self.request,
- size=QUEUE_SIZE)
+ self.batchnav = BatchNavigator(
+ queue_items, self.request, size=QUEUE_SIZE)
def builds_dict(self, upload_ids, binary_files):
"""Return a dictionary of PackageUploadBuild keyed on build ID.
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py 2011-06-15 02:41:34 +0000
+++ lib/lp/soyuz/interfaces/queue.py 2011-06-16 07:51:26 +0000
@@ -616,7 +616,8 @@
:param created_since_date: If specified, only returns items uploaded
since the timestamp supplied.
- :param status: Filter results by this `PackageUploadStatus`
+ :param status: Filter results by this `PackageUploadStatus`, or list
+ of `PackageUploadStatus`es.
:param archive: Filter results for this `IArchive`
:param pocket: Filter results by this `PackagePublishingPocket`
:param custom_type: Filter results by this `PackageUploadCustomFormat`
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2011-06-15 02:41:34 +0000
+++ lib/lp/soyuz/model/queue.py 2011-06-16 07:51:26 +0000
@@ -1391,16 +1391,6 @@
archive=None, pocket=None, custom_type=None, name=None,
version=None, exact_match=False):
"""See `IPackageUploadSet`."""
- # XXX Julian 2009-07-02 bug=394645
- # This method is an incremental deprecation of
- # IDistroSeries.getQueueItems(). It's basically re-writing it
- # using Storm queries instead of SQLObject, but not everything
- # is implemented yet. When it is, this comment and the old
- # method can be removed and call sites updated to use this one.
-
- # XXX 2011-06-11 JeroenVermeulen bug=795651: The "archive"
- # argument is currently ignored. Not sure why.
-
# Avoid circular imports.
from lp.soyuz.model.packagecopyjob import PackageCopyJob
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py 2011-06-14 19:49:18 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py 2011-06-16 07:51:26 +0000
@@ -124,14 +124,16 @@
"""
# only the RELEASE pocket is inherited, so we only check
# queue items for it.
- for queue in (
- PackageUploadStatus.NEW, PackageUploadStatus.ACCEPTED,
- PackageUploadStatus.UNAPPROVED):
- items = self.parent.getQueueItems(
- queue, pocket=PackagePublishingPocket.RELEASE)
- if items:
- raise InitializationError(
- "Parent series queues are not empty.")
+ statuses = [
+ PackageUploadStatus.NEW,
+ PackageUploadStatus.ACCEPTED,
+ PackageUploadStatus.UNAPPROVED,
+ ]
+ items = self.parent.getPackageUploads(
+ status=statuses, pocket=PackagePublishingPocket.RELEASE)
+ if not items.is_empty():
+ raise InitializationError(
+ "Parent series queues are not empty.")
def _checkSeries(self):
error = (
=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py 2011-05-27 21:12:25 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py 2011-06-16 07:51:26 +0000
@@ -265,8 +265,8 @@
self.logger.debug("Processing queue for %s %s" % (
distroseries.name, description))
- queue_items = distroseries.getQueueItems(
- PackageUploadStatus.ACCEPTED, archive=archive)
+ queue_items = distroseries.getPackageUploads(
+ status=PackageUploadStatus.ACCEPTED, archive=archive)
for queue_item in queue_items:
self.logger.debug(
"Processing queue item %d" % queue_item.id)
=== modified file 'lib/lp/soyuz/scripts/queue.py'
--- lib/lp/soyuz/scripts/queue.py 2011-06-15 02:41:34 +0000
+++ lib/lp/soyuz/scripts/queue.py 2011-06-16 07:51:26 +0000
@@ -194,12 +194,10 @@
term, version = term.strip().split('/')
# Expand SQLObject results.
- # XXX 2011-06-13 JeroenVermeulen bug=394645: This should
- # use getPackageUploads, not getQueueItems, so that it
- # will also include copy-job uploads.
- for item in self.distroseries.getQueueItems(
+ queue_items = self.distroseries.getPackageUploads(
status=self.queue, name=term, version=version,
- exact_match=self.exact_match, pocket=self.pocket):
+ exact_match=self.exact_match, pocket=self.pocket)
+ for item in queue_items:
if item not in self.items:
self.items.append(item)
self.package_names.append(term)
@@ -683,7 +681,7 @@
# check syntax, abort process if anything gets wrong
try:
action = terms[0]
- arguments = terms[1:]
+ arguments = [unicode(term) for term in terms[1:]]
except IndexError:
raise CommandRunnerError('Invalid sentence, use help.')
