Merge lp:~jtv/launchpad/db-bug-790084 into lp:launchpad/db-devel

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 10622
Proposed branch: lp:~jtv/launchpad/db-bug-790084
Merge into: lp:launchpad/db-devel
Diff against target: 69 lines (+19/-1)
3 files modified
lib/lp/registry/interfaces/distroseriesdifference.py (+3/-0)
lib/lp/soyuz/model/packagecopyjob.py (+6/-1)
lib/lp/soyuz/tests/test_packagecopyjob.py (+10/-0)
To merge this branch: bzr merge lp:~jtv/launchpad/db-bug-790084
Reviewer Review Type Date Requested Status
Henning Eggers (community) Approve
Review via email: mp+62845@code.launchpad.net

Commit message

[r=henninge][bug=790084] Attach package sync error only to the right DSD.

Description of the change

= 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

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

Thanks. ;-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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:58:10 +0000
@@ -68,6 +68,8 @@
68 "The distribution series which identifies the parent series "68 "The distribution series which identifies the parent series "
69 "with the difference.")))69 "with the difference.")))
7070
71 source_package_name_id = Int(
72 title=u"Source package name id", required=True, readonly=True)
71 source_package_name = Reference(73 source_package_name = Reference(
72 ISourcePackageName,74 ISourcePackageName,
73 title=_("Source package name"), required=True, readonly=True,75 title=_("Source package name"), required=True, readonly=True,
@@ -234,6 +236,7 @@
234 cannot be requested.236 cannot be requested.
235 """237 """
236238
239
237class IDistroSeriesDifferenceAdmin(Interface):240class IDistroSeriesDifferenceAdmin(Interface):
238 """Difference attributes requiring launchpad.Admin."""241 """Difference attributes requiring launchpad.Admin."""
239242
240243
=== 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:58:10 +0000
@@ -38,6 +38,7 @@
38 IDistroSeriesDifferenceSource,38 IDistroSeriesDifferenceSource,
39 )39 )
40from lp.registry.interfaces.pocket import PackagePublishingPocket40from lp.registry.interfaces.pocket import PackagePublishingPocket
41from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
41from lp.registry.model.distroseries import DistroSeries42from lp.registry.model.distroseries import DistroSeries
42from lp.registry.interfaces.distroseriesdifferencecomment import (43from lp.registry.interfaces.distroseriesdifferencecomment import (
43 IDistroSeriesDifferenceCommentSource,44 IDistroSeriesDifferenceCommentSource,
@@ -291,10 +292,14 @@
291 # changed. We can however filter out DSDs that are from292 # changed. We can however filter out DSDs that are from
292 # different distributions, based on the job's target archive.293 # different distributions, based on the job's target archive.
293 source_distro_id = self.source_archive.distributionID294 source_distro_id = self.source_archive.distributionID
295 package_ids = set(
296 getUtility(ISourcePackageNameSet).queryByName(name).id
297 for name, version in self.metadata["source_packages"])
294 return [298 return [
295 dsd299 dsd
296 for dsd in candidates300 for dsd in candidates
297 if dsd.parent_series.distributionID == source_distro_id]301 if dsd.parent_series.distributionID == source_distro_id and
302 dsd.source_package_name_id in package_ids]
298303
299 def reportFailure(self, cannotcopy_exception):304 def reportFailure(self, cannotcopy_exception):
300 """Attempt to report failure to the user."""305 """Attempt to report failure to the user."""
301306
=== 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:58:10 +0000
@@ -418,6 +418,16 @@
418418
419 self.assertContentEqual([], naked_job.findMatchingDSDs())419 self.assertContentEqual([], naked_job.findMatchingDSDs())
420420
421 def test_findMatchingDSDs_ignores_other_packages(self):
422 # findMatchingDSDs does not return DSDs that are similar to the
423 # information in the job, but are for different packages.
424 dsd = self.factory.makeDistroSeriesDifference()
425 self.factory.makeDistroSeriesDifference(
426 derived_series=dsd.derived_series,
427 parent_series=dsd.parent_series)
428 naked_job = removeSecurityProxy(self.makeJob(dsd))
429 self.assertContentEqual([dsd], naked_job.findMatchingDSDs())
430
421431
422class TestPlainPackageCopyJobPrivileges(TestCaseWithFactory, LocalTestHelper):432class TestPlainPackageCopyJobPrivileges(TestCaseWithFactory, LocalTestHelper):
423 """Test that `PlainPackageCopyJob` has the privileges it needs.433 """Test that `PlainPackageCopyJob` has the privileges it needs.

Subscribers

People subscribed via source and target branches

to status/vote changes: