launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17192
[Merge] lp:~wgrant/launchpad/overrides-copies into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/overrides-copies into lp:launchpad with lp:~wgrant/launchpad/overrides-init as a prerequisite.
Commit message:
Use normal override policies in PlainPackageCopyJob._checkPolicies. IOverride now has a 'new' flag to make that feasible.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1073755 in Launchpad itself: "package overrides functionality is inconsistently implemented"
https://bugs.launchpad.net/launchpad/+bug/1073755
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/overrides-copies/+merge/227669
This branch removes manual OverridePolicy instantiation from packagecopier. Instead it just uses Archive.getOverridePolicy.
One infrastructure enhancement was required: an IOverride can now specify whether it's new or not. FromExistingOverridePolicy sets new=False unless the overrides are derived from a Deleted publication, and UnknownOverridePolicy always sets new=True.
--
https://code.launchpad.net/~wgrant/launchpad/overrides-copies/+merge/227669
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/overrides-copies into lp:launchpad.
=== modified file 'lib/lp/soyuz/adapters/overrides.py'
--- lib/lp/soyuz/adapters/overrides.py 2014-07-22 06:13:12 +0000
+++ lib/lp/soyuz/adapters/overrides.py 2014-07-22 06:13:13 +0000
@@ -56,6 +56,7 @@
component = Attribute("The IComponent override")
section = Attribute("The ISection override")
version = Attribute("The exclusive lower version limit")
+ new = Attribute("Whether the package is considered new")
class ISourceOverride(IOverride):
@@ -80,10 +81,11 @@
class Override:
"""See `IOverride`."""
- def __init__(self, component=None, section=None, version=None):
+ def __init__(self, component=None, section=None, version=None, new=None):
self.component = component
self.section = section
self.version = version
+ self.new = new
def __ne__(self, other):
return not self == other
@@ -104,13 +106,14 @@
self.__class__ == other.__class__ and
self.component == other.component and
self.section == other.section and
- self.version == other.version)
+ self.version == other.version and
+ self.new == other.new)
def __repr__(self):
return (
- "<%s at %x component=%r section=%r version=%r>" %
+ "<%s at %x component=%r section=%r version=%r new=%r>" %
(self.__class__.__name__, id(self), self.component, self.section,
- self.version))
+ self.version, self.new))
class BinaryOverride(Override):
@@ -118,9 +121,9 @@
implements(IBinaryOverride)
def __init__(self, component=None, section=None, priority=None,
- phased_update_percentage=None, version=None):
+ phased_update_percentage=None, version=None, new=None):
super(BinaryOverride, self).__init__(
- component=component, section=section, version=version)
+ component=component, section=section, version=version, new=new)
self.priority = priority
self.phased_update_percentage = phased_update_percentage
@@ -131,14 +134,16 @@
self.section == other.section and
self.priority == other.priority and
self.phased_update_percentage == other.phased_update_percentage and
- self.version == other.version)
+ self.version == other.version and
+ self.new == other.new)
def __repr__(self):
return (
"<%s at %x component=%r section=%r priority=%r "
- "phased_update_percentage=%r version=%r>" %
+ "phased_update_percentage=%r version=%r new=%r>" %
(self.__class__.__name__, id(self), self.component, self.section,
- self.priority, self.phased_update_percentage, self.version))
+ self.priority, self.phased_update_percentage, self.version,
+ self.new))
class IOverridePolicy(Interface):
@@ -241,6 +246,7 @@
(SourcePackagePublishingHistory.sourcepackagenameID,
SourcePackagePublishingHistory.componentID,
SourcePackagePublishingHistory.sectionID,
+ SourcePackagePublishingHistory.status,
SourcePackageRelease.version),
SourcePackageRelease.id ==
SourcePackagePublishingHistory.sourcepackagereleaseID,
@@ -258,12 +264,14 @@
).config(
distinct=(
SourcePackagePublishingHistory.sourcepackagenameID,)),
- id_resolver((SourcePackageName, Component, Section, None)),
+ id_resolver((SourcePackageName, Component, Section, None, None)),
pre_iter_hook=eager_load)
return dict(
(name, SourceOverride(
- component=component, section=section, version=version))
- for (name, component, section, version) in already_published)
+ component=component, section=section, version=version,
+ new=(status == PackagePublishingStatus.DELETED)))
+ for (name, component, section, status, version)
+ in already_published)
def calculateBinaryOverrides(self, binaries):
def eager_load(rows):
@@ -304,6 +312,7 @@
BinaryPackagePublishingHistory.componentID,
BinaryPackagePublishingHistory.sectionID,
BinaryPackagePublishingHistory.priority,
+ BinaryPackagePublishingHistory.status,
BinaryPackageRelease.version),
BinaryPackageRelease.id ==
BinaryPackagePublishingHistory.binarypackagereleaseID,
@@ -322,10 +331,10 @@
),
id_resolver(
(BinaryPackageName, DistroArchSeries, Component, Section,
- None, None)),
+ None, None, None)),
pre_iter_hook=eager_load)
overrides = {}
- for name, das, component, section, priority, ver in already_published:
+ for (name, das, comp, sect, prio, status, ver) in already_published:
# These details can always fulfill their own archtag, and may
# satisfy a None archtag if the DAS is nominatedarchindep.
if not self.any_arch:
@@ -339,9 +348,10 @@
if key not in binaries:
continue
overrides[key] = BinaryOverride(
- component=component, section=section, priority=priority,
+ component=comp, section=sect, priority=prio,
phased_update_percentage=self.phased_update_percentage,
- version=ver)
+ version=ver,
+ new=(status == PackagePublishingStatus.DELETED))
return overrides
@@ -387,7 +397,8 @@
component=(
self.archive.default_component or
UnknownOverridePolicy.getComponentOverride(
- override.component, return_component=True))))
+ override.component, return_component=True)),
+ new=True))
for spn, override in sources.items())
def calculateBinaryOverrides(self, binaries):
@@ -395,7 +406,7 @@
IComponentSet)['universe']
return dict(
((binary_package_name, architecture_tag), BinaryOverride(
- component=default_component,
+ component=default_component, new=True,
phased_update_percentage=self.phased_update_percentage))
for binary_package_name, architecture_tag in binaries.keys())
=== modified file 'lib/lp/soyuz/adapters/tests/test_overrides.py'
--- lib/lp/soyuz/adapters/tests/test_overrides.py 2014-07-22 06:13:12 +0000
+++ lib/lp/soyuz/adapters/tests/test_overrides.py 2014-07-22 06:13:13 +0000
@@ -41,7 +41,7 @@
expected = {
spph.sourcepackagerelease.sourcepackagename: SourceOverride(
component=spph.component, section=spph.section,
- version=spph.sourcepackagerelease.version)}
+ version=spph.sourcepackagerelease.version, new=False)}
self.assertEqual(expected, overrides)
def test_source_overrides_pocket(self):
@@ -86,11 +86,11 @@
overrides = FromExistingOverridePolicy(
spph.distroseries.main_archive, spph.distroseries,
spph.pocket).calculateSourceOverrides(
- {spn: SourceOverride(spn)})
+ {spn: SourceOverride()})
self.assertEqual(
{spn: SourceOverride(
component=spph.component, section=spph.section,
- version=spph.sourcepackagerelease.version)},
+ version=spph.sourcepackagerelease.version, new=False)},
overrides)
def test_source_overrides_can_include_deleted(self):
@@ -118,7 +118,7 @@
self.assertEqual(
{spn: SourceOverride(
component=spph.component, section=spph.section,
- version=spph.sourcepackagerelease.version)},
+ version=spph.sourcepackagerelease.version, new=False)},
overrides)
# But with include_deleted=True the newer Deleted publication is
@@ -130,7 +130,7 @@
self.assertEqual(
{spn: SourceOverride(
component=deleted_spph.component, section=deleted_spph.section,
- version=deleted_spph.sourcepackagerelease.version)},
+ version=deleted_spph.sourcepackagerelease.version, new=True)},
overrides)
def test_source_overrides_constant_query_count(self):
@@ -195,18 +195,18 @@
bpph1.distroarchseries.architecturetag):
BinaryOverride(
component=bpph1.component, section=bpph1.section,
- priority=bpph1.priority,
+ priority=bpph1.priority, new=False,
version=bpph1.binarypackagerelease.version),
(bpph2.binarypackagerelease.binarypackagename,
bpph2.distroarchseries.architecturetag):
BinaryOverride(
component=bpph2.component, section=bpph2.section,
- priority=bpph2.priority,
+ priority=bpph2.priority, new=False,
version=bpph2.binarypackagerelease.version),
(bpph2.binarypackagerelease.binarypackagename, None):
BinaryOverride(
component=bpph2.component, section=bpph2.section,
- priority=bpph2.priority,
+ priority=bpph2.priority, new=False,
version=bpph2.binarypackagerelease.version),
}
self.assertEqual(expected, overrides)
@@ -273,7 +273,8 @@
pocket=pocket, binarypackagename=bpn, architecturespecific=True)
bpph_override = BinaryOverride(
component=bpph.component, section=bpph.section,
- priority=bpph.priority, version=bpph.binarypackagerelease.version)
+ priority=bpph.priority, version=bpph.binarypackagerelease.version,
+ new=False)
# With any_arch=False only amd64 is found.
policy = FromExistingOverridePolicy(
@@ -324,7 +325,7 @@
{(bpn, 'amd64'): BinaryOverride(
component=bpph.component, section=bpph.section,
priority=bpph.priority,
- version=bpph.binarypackagerelease.version)},
+ version=bpph.binarypackagerelease.version, new=False)},
overrides)
# But with include_deleted=True we get the newer Deleted pub instead.
@@ -337,7 +338,7 @@
{(bpn, 'amd64'): BinaryOverride(
component=deleted_bpph.component, section=deleted_bpph.section,
priority=deleted_bpph.priority,
- version=deleted_bpph.binarypackagerelease.version)},
+ version=deleted_bpph.binarypackagerelease.version, new=True)},
overrides)
def test_binary_overrides_constant_query_count(self):
@@ -411,7 +412,7 @@
zip(spns, ('main', 'contrib', 'non-free'))))
expected = dict(
(spn, SourceOverride(
- component=getUtility(IComponentSet)[component]))
+ component=getUtility(IComponentSet)[component], new=True))
for spn, component in
zip(spns, ('universe', 'multiverse', 'multiverse')))
self.assertEqual(expected, overrides)
@@ -434,7 +435,7 @@
zip(spns, ('main', 'contrib', 'non-free'))))
expected = dict(
(spn, SourceOverride(
- component=getUtility(IComponentSet)[component]))
+ component=getUtility(IComponentSet)[component], new=True))
for spn, component in zip(spns, ('main', 'main', 'main')))
self.assertEqual(expected, overrides)
@@ -452,7 +453,7 @@
universe = getUtility(IComponentSet)['universe']
expected = {
(bpph.binarypackagerelease.binarypackagename, None):
- BinaryOverride(component=universe)}
+ BinaryOverride(component=universe, new=True)}
self.assertEqual(expected, overrides)
@@ -465,7 +466,7 @@
# policy.
universe = getUtility(IComponentSet)['universe']
spns = [self.factory.makeSourcePackageName()]
- expected = {spns[0]: SourceOverride(component=universe)}
+ expected = {spns[0]: SourceOverride(component=universe, new=True)}
distroseries = self.factory.makeDistroSeries()
pocket = self.factory.getAnyPocket()
for i in xrange(8):
@@ -476,9 +477,9 @@
expected[spph.sourcepackagerelease.sourcepackagename] = (
SourceOverride(
component=spph.component, section=spph.section,
- version=spph.sourcepackagerelease.version))
+ version=spph.sourcepackagerelease.version, new=False))
spns.append(self.factory.makeSourcePackageName())
- expected[spns[-1]] = SourceOverride(component=universe)
+ expected[spns[-1]] = SourceOverride(component=universe, new=True)
policy = UbuntuOverridePolicy(
distroseries.main_archive, distroseries, pocket)
overrides = policy.calculateSourceOverrides(
@@ -510,14 +511,14 @@
expected[(bpn, distroarchseries.architecturetag)] = (
BinaryOverride(
component=bpph.component, section=bpph.section,
- priority=bpph.priority,
+ priority=bpph.priority, new=False,
version=bpph.binarypackagerelease.version))
for i in xrange(2):
distroarchseries = self.factory.makeDistroArchSeries(
distroseries=distroseries)
bpns.append((bpn, distroarchseries.architecturetag))
expected[bpn, distroarchseries.architecturetag] = BinaryOverride(
- component=universe)
+ component=universe, new=True)
distroseries.nominatedarchindep = distroarchseries
policy = UbuntuOverridePolicy(
distroseries.main_archive, distroseries, pocket)
@@ -548,12 +549,12 @@
expected[(bpn, distroarchseries.architecturetag)] = BinaryOverride(
component=bpph.component, section=bpph.section,
priority=bpph.priority, phased_update_percentage=50,
- version=bpph.binarypackagerelease.version)
+ version=bpph.binarypackagerelease.version, new=False)
distroarchseries = self.factory.makeDistroArchSeries(
distroseries=distroseries)
bpns.append((bpn, distroarchseries.architecturetag))
expected[(bpn, distroarchseries.architecturetag)] = BinaryOverride(
- component=universe, phased_update_percentage=50)
+ component=universe, phased_update_percentage=50, new=True)
distroseries.nominatedarchindep = distroarchseries
policy = UbuntuOverridePolicy(
distroseries.main_archive, distroseries, pocket,
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2014-07-22 06:13:12 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2014-07-22 06:13:13 +0000
@@ -58,7 +58,6 @@
from lp.services.job.runner import BaseRunnableJob
from lp.services.mail.sendmail import format_address_for_person
from lp.soyuz.adapters.overrides import (
- FromExistingOverridePolicy,
SourceOverride,
UnknownOverridePolicy,
)
@@ -518,50 +517,42 @@
# This helper will only return if it's safe to carry on with the
# copy, otherwise it raises SuspendJobException to tell the job
# runner to suspend the job.
- override_policy = FromExistingOverridePolicy(
- self.target_archive, self.target_distroseries, None)
- ancestry = override_policy.calculateSourceOverrides(
- {source_name: SourceOverride()})
-
- copy_policy = self.getPolicyImplementation()
-
- if len(ancestry) == 0:
- # We need to get the default overrides and put them in the
- # metadata.
- default_policy = UnknownOverridePolicy(
+ override_policy = self.target_archive.getOverridePolicy(
+ self.target_distroseries, None)
+ if override_policy is None:
+ override_policy = UnknownOverridePolicy(
self.target_archive, self.target_distroseries, None)
- defaults = default_policy.calculateSourceOverrides(
- {source_name: SourceOverride(component=source_component)})
- self.addSourceOverride(defaults[source_name])
- if auto_approve:
- auto_approve = self.target_archive.canAdministerQueue(
- self.requester, self.getSourceOverride().component,
- self.target_pocket, self.target_distroseries)
-
+ overrides = override_policy.calculateSourceOverrides(
+ {source_name: SourceOverride(component=source_component)})
+ override = overrides[source_name]
+
+ copy_policy = self.getPolicyImplementation()
+
+ # Put the (existing or default) override in the metadata.
+ self.addSourceOverride(override)
+ if auto_approve:
+ auto_approve = self.target_archive.canAdministerQueue(
+ self.requester, self.getSourceOverride().component,
+ self.target_pocket, self.target_distroseries)
+
+ if override.new:
approve_new = auto_approve or copy_policy.autoApproveNew(
self.target_archive, self.target_distroseries,
self.target_pocket)
-
if not approve_new:
# There's no existing package with the same name and the
# policy says unapproved, so we poke it in the NEW queue.
self._createPackageUpload()
raise SuspendJobException
else:
- # Put the existing override in the metadata.
- self.addSourceOverride(ancestry[source_name])
- if auto_approve:
- auto_approve = self.target_archive.canAdministerQueue(
- self.requester, self.getSourceOverride().component,
- self.target_pocket, self.target_distroseries)
-
- # The package is not new (it has ancestry) so check the copy
- # policy for existing packages.
- approve_existing = auto_approve or copy_policy.autoApprove(
- self.target_archive, self.target_distroseries, self.target_pocket)
- if not approve_existing:
- self._createPackageUpload(unapproved=True)
- raise SuspendJobException
+ # The package is not new (it has ancestry) so check the copy
+ # policy for existing packages.
+ approve_existing = auto_approve or copy_policy.autoApprove(
+ self.target_archive, self.target_distroseries,
+ self.target_pocket)
+ if not approve_existing:
+ self._createPackageUpload(unapproved=True)
+ raise SuspendJobException
def _rejectPackageUpload(self):
# Helper to find and reject any associated PackageUpload.
Follow ups
