Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-bug-790084 into
lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #790084 in Launchpad itself: "PlainPackageCopyJob.findMatchingDSDs fails
to filter by package"
https://bugs.launchpad.net/launchpad/+bug/790084
For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-790084/+merge/62845
= Summary =
When an asynchronous package copy requested from a DistroSeriesDifference fails
with a CannotCopy exception, it leaves a note in the form of a
DistroSeriesDifferenceComment. But it's currently doing that on all DSDs for
the same distroseries.
== Proposed fix ==
I neglected to filter the DSDs by source package name. Easy fix.
== Pre-implementation notes ==
Failed to have one. I'm bad.
== Implementation details ==
I had to add IDistroSeriesDifference.source_package_name_id, in order to give
the model code access to the attribute on objects as returned by the
ISourcePackageNameSet utility. Matching on id is the right thing to do here
because checking source_package_name attributes will cause individual
SourcePackageName attributes to be fetched for lots of irrelevant DSDs.
== Tests ==
{{{
./bin/test -vvc lp.soyuz.tests.test_packagecopyjob -t findMatchingDSDs
}}}
== Demo and Q/A ==
Request an asynchronous sync that will fail with a CannotCopy exception.
Presently amsn is a usable example. Make sure that
cronscripts/process-job-source-groups.py is run:
{{{
./cronscripts/process-job-source-groups.py MAIN -vvv -e
IMembershipNotificationJobSource -e IPersonMergeJobSource -e
IQuestionEmailJobSource
}}}
The failure should leave a note on its DSD entry on +localpackagediffs, but not
on the entries for other packages.
= Launchpad lint =
Oops, I missed a spot which I shall fix forthwith:
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/model/packagecopyjob.py
lib/lp/registry/interfaces/distroseriesdifference.py
lib/lp/soyuz/tests/test_packagecopyjob.py
./lib/lp/soyuz/tests/test_packagecopyjob.py
423: local variable 'other_dsd' is assigned to but never used
--
https://code.launchpad.net/~jtv/launchpad/db-bug-790084/+merge/62845
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/launchpad/db-bug-790084 into lp:launchpad/db-devel.
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2011-05-26 09:07:28 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2011-05-30 07:56:02 +0000
@@ -68,6 +68,8 @@
"The distribution series which identifies the parent series "
"with the difference.")))
+ source_package_name_id = Int(
+ title=u"Source package name id", required=True, readonly=True)
source_package_name = Reference(
ISourcePackageName,
title=_("Source package name"), required=True, readonly=True,
@@ -234,6 +236,7 @@
cannot be requested.
"""
+
class IDistroSeriesDifferenceAdmin(Interface):
"""Difference attributes requiring launchpad.Admin."""
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2011-05-29 21:18:09 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2011-05-30 07:56:02 +0000
@@ -38,6 +38,7 @@
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,
@@ -291,10 +292,14 @@
# 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]
+ if dsd.parent_series.distributionID == source_distro_id and
+ dsd.source_package_name_id in package_ids]
def reportFailure(self, cannotcopy_exception):
"""Attempt to report failure to the user."""
=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py 2011-05-27 02:48:33 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py 2011-05-30 07:56:02 +0000
@@ -418,6 +418,14 @@
self.assertContentEqual([], naked_job.findMatchingDSDs())
+ def test_findMatchingDSDs_ignores_other_packages(self):
+ dsd = self.factory.makeDistroSeriesDifference()
+ other_dsd = self.factory.makeDistroSeriesDifference(
+ derived_series=dsd.derived_series,
+ parent_series=dsd.parent_series)
+ naked_job = removeSecurityProxy(self.makeJob(dsd))
+ self.assertContentEqual([dsd], naked_job.findMatchingDSDs())
+
class TestPlainPackageCopyJobPrivileges(TestCaseWithFactory, LocalTestHelper):
"""Test that `PlainPackageCopyJob` has the privileges it needs.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
