launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03811
[Merge] lp:~jtv/launchpad/db-bug-790761 into lp:launchpad/db-devel
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-bug-790761 into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #790761 in Launchpad itself: "Package copy policy adapter"
https://bugs.launchpad.net/launchpad/+bug/790761
For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-790761/+merge/63202
= Summary =
We need to look up package-copying policies based on the database representation.
== Proposed fix ==
A new interface ICopyPolicy defines what we expect from copy policies. A new adapter gives you the ICopyPolicy for a given PackageCopyPolicy enum value.
== Pre-implementation notes ==
This was originally spec'ed to take an identifying policy string. But pushback on review for a preceding branch made us use an enumeration instead.
The ZCML registration for the adapter actually takes any DBItem value; I'm not aware of any way to adapt a specific DBEnumeration. Robert didn't seem to know of one either.
== Implementation details ==
I rearranged the copy-policy inheritance hierarchy a bit. Now that there's an interface for ICopyPolicy, there's no more need for a base class that raises "not implemented" errors from every method. Instead we can have tests verify directly whether each policy class implements the interface, which is slightly more like what a compiled language would do.
That also made it reasonable to remodel the two concrete policy classes not as one being a modification of the other, but as both delegating most of their logic to a separate base class. That should make the consequences of change slightly easier to oversee: the style rule of "don't inherit from concrete classes" is surprisingly effective at improving designs.
== Tests ==
{{{
./bin/test -vvc lp.soyuz.adapters.tests.test_copypolicy
./bin/test -vvc lp.soyuz.browser.tests.test_archive
}}}
== Demo and Q/A ==
The new stuff isn't really reachable yet. As long as we can still request package updates and asynchronous package syncs, it should be fine.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/adapters/tests/test_copypolicy.py
lib/lp/soyuz/adapters/copypolicy.py
lib/lp/soyuz/model/packagecopyjob.py
lib/lp/registry/interfaces/distroseriesdifference.py
database/schema/patch-2208-73-0.sql
lib/lp/soyuz/configure.zcml
lib/lp/soyuz/interfaces/packagecopyjob.py
lib/lp/testing/factory.py
lib/lp/soyuz/tests/test_packagecopyjob.py
lib/lp/registry/browser/tests/test_distroseries.py
lib/lp/soyuz/enums.py
lib/lp/soyuz/interfaces/copypolicy.py
lib/lp/soyuz/browser/tests/test_package_copying_mixin.py
lib/lp/registry/browser/distroseries.py
lib/lp/soyuz/browser/archive.py
--
https://code.launchpad.net/~jtv/launchpad/db-bug-790761/+merge/63202
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/db-bug-790761 into lp:launchpad/db-devel.
=== added file 'database/schema/patch-2208-73-0.sql'
--- database/schema/patch-2208-73-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-73-0.sql 2011-06-02 08:48:54 +0000
@@ -0,0 +1,16 @@
+-- Copyright 2011 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+-- Thanks to our feature flags, this table should be empty. If not,
+-- adding a NOT NULL column would be problematic!
+ALTER TABLE PackageCopyJob ADD COLUMN package_name text NOT NULL;
+
+ALTER TABLE PackageCopyJob ADD COLUMN copy_policy integer;
+
+-- For getPendingJobsForTargetSeries, which happens on web-request time.
+CREATE UNIQUE INDEX packagecopyjob__job_type__target_ds__id__key
+ ON PackageCopyJob(job_type, target_distroseries, id);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 73, 0);
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-05-28 11:35:38 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-06-02 08:48:54 +0000
@@ -109,7 +109,6 @@
)
from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJobSource
from lp.soyuz.interfaces.queue import IPackageUploadSet
-from lp.soyuz.model.packagecopyjob import specify_dsd_package
from lp.soyuz.model.queue import PackageUploadQueue
from lp.translations.browser.distroseries import (
check_distroseries_translations_viewable,
@@ -860,8 +859,7 @@
def pending_syncs(self):
"""Pending synchronization jobs for this distroseries.
- :return: A dict mapping (name, version) package specifications to
- pending sync jobs.
+ :return: A dict mapping package names to pending sync jobs.
"""
job_source = getUtility(IPlainPackageCopyJobSource)
return job_source.getPendingJobsPerPackage(self.context)
@@ -883,7 +881,8 @@
def hasPendingSync(self, dsd):
"""Is there a package-copying job pending to resolve `dsd`?"""
- return self.pending_syncs.get(specify_dsd_package(dsd)) is not None
+ pending_sync = self.pending_syncs.get(dsd.source_package_name.name)
+ return pending_sync is not None
def isNewerThanParent(self, dsd):
"""Is the child's version of this package newer than the parent's?
@@ -1082,9 +1081,10 @@
for dsd in self.getUpgrades():
job_source.create(
- [specify_dsd_package(dsd)], dsd.parent_series.main_archive,
- target_archive, target_distroseries,
- PackagePublishingPocket.UPDATES)
+ dsd.source_package_name.name,
+ dsd.parent_series.main_archive, target_archive,
+ target_distroseries, PackagePublishingPocket.UPDATES,
+ package_version=dsd.parent_source_version)
self.request.response.addInfoNotification(
(u"Upgrades of {context.displayname} packages have been "
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-05-31 11:55:38 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-06-02 08:48:54 +0000
@@ -75,7 +75,6 @@
)
from lp.soyuz.model.archivepermission import ArchivePermission
from lp.soyuz.model import distroseriesdifferencejob
-from lp.soyuz.model.packagecopyjob import specify_dsd_package
from lp.testing import (
anonymous_logged_in,
celebrity_logged_in,
@@ -1058,11 +1057,8 @@
self.assertEquals(1, len(jobs))
job = jobs[0]
self.assertEquals(series, job.target_distroseries)
- source_package_info = list(job.source_packages)
- self.assertEquals(1, len(source_package_info))
- self.assertEqual(
- (dsd.source_package_name.name, dsd.parent_source_version),
- source_package_info[0][:2])
+ self.assertEqual(dsd.source_package_name.name, job.package_name)
+ self.assertEqual(dsd.parent_source_version, job.package_version)
def test_upgrade_gives_feedback(self):
# requestUpgrades doesn't instantly perform package upgrades,
@@ -1392,7 +1388,7 @@
dsd = self.factory.makeDistroSeriesDifference()
view = create_initialized_view(
dsd.derived_series, '+localpackagediffs')
- view.pending_syncs = {specify_dsd_package(dsd): object()}
+ view.pending_syncs = {dsd.source_package_name.name: object()}
self.assertTrue(view.hasPendingSync(dsd))
def test_isNewerThanParent_compares_versions_not_strings(self):
@@ -1444,7 +1440,7 @@
dsd = self.factory.makeDistroSeriesDifference()
view = create_initialized_view(
dsd.derived_series, '+localpackagediffs')
- view.pending_syncs = {specify_dsd_package(dsd): object()}
+ view.pending_syncs = {dsd.source_package_name.name: object()}
self.assertFalse(view.canRequestSync(dsd))
def test_canRequestSync_returns_False_if_child_is_newer(self):
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2011-05-30 07:42:21 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2011-06-02 08:48:54 +0000
@@ -303,7 +303,8 @@
:param difference_type: The type of difference to include in the
results.
:type difference_type: `DistroSeriesDifferenceType`.
- :param source_package_name_filter: Package source name filter.
+ :param source_package_name_filter: Name of a source package. If
+ given, restricts the search to this package.
:type source_package_name_filter: unicode.
:param status: Only differences matching the status(es) will be
included.
=== modified file 'lib/lp/soyuz/adapters/copypolicy.py'
--- lib/lp/soyuz/adapters/copypolicy.py 2011-05-30 11:47:46 +0000
+++ lib/lp/soyuz/adapters/copypolicy.py 2011-06-02 08:48:54 +0000
@@ -15,57 +15,30 @@
]
+from zope.interface import implements
+
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
-
-
-class BaseCopyPolicy:
- """Encapsulation of the policies for copying a package in Launchpad."""
-
- @property
- def name(self):
- """The name of this policy that's used in adapter lookups."""
- raise AssertionError("Subclass must provide name property")
-
- def autoApprove(self, archive, distroseries, pocket):
- """Decide if the upload can be approved automatically or
- should be held in the queue.
-
- This should only be called for packages that are known not new.
-
- :param archive: The target `IArchive` for the upload.
- :param distroseries: The target `IDistroSeries` for the upload.
- :param pocket: The target `PackagePublishingPocket` for the upload.
- """
- raise AssertionError("Subclass must provide autoApprove")
-
- def autoApproveNew(self, archive, distroseries, pocket):
- """Decide if a previously unknown package is approved automatically
- or should be held in the queue.
-
- :param archive: The target `IArchive` for the upload.
- :param distroseries: The target `IDistroSeries` for the upload.
- :param pocket: The target `PackagePublishingPocket` for the upload.
- """
- raise AssertionError("Subclass must provide autoApproveNew")
-
- @property
- def send_email(self):
- """Whether or not the copy should send emails after completing."""
- raise AssertionError("Subclass must provide send_email")
-
-
-class InsecureCopyPolicy(BaseCopyPolicy):
- """A policy for copying from insecure sources."""
-
- name = "insecure"
+from lp.soyuz.enums import PackageCopyPolicy
+from lp.soyuz.interfaces.copypolicy import ICopyPolicy
+
+
+class BasicCopyPolicy:
+ """Useful standard copy policy.
+
+ This policy auto-approves all PPA uploads. For distribution archives it
+ auto-approves only uploads to the Release pocket, and only while the
+ series is not frozen.
+ """
def autoApproveNew(self, archive, distroseries=None, pocket=None):
+ """See `ICopyPolicy`."""
if archive.is_ppa:
return True
return False
def autoApprove(self, archive, distroseries, pocket):
+ """See `ICopyPolicy`."""
if archive.is_ppa:
return True
@@ -80,16 +53,34 @@
return False
- @property
- def send_email(self):
- return True
-
-
-class SyncCopyPolicy(InsecureCopyPolicy):
+
+class InsecureCopyPolicy(BasicCopyPolicy):
+ """A policy for copying from insecure sources."""
+ implements(ICopyPolicy)
+
+ enum_value = PackageCopyPolicy.INSECURE
+
+ send_email = True
+
+
+class SyncCopyPolicy(BasicCopyPolicy):
"""A policy for mass 'sync' copies."""
-
- name = "sync"
-
- @property
- def send_email(self):
- return False
+ implements(ICopyPolicy)
+
+ enum_value = PackageCopyPolicy.SYNC
+
+ send_email = False
+
+
+policies = [
+ InsecureCopyPolicy,
+ SyncCopyPolicy,
+ ]
+
+
+enum_to_policy = dict((policy.enum_value, policy()) for policy in policies)
+
+
+def get_icopypolicy_for_packagecopypolicy(packagecopypolicy):
+ """Look up the `ICopyPolicy` for a given `PackageCopyPolicy`."""
+ return enum_to_policy[packagecopypolicy]
=== modified file 'lib/lp/soyuz/adapters/tests/test_copypolicy.py'
--- lib/lp/soyuz/adapters/tests/test_copypolicy.py 2011-05-30 11:47:46 +0000
+++ lib/lp/soyuz/adapters/tests/test_copypolicy.py 2011-06-02 08:48:54 +0000
@@ -1,16 +1,20 @@
# Copyright 2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from canonical.launchpad.webapp.testing import verifyObject
from canonical.testing.layers import ZopelessDatabaseLayer
-from lp.testing import TestCaseWithFactory
-
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.soyuz.adapters.copypolicy import (
InsecureCopyPolicy,
SyncCopyPolicy,
)
-from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.interfaces.copypolicy import ICopyPolicy
+from lp.soyuz.enums import (
+ ArchivePurpose,
+ PackageCopyPolicy,
+ )
+from lp.testing import TestCaseWithFactory
class TestCopyPolicy(TestCaseWithFactory):
@@ -30,7 +34,8 @@
self.assertTrue(approved)
def assertUnapproved(self, archive_purpose, method):
- archive, distroseries, pocket = self._getUploadCriteria(archive_purpose)
+ archive, distroseries, pocket = self._getUploadCriteria(
+ archive_purpose)
approved = method(archive, distroseries, pocket)
self.assertFalse(approved)
@@ -42,7 +47,7 @@
cp = InsecureCopyPolicy()
self.assertApproved(ArchivePurpose.PPA, cp.autoApproveNew)
- def test_insecure_approves_existing_distro_package_to_unfrozen_release(self):
+ def test_insecure_approves_known_distro_package_to_unfrozen_release(self):
archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
distroseries = self.factory.makeDistroSeries()
pocket = PackagePublishingPocket.RELEASE
@@ -80,8 +85,6 @@
cp = SyncCopyPolicy()
self.assertFalse(cp.send_email)
- def test_policy_names(self):
- icp = InsecureCopyPolicy()
- scp = SyncCopyPolicy()
- self.assertEquals("insecure", icp.name)
- self.assertEquals("sync", scp.name)
+ def test_policies_implement_ICopyPolicy(self):
+ for policy in PackageCopyPolicy.items:
+ self.assertTrue(verifyObject(ICopyPolicy, ICopyPolicy(policy)))
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2011-05-29 21:18:09 +0000
+++ lib/lp/soyuz/browser/archive.py 2011-06-02 08:48:54 +0000
@@ -1285,17 +1285,6 @@
dest_display_name)
-def name_pubs_with_versions(source_pubs):
- """Annotate each entry from `source_pubs` with its version.
-
- :param source_pubs: A sequence of `SourcePackagePublishingHistory`.
- :return: A list of tuples (name, version), one for each respective
- entry in `source_pubs`.
- """
- sprs = [spph.sourcepackagerelease for spph in source_pubs]
- return [(spr.sourcepackagename.name, spr.version) for spr in sprs]
-
-
def copy_asynchronously(source_pubs, dest_archive, dest_series, dest_pocket,
include_binaries, dest_url=None,
dest_display_name=None, person=None,
@@ -1317,8 +1306,9 @@
job_source = getUtility(IPlainPackageCopyJobSource)
for spph in source_pubs:
job_source.create(
- name_pubs_with_versions([spph]), spph.archive, dest_archive,
- dest_series, dest_pocket, include_binaries=include_binaries)
+ spph.source_package_name, spph.archive, dest_archive, dest_series,
+ dest_pocket, include_binaries=include_binaries,
+ package_version=spph.sourcepackagerelease.version)
return structured("""
<p>Requested sync of %s packages.</p>
=== modified file 'lib/lp/soyuz/browser/tests/test_package_copying_mixin.py'
--- lib/lp/soyuz/browser/tests/test_package_copying_mixin.py 2011-05-26 09:07:28 +0000
+++ lib/lp/soyuz/browser/tests/test_package_copying_mixin.py 2011-06-02 08:48:54 +0000
@@ -17,7 +17,6 @@
copy_asynchronously,
copy_synchronously,
FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
- name_pubs_with_versions,
PackageCopyingMixin,
render_cannotcopy_as_html,
)
@@ -103,15 +102,6 @@
packages = [self.getUniqueString() for counter in range(300)]
self.assertFalse(PackageCopyingMixin().canCopySynchronously(packages))
- def test_name_pubs_with_versions_lists_packages_and_versions(self):
- # name_pubs_with_versions returns a list of tuples of source
- # package name and source package version, one per SPPH.
- spph = FakeSPPH()
- spr = spph.sourcepackagerelease
- self.assertEqual(
- [(spr.sourcepackagename.name, spr.version)],
- name_pubs_with_versions([spph]))
-
def test_render_cannotcopy_as_html_lists_errors(self):
# render_cannotcopy_as_html includes a CannotCopy error message
# into its HTML notice.
@@ -257,10 +247,10 @@
jobs = list(getUtility(IPlainPackageCopyJobSource).getActiveJobs(
archive))
self.assertEqual(1, len(jobs))
+ job = jobs[0]
spr = spph.sourcepackagerelease
- self.assertEqual(
- [[spr.sourcepackagename.name, spr.version]],
- jobs[0].metadata['source_packages'])
+ self.assertEqual(spr.sourcepackagename.name, job.package_name)
+ self.assertEqual(spr.version, job.package_version)
def test_do_copy_goes_async_if_canCopySynchronously_says_so(self):
# The view opts for asynchronous copying if canCopySynchronously
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2011-05-29 21:18:09 +0000
+++ lib/lp/soyuz/configure.zcml 2011-06-02 08:48:54 +0000
@@ -919,6 +919,18 @@
<allow interface=".interfaces.packagecopyjob.IPlainPackageCopyJob" />
</class>
+ <!-- PackageCopyPolicy -->
+ <class class=".adapters.copypolicy.InsecureCopyPolicy">
+ <allow interface=".interfaces.copypolicy.ICopyPolicy"/>
+ </class>
+ <class class=".adapters.copypolicy.SyncCopyPolicy">
+ <allow interface=".interfaces.copypolicy.ICopyPolicy"/>
+ </class>
+ <adapter
+ for="lazr.enum.DBItem"
+ provides="lp.soyuz.interfaces.copypolicy.ICopyPolicy"
+ factory="lp.soyuz.adapters.copypolicy.get_icopypolicy_for_packagecopypolicy"/>
+
<!-- PackageCopyRequestSet -->
<securedutility
class="lp.soyuz.model.packagecopyrequest.PackageCopyRequestSet"
=== modified file 'lib/lp/soyuz/enums.py'
--- lib/lp/soyuz/enums.py 2011-03-30 04:10:29 +0000
+++ lib/lp/soyuz/enums.py 2011-06-02 08:48:54 +0000
@@ -13,6 +13,7 @@
'archive_suffixes',
'BinaryPackageFileType',
'BinaryPackageFormat',
+ 'PackageCopyPolicy',
'PackageCopyStatus',
'PackageDiffStatus',
'PackagePublishingPriority',
@@ -234,6 +235,25 @@
in Ubuntu and similar distributions.""")
+class PackageCopyPolicy(DBEnumeratedType):
+ """Package copying policy.
+
+ Each of these is associated with one `ICopyPolicy`.
+ """
+
+ INSECURE = DBItem(1, """
+ Copy from insecure source.
+
+ This is the default.
+ """)
+
+ SYNC = DBItem(2, """
+ Package sync.
+
+ This policy applies when synchronizing packages en masses.
+ """)
+
+
class PackageCopyStatus(DBEnumeratedType):
"""Package copy status type.
@@ -282,7 +302,6 @@
class PackageDiffStatus(DBEnumeratedType):
"""The status of a PackageDiff request."""
-
PENDING = DBItem(0, """
Pending
=== added file 'lib/lp/soyuz/interfaces/copypolicy.py'
--- lib/lp/soyuz/interfaces/copypolicy.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/interfaces/copypolicy.py 2011-06-02 08:48:54 +0000
@@ -0,0 +1,53 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Package copy policies."""
+
+__metaclass__ = type
+__all__ = [
+ 'ICopyPolicy',
+ ]
+
+
+from zope.interface import Interface
+from zope.schema import (
+ Bool,
+ Choice,
+ )
+
+from canonical.launchpad import _
+from lp.soyuz.enums import PackageCopyPolicy
+
+
+class ICopyPolicy(Interface):
+ """Policies for copying packages, as enumerated by `PackageCopyPolicy`."""
+
+ enum_value = Choice(
+ title=_("PackageCopyPolicy number associated with this policy."),
+ values=PackageCopyPolicy, readonly=True, required=True)
+
+ send_email = Bool(
+ title=_("Should completion of this copy be announced by email?"),
+ readonly=True, required=True)
+
+ def autoApprove(archive, distroseries, pocket):
+ """Can this upload of a known package be approved automatically?
+
+ This should only be called for packages that are known not new.
+
+ :param archive: The target `IArchive` for the upload.
+ :param distroseries: The target `IDistroSeries` for the upload.
+ :param pocket: The target `PackagePublishingPocket` for the upload.
+ :return: True if the upload can be auto-approved, or False if it
+ should be held in the queue.
+ """
+
+ def autoApproveNew(archive, distroseries, pocket):
+ """Can this upload of a new package be approved automatically?
+
+ :param archive: The target `IArchive` for the upload.
+ :param distroseries: The target `IDistroSeries` for the upload.
+ :param pocket: The target `PackagePublishingPocket` for the upload.
+ :return: True if the upload can be auto-approved, or False if it
+ should be held in the queue.
+ """
=== modified file 'lib/lp/soyuz/interfaces/packagecopyjob.py'
--- lib/lp/soyuz/interfaces/packagecopyjob.py 2011-05-19 09:53:13 +0000
+++ lib/lp/soyuz/interfaces/packagecopyjob.py 2011-06-02 08:48:54 +0000
@@ -22,8 +22,7 @@
from zope.schema import (
Bool,
Int,
- List,
- Tuple,
+ TextLine,
)
from canonical.launchpad import _
@@ -63,6 +62,9 @@
schema=IDistroSeries, title=_('Target DistroSeries.'),
required=True, readonly=True)
+ package_name = TextLine(
+ title=_("Package name"), required=True, readonly=True)
+
job = Reference(
schema=IJob, title=_('The common Job attributes'),
required=True, readonly=True)
@@ -82,14 +84,13 @@
class IPlainPackageCopyJobSource(IJobSource):
"""An interface for acquiring `IPackageCopyJobs`."""
- def create(cls, source_packages, source_archive,
+ def create(cls, package_name, source_archive,
target_archive, target_distroseries, target_pocket,
- include_binaries=False):
+ include_binaries=False, package_version=None,
+ copy_policy=None):
"""Create a new `IPackageCopyJob`.
- :param source_packages: An iterable of `(source_package_name,
- version)` tuples, where both `source_package_name` and `version`
- are strings.
+ :param package_name: The name of the source package to copy.
:param source_archive: The `IArchive` in which `source_packages` are
found.
:param target_archive: The `IArchive` to which to copy the packages.
@@ -98,6 +99,9 @@
:param target_pocket: The pocket into which to copy the packages. Must
be a member of `PackagePublishingPocket`.
:param include_binaries: See `do_copy`.
+ :param package_version: The version string for the package version
+ that is to be copied.
+ :param copy_policy: Applicable `PackageCopyPolicy`.
"""
def getActiveJobs(target_archive):
@@ -107,7 +111,7 @@
"""Find pending jobs for each package in `target_series`.
This is meant for finding jobs that will resolve specific
- `DistroSeriesDifference`s, so see also `specify_dsd_package`.
+ `DistroSeriesDifference`s.
:param target_series: Target `DistroSeries`; this corresponds to
`DistroSeriesDifference.derived_series`.
@@ -122,15 +126,13 @@
class IPlainPackageCopyJob(IRunnableJob):
"""A no-frills job to copy packages between `IArchive`s."""
- source_packages = List(
- title=_("Source Packages"),
- value_type=Tuple(min_length=3, max_length=3),
- required=True, readonly=True)
-
target_pocket = Int(
title=_("Target package publishing pocket"), required=True,
readonly=True)
+ package_version = TextLine(
+ title=_("Package version"), required=True, readonly=True)
+
include_binaries = Bool(
title=_("Copy binaries"),
required=False, readonly=True)
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2011-05-31 13:31:05 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2011-06-02 08:48:54 +0000
@@ -6,7 +6,6 @@
__all__ = [
"PackageCopyJob",
"PlainPackageCopyJob",
- "specify_dsd_package",
]
from lazr.delegates import delegates
@@ -38,7 +37,6 @@
IDistroSeriesDifferenceSource,
)
from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.registry.model.distroseries import DistroSeries
from lp.registry.interfaces.distroseriesdifferencecomment import (
IDistroSeriesDifferenceCommentSource,
@@ -47,6 +45,7 @@
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
from lp.services.job.runner import BaseRunnableJob
+from lp.soyuz.enums import PackageCopyPolicy
from lp.soyuz.interfaces.archive import CannotCopy
from lp.soyuz.interfaces.packagecopyjob import (
IPackageCopyJob,
@@ -58,17 +57,6 @@
from lp.soyuz.scripts.packagecopier import do_copy
-def specify_dsd_package(dsd):
- """Return (name, parent version) for `dsd`'s package.
-
- This describes the package that `dsd` is for in a format suitable for
- `PlainPackageCopyJobSource`.
-
- :param dsd: A `DistroSeriesDifference`.
- """
- return (dsd.source_package_name.name, dsd.parent_source_version)
-
-
class PackageCopyJob(StormBase):
"""Base class for package copying jobs."""
@@ -90,18 +78,23 @@
target_distroseries_id = Int(name='target_distroseries')
target_distroseries = Reference(target_distroseries_id, DistroSeries.id)
+ package_name = Unicode('package_name')
+ copy_policy = EnumCol(enum=PackageCopyPolicy)
+
job_type = EnumCol(enum=PackageCopyJobType, notNull=True)
_json_data = Unicode('json_data')
def __init__(self, source_archive, target_archive, target_distroseries,
- job_type, metadata):
+ job_type, metadata, package_name=None, copy_policy=None):
super(PackageCopyJob, self).__init__()
self.job = Job()
+ self.job_type = job_type
self.source_archive = source_archive
self.target_archive = target_archive
self.target_distroseries = target_distroseries
- self.job_type = job_type
+ self.package_name = unicode(package_name)
+ self.copy_policy = copy_policy
self._json_data = self.serializeMetadata(metadata)
@classmethod
@@ -168,7 +161,7 @@
class PlainPackageCopyJob(PackageCopyJobDerived):
- """Job that copies packages between archives."""
+ """Job that copies a package from one archive to another."""
# This job type serves in different places: it supports copying
# packages between archives, but also the syncing of packages from
# parents into a derived distroseries. We may split these into
@@ -181,20 +174,24 @@
classProvides(IPlainPackageCopyJobSource)
@classmethod
- def create(cls, source_packages, source_archive,
+ def create(cls, package_name, source_archive,
target_archive, target_distroseries, target_pocket,
- include_binaries=False):
+ include_binaries=False, package_version=None,
+ copy_policy=None):
"""See `IPlainPackageCopyJobSource`."""
+ assert package_version is not None, "No package version specified."
metadata = {
- 'source_packages': source_packages,
'target_pocket': target_pocket.value,
+ 'package_version': package_version,
'include_binaries': bool(include_binaries),
}
job = PackageCopyJob(
+ job_type=cls.class_job_type,
source_archive=source_archive,
target_archive=target_archive,
target_distroseries=target_distroseries,
- job_type=cls.class_job_type,
+ package_name=package_name,
+ copy_policy=copy_policy,
metadata=metadata)
IMasterStore(PackageCopyJob).add(job)
return cls(job)
@@ -232,22 +229,18 @@
# getPendingJobsForTargetSeries orders its results, the first
# will be the oldest and thus presumably the first to finish.
for job in cls.getPendingJobsForTargetSeries(target_series):
- for package in job.metadata["source_packages"]:
- result.setdefault(tuple(package), job)
+ result.setdefault(job.package_name, job)
return result
@property
- def source_packages(self):
- getPublishedSources = self.source_archive.getPublishedSources
- for name, version in self.metadata['source_packages']:
- yield name, version, getPublishedSources(
- name=name, version=version, exact_match=True).first()
-
- @property
def target_pocket(self):
return PackagePublishingPocket.items[self.metadata['target_pocket']]
@property
+ def package_version(self):
+ return self.metadata["package_version"]
+
+ @property
def include_binaries(self):
return self.metadata['include_binaries']
@@ -270,16 +263,15 @@
raise CannotCopy(
"Destination pocket must be 'release' for a PPA.")
- source_packages = set()
- for name, version, source_package in self.source_packages:
- if source_package is None:
- raise CannotCopy(
- "Package %r %r not found." % (name, version))
- else:
- source_packages.add(source_package)
+ name = self.package_name
+ version = self.package_version
+ source_package = self.source_archive.getPublishedSources(
+ name=name, version=version, exact_match=True).first()
+ if source_package is None:
+ raise CannotCopy("Package %r %r not found." % (name, version))
do_copy(
- sources=source_packages, archive=self.target_archive,
+ sources=[source_package], archive=self.target_archive,
series=self.target_distroseries, pocket=self.target_pocket,
include_binaries=self.include_binaries, check_permissions=False)
@@ -292,20 +284,18 @@
dsd_source = getUtility(IDistroSeriesDifferenceSource)
target_series = self.target_distroseries
candidates = dsd_source.getForDistroSeries(
- distro_series=target_series)
+ distro_series=target_series,
+ source_package_name_filter=self.package_name)
+
# The job doesn't know what distroseries a given package is
# coming from, and the version number in the DSD may have
# changed. We can however filter out DSDs that are from
# different distributions, based on the job's target archive.
source_distro_id = self.source_archive.distributionID
- package_ids = set(
- getUtility(ISourcePackageNameSet).queryByName(name).id
- for name, version in self.metadata["source_packages"])
return [
dsd
for dsd in candidates
- if dsd.parent_series.distributionID == source_distro_id and
- dsd.source_package_name_id in package_ids]
+ if dsd.parent_series.distributionID == source_distro_id]
def reportFailure(self, cannotcopy_exception):
"""Attempt to report failure to the user."""
@@ -326,11 +316,10 @@
def __repr__(self):
"""Returns an informative representation of the job."""
parts = ["%s to copy" % self.__class__.__name__]
- source_packages = self.metadata["source_packages"]
- if len(source_packages) == 0:
- parts.append(" no packages (!)")
+ if self.package_name is None:
+ parts.append(" no package (!)")
else:
- parts.append(" %d package(s)" % len(source_packages))
+ parts.append(" package %s" % self.package_name)
parts.append(
" from %s/%s" % (
self.source_archive.distribution.name,
=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py 2011-05-30 07:56:15 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py 2011-06-02 08:48:54 +0000
@@ -33,7 +33,6 @@
from lp.soyuz.interfaces.sourcepackageformat import (
ISourcePackageFormatSelectionSet,
)
-from lp.soyuz.model.packagecopyjob import specify_dsd_package
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
run_script,
@@ -56,14 +55,14 @@
"""Create a `PlainPackageCopyJob` that would resolve `dsd`."""
if dsd is None:
dsd = self.factory.makeDistroSeriesDifference()
- source_packages = [specify_dsd_package(dsd)]
source_archive = dsd.parent_series.main_archive
target_archive = dsd.derived_series.main_archive
target_distroseries = dsd.derived_series
target_pocket = self.factory.getAnyPocket()
return getUtility(IPlainPackageCopyJobSource).create(
- source_packages, source_archive, target_archive,
- target_distroseries, target_pocket)
+ dsd.source_package_name.name, source_archive, target_archive,
+ target_distroseries, target_pocket,
+ package_version=dsd.parent_source_version)
def runJob(self, job):
"""Helper to switch to the right DB user and run the job."""
@@ -84,11 +83,10 @@
archive2 = self.factory.makeArchive(distroseries.distribution)
source = getUtility(IPlainPackageCopyJobSource)
job = source.create(
- source_packages=[("foo", "1.0-1"), ("bar", "2.4")],
- source_archive=archive1, target_archive=archive2,
- target_distroseries=distroseries,
+ package_name="foo", source_archive=archive1,
+ target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=False)
+ package_version="1.0-1", include_binaries=False)
self.assertProvides(job, IPackageCopyJob)
self.assertEquals(archive1.id, job.source_archive_id)
self.assertEquals(archive1, job.source_archive)
@@ -96,9 +94,8 @@
self.assertEquals(archive2, job.target_archive)
self.assertEquals(distroseries, job.target_distroseries)
self.assertEquals(PackagePublishingPocket.RELEASE, job.target_pocket)
- self.assertContentEqual(
- job.source_packages,
- [("foo", "1.0-1", None), ("bar", "2.4", None)])
+ self.assertEqual("foo", job.package_name)
+ self.assertEqual("1.0-1", job.package_version)
self.assertEquals(False, job.include_binaries)
def test_getActiveJobs(self):
@@ -108,10 +105,10 @@
archive2 = self.factory.makeArchive(distroseries.distribution)
source = getUtility(IPlainPackageCopyJobSource)
job = source.create(
- source_packages=[("foo", "1.0-1")], source_archive=archive1,
+ package_name="foo", source_archive=archive1,
target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=False)
+ package_version="1.0-1", include_binaries=False)
self.assertContentEqual([job], source.getActiveJobs(archive2))
def test_getActiveJobs_gets_oldest_first(self):
@@ -167,10 +164,10 @@
archive2 = self.factory.makeArchive(distroseries.distribution)
job_source = getUtility(IPlainPackageCopyJobSource)
job = job_source.create(
- source_packages=[("foo", "1.0-1")], source_archive=archive1,
+ package_name="foo", source_archive=archive1,
target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=False)
+ package_version="1.0-1", include_binaries=False)
naked_job = removeSecurityProxy(job)
naked_job.reportFailure = FakeMethod()
@@ -181,14 +178,15 @@
def test_target_ppa_non_release_pocket(self):
# When copying to a PPA archive the target must be the release pocket.
distroseries = self.factory.makeDistroSeries()
+ package = self.factory.makeSourcePackageName()
archive1 = self.factory.makeArchive(distroseries.distribution)
archive2 = self.factory.makeArchive(distroseries.distribution)
source = getUtility(IPlainPackageCopyJobSource)
job = source.create(
- source_packages=[], source_archive=archive1,
+ package_name=package.name, source_archive=archive1,
target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.UPDATES,
- include_binaries=False)
+ include_binaries=False, package_version='1.0')
naked_job = removeSecurityProxy(job)
naked_job.reportFailure = FakeMethod()
@@ -218,20 +216,20 @@
getUtility(ISourcePackageFormatSelectionSet).add(
target_series, SourcePackageFormat.FORMAT_1_0)
- source_package = publisher.getPubSource(
+ publisher.getPubSource(
distroseries=distroseries, sourcename="libc",
version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
archive=breezy_archive)
source = getUtility(IPlainPackageCopyJobSource)
job = source.create(
- source_packages=[("libc", "2.8-1")],
+ package_name="libc",
source_archive=breezy_archive, target_archive=target_archive,
target_distroseries=target_series,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=False)
- self.assertContentEqual(
- job.source_packages, [("libc", "2.8-1", source_package)])
+ package_version="2.8-1", include_binaries=False)
+ self.assertEqual("libc", job.package_name)
+ self.assertEqual("2.8-1", job.package_version)
# Make sure everything hits the database, switching db users
# aborts.
@@ -256,10 +254,10 @@
archive2 = self.factory.makeArchive(distroseries.distribution)
source = getUtility(IPlainPackageCopyJobSource)
job = source.create(
- source_packages=[("foo", "1.0-1")], source_archive=archive1,
+ package_name="foo", source_archive=archive1,
target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=False)
+ package_version="1.0-1", include_binaries=False)
oops_vars = job.getOopsVars()
naked_job = removeSecurityProxy(job)
self.assertIn(
@@ -285,10 +283,10 @@
version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
archive=archive1)
getUtility(IPlainPackageCopyJobSource).create(
- source_packages=[("libc", "2.8-1")], source_archive=archive1,
+ package_name="libc", source_archive=archive1,
target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=False)
+ package_version="2.8-1", include_binaries=False)
transaction.commit()
out, err, exit_code = run_script(
@@ -309,13 +307,12 @@
archive2 = self.factory.makeArchive(distroseries.distribution)
source = getUtility(IPlainPackageCopyJobSource)
job = source.create(
- source_packages=[("foo", "1.0-1"), ("bar", "2.4")],
- source_archive=archive1, target_archive=archive2,
- target_distroseries=distroseries,
+ package_name="foo", source_archive=archive1,
+ target_archive=archive2, target_distroseries=distroseries,
target_pocket=PackagePublishingPocket.RELEASE,
- include_binaries=True)
+ package_version="1.0-1", include_binaries=True)
self.assertEqual(
- ("<PlainPackageCopyJob to copy 2 package(s) from "
+ ("<PlainPackageCopyJob to copy package foo from "
"{distroseries.distribution.name}/{archive1.name} to "
"{distroseries.distribution.name}/{archive2.name}, "
"RELEASE pocket, in {distroseries.distribution.name} "
@@ -331,7 +328,7 @@
job = self.makeJob(dsd)
job_source = getUtility(IPlainPackageCopyJobSource)
self.assertEqual(
- {specify_dsd_package(dsd): job},
+ {dsd.source_package_name.name: job},
job_source.getPendingJobsPerPackage(dsd.derived_series))
def test_getPendingJobsPerPackage_ignores_other_distroseries(self):
@@ -347,7 +344,6 @@
# getPendingJobsPerPackage ignores jobs that have already been
# run.
dsd = self.factory.makeDistroSeriesDifference()
- package = specify_dsd_package(dsd)
job = self.makeJob(dsd)
job_source = getUtility(IPlainPackageCopyJobSource)
found_by_state = {}
@@ -355,7 +351,7 @@
removeSecurityProxy(job).job._status = status
result = job_source.getPendingJobsPerPackage(dsd.derived_series)
if len(result) > 0:
- found_by_state[status] = result[package]
+ found_by_state[status] = result[dsd.source_package_name.name]
expected = {
JobStatus.WAITING: job,
JobStatus.RUNNING: job,
@@ -374,7 +370,7 @@
jobs = map(self.makeJob, dsds)
job_source = getUtility(IPlainPackageCopyJobSource)
self.assertEqual(
- dict(zip(map(specify_dsd_package, dsds), jobs)),
+ dict(zip([dsd.source_package_name.name for dsd in dsds], jobs)),
job_source.getPendingJobsPerPackage(derived_series))
def test_getPendingJobsPerPackage_picks_oldest_job_for_dsd(self):
@@ -384,7 +380,7 @@
jobs = [self.makeJob(dsd) for counter in xrange(2)]
job_source = getUtility(IPlainPackageCopyJobSource)
self.assertEqual(
- {specify_dsd_package(dsd): jobs[0]},
+ {dsd.source_package_name.name: jobs[0]},
job_source.getPendingJobsPerPackage(dsd.derived_series))
def test_getPendingJobsPerPackage_ignores_dsds_without_jobs(self):
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-05-29 21:18:09 +0000
+++ lib/lp/testing/factory.py 2011-06-02 08:48:54 +0000
@@ -4084,7 +4084,6 @@
if package_name is None and package_version is None:
package_name = self.makeSourcePackageName().name
package_version = unicode(self.getUniqueInteger()) + 'version'
- package_tuple = (package_name, package_version)
if source_archive is None:
source_archive = self.makeArchive()
if target_archive is None:
@@ -4094,8 +4093,9 @@
if target_pocket is None:
target_pocket = self.getAnyPocket()
return getUtility(IPlainPackageCopyJobSource).create(
- package_tuple, source_archive, target_archive,
- target_distroseries, target_pocket)
+ package_name, source_archive, target_archive,
+ target_distroseries, target_pocket,
+ package_version=package_version)
# Some factory methods return simple Python types. We don't add
