launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09478
[Merge] lp:~cjwatson/launchpad/queue-api-refactor-overrides into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-api-refactor-overrides into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1006173 in Launchpad itself: "Queue tool requires direct DB access"
https://bugs.launchpad.net/launchpad/+bug/1006173
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-api-refactor-overrides/+merge/113074
== Summary ==
The PackageUpload.overrideBinaries interface will be unacceptably slow when exported on the API.
The PackageUpload.overrideSource and PackageUpload.overrideBinaries interfaces will need to accept strings as well as component/section/priority objects once exported on the API.
== Proposed fix ==
There are source packages with lots of binaries that sometimes need to be overridden individually (e.g. linux) and API requests aren't especially fast. I've therefore amended Archive.overrideBinaries to take a similar list of dicts as a "changes" parameter, allowing many override changes to be made in a single request.
== Pre-implementation notes ==
Override design based on conversations with William Grant.
This is split out of a larger branch, https://code.launchpad.net/~cjwatson/launchpad/queue-api/+merge/108967, at Benji's suggestion in an attempt to make it more easily reviewable.
== LOC Rationale ==
+86. As noted in https://code.launchpad.net/~cjwatson/launchpad/queue-api/+merge/108967, this arc will be LoC-negative.
== Tests ==
bin/test -vvct nascentupload-ddebs.txt -t distroseriesqueue.txt
Test coverage should be sufficient to avoid regressions, but will nevertheless be improved by a follow-up branch to export things on the webservice.
--
https://code.launchpad.net/~cjwatson/launchpad/queue-api-refactor-overrides/+merge/113074
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-api-refactor-overrides into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/tests/nascentupload-ddebs.txt'
--- lib/lp/archiveuploader/tests/nascentupload-ddebs.txt 2012-01-20 16:11:11 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-ddebs.txt 2012-07-02 16:53:22 +0000
@@ -89,7 +89,8 @@
>>> switch_dbuser('launchpad')
- >>> bin.queue_root.overrideBinaries(main, devel, None, [main, universe])
+ >>> bin.queue_root.overrideBinaries(
+ ... [{"component": main, "section": devel}], [main, universe])
True
>>> bin.queue_root.acceptFromQueue()
=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py 2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/browser/queue.py 2012-07-02 16:53:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser views for package queue."""
@@ -385,9 +385,13 @@
try:
source_overridden = queue_item.overrideSource(
new_component, new_section, allowed_components)
+ binary_changes = [{
+ "component": new_component,
+ "section": new_section,
+ "priority": new_priority,
+ }]
binary_overridden = queue_item.overrideBinaries(
- new_component, new_section, new_priority,
- allowed_components)
+ binary_changes, allowed_components)
except QueueInconsistentStateError, info:
failure.append("FAILED: %s (%s)" %
(queue_item.displayname, info))
=== modified file 'lib/lp/soyuz/doc/distroseriesqueue.txt'
--- lib/lp/soyuz/doc/distroseriesqueue.txt 2012-01-06 11:08:30 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue.txt 2012-07-02 16:53:22 +0000
@@ -710,29 +710,25 @@
main/base/Important
>>> from lp.soyuz.enums import PackagePublishingPriority
+ >>> binary_changes = [{
+ ... "component": restricted,
+ ... "section": web,
+ ... "priority": PackagePublishingPriority.EXTRA,
+ ... }]
>>> print item.overrideBinaries(
- ... new_component=restricted,
- ... new_section=web,
- ... new_priority=PackagePublishingPriority.EXTRA,
- ... allowed_components=(universe,))
+ ... binary_changes, allowed_components=(universe,))
Traceback (most recent call last):
...
QueueInconsistentStateError: No rights to override to restricted
>>> print item.overrideBinaries(
- ... new_component=restricted,
- ... new_section=web,
- ... new_priority=PackagePublishingPriority.EXTRA,
- ... allowed_components=(restricted,))
+ ... binary_changes, allowed_components=(restricted,))
Traceback (most recent call last):
...
QueueInconsistentStateError: No rights to override from main
>>> print item.overrideBinaries(
- ... new_component=restricted,
- ... new_section=web,
- ... new_priority=PackagePublishingPriority.EXTRA,
- ... allowed_components=(main,restricted))
+ ... binary_changes, allowed_components=(main,restricted))
True
>>> print "%s/%s/%s" % (
... binary_package.component.name,
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2012-06-19 23:49:20 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2012-07-02 16:53:22 +0000
@@ -43,6 +43,8 @@
'NoSuchPPA',
'NoTokensForTeams',
'PocketNotFound',
+ 'PriorityNotFound',
+ 'SectionNotFound',
'VersionRequiresName',
'default_name_by_purpose',
'validate_external_dependencies',
@@ -156,7 +158,7 @@
class ComponentNotFound(NameLookupFailed):
- """Invalid source name."""
+ """Invalid component name."""
_message_prefix = 'No such component'
@@ -165,6 +167,16 @@
"""Invalid component name."""
+class SectionNotFound(NameLookupFailed):
+ """Invalid section name."""
+ _message_prefix = "No such section"
+
+
+class PriorityNotFound(NameLookupFailed):
+ """Invalid priority name."""
+ _message_prefix = "No such priority"
+
+
class NoSuchPPA(NameLookupFailed):
"""Raised when we try to look up an PPA that doesn't exist."""
_message_prefix = "No such ppa"
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py 2012-06-30 17:41:37 +0000
+++ lib/lp/soyuz/interfaces/queue.py 2012-07-02 16:53:22 +0000
@@ -349,16 +349,16 @@
:return: True if the source was overridden.
"""
- def overrideBinaries(new_component, new_section, new_priority,
- allowed_components):
- """Override all the binaries in a binary queue item.
+ def overrideBinaries(changes, allowed_components):
+ """Override binary packages in a binary queue item.
- :param new_component: An IComponent to replace the existing one
- in the upload's source.
- :param new_section: An ISection to replace the existing one
- in the upload's source.
- :param new_priority: A valid PackagePublishingPriority to replace
- the existing one in the upload's binaries.
+ :param changes: A sequence of mappings of changes to apply. Each
+ change mapping may have a "name" item which specifies the binary
+ package name to override; otherwise, the change applies to all
+ binaries in the upload. It may also have "component", "section",
+ and "priority" items which replace the corresponding existing
+ one in the upload's overridden binaries. Any missing items are
+ left unchanged.
:param allowed_components: A sequence of components that the
callsite is allowed to override from and to.
@@ -366,10 +366,7 @@
or the new_component are not in the allowed_components
sequence.
- The override values may be None, in which case they are not
- changed.
-
- :return: True if the binaries were overridden.
+ :return: True if any binaries were overridden.
"""
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2012-06-30 23:34:51 +0000
+++ lib/lp/soyuz/model/queue.py 2012-07-02 16:53:22 +0000
@@ -72,11 +72,18 @@
PackageUploadCustomFormat,
PackageUploadStatus,
)
-from lp.soyuz.interfaces.archive import MAIN_ARCHIVE_PURPOSES
+from lp.soyuz.interfaces.archive import (
+ ComponentNotFound,
+ MAIN_ARCHIVE_PURPOSES,
+ PriorityNotFound,
+ SectionNotFound,
+ )
+from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.packagecopyjob import IPackageCopyJobSource
from lp.soyuz.interfaces.publishing import (
IPublishingSet,
ISourcePackagePublishingHistory,
+ name_priority_map,
)
from lp.soyuz.interfaces.queue import (
IPackageUpload,
@@ -91,6 +98,7 @@
QueueSourceAcceptError,
QueueStateWriteProtectedError,
)
+from lp.soyuz.interfaces.section import ISectionSet
from lp.soyuz.model.binarypackagename import BinaryPackageName
from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
from lp.soyuz.pas import BuildDaemonPackagesArchSpecific
@@ -913,6 +921,33 @@
"""See `IPackageUpload`."""
return getUtility(IPackageCopyJobSource).wrap(self.package_copy_job)
+ def _nameToComponent(self, component):
+ """Helper to convert a possible string component to IComponent."""
+ try:
+ if isinstance(component, basestring):
+ component = getUtility(IComponentSet)[component]
+ return component
+ except NotFoundError:
+ raise ComponentNotFound(component)
+
+ def _nameToSection(self, section):
+ """Helper to convert a possible string section to ISection."""
+ try:
+ if isinstance(section, basestring):
+ section = getUtility(ISectionSet)[section]
+ return section
+ except NotFoundError:
+ raise SectionNotFound(section)
+
+ def _nameToPriority(self, priority):
+ """Helper to convert a possible string priority to its enum."""
+ try:
+ if isinstance(priority, basestring):
+ priority = name_priority_map[priority]
+ return priority
+ except KeyError:
+ raise PriorityNotFound(priority)
+
def _overrideSyncSource(self, new_component, new_section,
allowed_components):
"""Override source on the upload's `PackageCopyJob`, if any."""
@@ -959,6 +994,9 @@
# Nothing needs overriding, bail out.
return False
+ new_component = self._nameToComponent(new_component)
+ new_section = self._nameToSection(new_section)
+
if new_component not in list(allowed_components) + [None]:
raise QueueInconsistentStateError(
"No rights to override to %s" % new_component.name)
@@ -969,35 +1007,73 @@
self._overrideNonSyncSource(
new_component, new_section, allowed_components))
- def overrideBinaries(self, new_component, new_section, new_priority,
- allowed_components):
+ def _filterBinaryChanges(self, changes):
+ """Process a binary changes mapping into a more convenient form."""
+ changes_by_name = {}
+ changes_for_all = None
+
+ for change in changes:
+ filtered_change = {}
+ if "component" in change:
+ filtered_change["component"] = self._nameToComponent(
+ change["component"])
+ if "section" in change:
+ filtered_change["section"] = self._nameToSection(
+ change["section"])
+ if "priority" in change:
+ filtered_change["priority"] = self._nameToPriority(
+ change["priority"])
+
+ if "name" in change:
+ changes_by_name[change["name"]] = filtered_change
+ else:
+ # Changes with no "name" item provide a default for all
+ # binaries.
+ changes_for_all = filtered_change
+
+ return changes_by_name, changes_for_all
+
+ def overrideBinaries(self, changes, allowed_components):
"""See `IPackageUpload`."""
if not self.contains_build:
return False
- if (new_component is None and new_section is None and
- new_priority is None):
+ if not changes:
# Nothing needs overriding, bail out.
return False
- if new_component not in allowed_components:
+ changes_by_name, changes_for_all = self._filterBinaryChanges(changes)
+
+ new_components = set()
+ for change in changes_by_name.values():
+ if "component" in change:
+ new_components.add(change["component"])
+ if changes_for_all is not None and "component" in changes_for_all:
+ new_components.add(changes_for_all["component"])
+ disallowed_components = sorted(
+ component.name
+ for component in new_components.difference(allowed_components))
+ if disallowed_components:
raise QueueInconsistentStateError(
- "No rights to override to %s" % new_component.name)
+ "No rights to override to %s" %
+ ", ".join(disallowed_components))
+ made_changes = False
for build in self.builds:
for binarypackage in build.build.binarypackages:
- if binarypackage.component not in allowed_components:
- # The old or the new component is not in the list of
- # allowed components to override.
- raise QueueInconsistentStateError(
- "No rights to override from %s" % (
- binarypackage.component.name))
- binarypackage.override(
- component=new_component,
- section=new_section,
- priority=new_priority)
+ change = changes_by_name.get(
+ binarypackage.name, changes_for_all)
+ if change is not None:
+ if binarypackage.component not in allowed_components:
+ # The old component is not in the list of allowed
+ # components to override.
+ raise QueueInconsistentStateError(
+ "No rights to override from %s" % (
+ binarypackage.component.name))
+ binarypackage.override(**change)
+ made_changes = True
- return bool(self.builds)
+ return made_changes
class PackageUploadBuild(SQLBase):
Follow ups
