Merge lp:~abentley/launchpad/celery-everywhere-8 into lp:launchpad

Proposed by Aaron Bentley
Status: Merged
Merged at revision: 15155
Proposed branch: lp:~abentley/launchpad/celery-everywhere-8
Merge into: lp:launchpad
Prerequisite: lp:~abentley/launchpad/celery-everywhere-7
Diff against target: 257 lines (+113/-15)
4 files modified
lib/lp/registry/model/persontransferjob.py (+15/-2)
lib/lp/registry/tests/test_membership_notification_job.py (+44/-1)
lib/lp/registry/tests/test_person_merge_job.py (+52/-11)
lib/lp/services/job/model/job.py (+2/-1)
To merge this branch: bzr merge lp:~abentley/launchpad/celery-everywhere-8
Reviewer Review Type Date Requested Status
Deryck Hodge (community) Approve
Review via email: mp+103545@code.launchpad.net

Commit message

Support running PersonTransferJobs via Celery.

Description of the change

= Summary =
Support running PersonTransferJob via Celery.

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of a resourced arc that will ultimately remove code.

== Implementation details ==
PersonTransferJob now provides makeDerived.
PersonTransferJobDerived now uses the EnumeratedSubclass metatype, so it can turn a PersonTransferJob into the appropriate subclass.
PersonTransferJobDerived.create now requests the job to run via Celery.
UniversalJobSource now supports PersonTransferJob.
PersonMergeJob and MembershipNotificationJob both provide configs.

== Tests ==
bin/test --layer=CeleryJobLayer -m 'membership_notification|person_merge'

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/tests/test_pofilestatsjob.py
  lib/lp/registry/tests/test_membership_notification_job.py
  lib/lp/translations/model/pofilestatsjob.py
  lib/lp/registry/model/persontransferjob.py
  lib/lp/registry/tests/test_person_merge_job.py
  lib/lp/services/job/model/job.py

To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/model/persontransferjob.py'
--- lib/lp/registry/model/persontransferjob.py 2012-01-27 11:12:53 +0000
+++ lib/lp/registry/model/persontransferjob.py 2012-04-26 16:21:20 +0000
@@ -52,7 +52,10 @@
52 IStore,52 IStore,
53 )53 )
54from lp.services.database.stormbase import StormBase54from lp.services.database.stormbase import StormBase
55from lp.services.job.model.job import Job55from lp.services.job.model.job import (
56 EnumeratedSubclass,
57 Job,
58 )
56from lp.services.job.runner import BaseRunnableJob59from lp.services.job.runner import BaseRunnableJob
57from lp.services.mail.helpers import (60from lp.services.mail.helpers import (
58 get_contact_email_addresses,61 get_contact_email_addresses,
@@ -115,6 +118,9 @@
115 # but the DB representation is unicode.118 # but the DB representation is unicode.
116 self._json_data = json_data.decode('utf-8')119 self._json_data = json_data.decode('utf-8')
117120
121 def makeDerived(self):
122 return PersonTransferJobDerived.makeSubclass(self)
123
118124
119class PersonTransferJobDerived(BaseRunnableJob):125class PersonTransferJobDerived(BaseRunnableJob):
120 """Intermediate class for deriving from PersonTransferJob.126 """Intermediate class for deriving from PersonTransferJob.
@@ -126,6 +132,7 @@
126 the run() method.132 the run() method.
127 """133 """
128134
135 __metaclass__ = EnumeratedSubclass
129 delegates(IPersonTransferJob)136 delegates(IPersonTransferJob)
130 classProvides(IPersonTransferJobSource)137 classProvides(IPersonTransferJobSource)
131138
@@ -146,7 +153,9 @@
146 major_person=major_person,153 major_person=major_person,
147 job_type=cls.class_job_type,154 job_type=cls.class_job_type,
148 metadata=metadata)155 metadata=metadata)
149 return cls(job)156 derived = cls(job)
157 derived.celeryRunOnCommit()
158 return derived
150159
151 @classmethod160 @classmethod
152 def iterReady(cls):161 def iterReady(cls):
@@ -176,6 +185,8 @@
176185
177 class_job_type = PersonTransferJobType.MEMBERSHIP_NOTIFICATION186 class_job_type = PersonTransferJobType.MEMBERSHIP_NOTIFICATION
178187
188 config = config.IMembershipNotificationJobSource
189
179 @classmethod190 @classmethod
180 def create(cls, member, team, reviewer, old_status, new_status,191 def create(cls, member, team, reviewer, old_status, new_status,
181 last_change_comment=None):192 last_change_comment=None):
@@ -342,6 +353,8 @@
342353
343 class_job_type = PersonTransferJobType.MERGE354 class_job_type = PersonTransferJobType.MERGE
344355
356 config = config.IPersonMergeJobSource
357
345 @classmethod358 @classmethod
346 def create(cls, from_person, to_person, reviewer=None, delete=False):359 def create(cls, from_person, to_person, reviewer=None, delete=False):
347 """See `IPersonMergeJobSource`."""360 """See `IPersonMergeJobSource`."""
348361
=== modified file 'lib/lp/registry/tests/test_membership_notification_job.py'
--- lib/lp/registry/tests/test_membership_notification_job.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_membership_notification_job.py 2012-04-26 16:21:20 +0000
@@ -19,14 +19,22 @@
19 TeamMembershipStatus,19 TeamMembershipStatus,
20 )20 )
21from lp.registry.model.persontransferjob import MembershipNotificationJob21from lp.registry.model.persontransferjob import MembershipNotificationJob
22from lp.services.features.testing import FeatureFixture
22from lp.services.job.interfaces.job import JobStatus23from lp.services.job.interfaces.job import JobStatus
24from lp.services.job.tests import (
25 block_on_job,
26 pop_remote_notifications,
27 )
23from lp.testing import (28from lp.testing import (
24 login_person,29 login_person,
25 person_logged_in,30 person_logged_in,
26 run_script,31 run_script,
27 TestCaseWithFactory,32 TestCaseWithFactory,
28 )33 )
29from lp.testing.layers import DatabaseFunctionalLayer34from lp.testing.layers import (
35 CeleryJobLayer,
36 DatabaseFunctionalLayer,
37 )
30from lp.testing.sampledata import ADMIN_EMAIL38from lp.testing.sampledata import ADMIN_EMAIL
3139
3240
@@ -110,3 +118,38 @@
110 self.assertEqual(0, exit_code)118 self.assertEqual(0, exit_code)
111 self.assertTrue(job_repr in err, err)119 self.assertTrue(job_repr in err, err)
112 self.assertTrue("MembershipNotificationJob sent email" in err, err)120 self.assertTrue("MembershipNotificationJob sent email" in err, err)
121
122
123class TestViaCelery(TestCaseWithFactory):
124
125 layer = CeleryJobLayer
126
127 def test_smoke_admining_team(self):
128 # Smoke test, primarily for DB permissions needed by queries to work
129 # with admining users and teams
130 # Check the oopses in /var/tmp/lperr.test if the assertions fail.
131 self.useFixture(FeatureFixture({
132 'jobs.celery.enabled_classes': 'MembershipNotificationJob'
133 }))
134 team = self.factory.makeTeam(name='a-team')
135 with person_logged_in(team.teamowner):
136 # This implicitly creates a job, but it is not the job under test.
137 admining_team = self.factory.makeTeam()
138 team.addMember(
139 admining_team, team.teamowner, force_team_add=True)
140 membership = getUtility(ITeamMembershipSet).getByPersonAndTeam(
141 admining_team, team)
142 membership.setStatus(
143 TeamMembershipStatus.ADMIN, team.teamowner)
144 person = self.factory.makePerson(name='murdock')
145 with block_on_job(self):
146 transaction.commit()
147 pop_remote_notifications()
148 job = getUtility(IMembershipNotificationJobSource).create(
149 person, team, team.teamowner,
150 TeamMembershipStatus.APPROVED, TeamMembershipStatus.ADMIN)
151 with block_on_job(self):
152 transaction.commit()
153 self.assertEqual(JobStatus.COMPLETED, job.status)
154 (notification,) = pop_remote_notifications()
155 self.assertIn('murdock made admin by', notification['Subject'])
113156
=== modified file 'lib/lp/registry/tests/test_person_merge_job.py'
--- lib/lp/registry/tests/test_person_merge_job.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_person_merge_job.py 2012-04-26 16:21:20 +0000
@@ -1,4 +1,4 @@
1# Copyright 2011 Canonical Ltd. This software is licensed under the1# Copyright 2012 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Tests of `PersonMergeJob`."""4"""Tests of `PersonMergeJob`."""
@@ -20,9 +20,11 @@
20 IMasterObject,20 IMasterObject,
21 IStore,21 IStore,
22 )22 )
23from lp.services.features.testing import FeatureFixture
23from lp.services.identity.interfaces.emailaddress import EmailAddressStatus24from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
24from lp.services.job.interfaces.job import JobStatus25from lp.services.job.interfaces.job import JobStatus
25from lp.services.job.model.job import Job26from lp.services.job.model.job import Job
27from lp.services.job.tests import block_on_job
26from lp.services.log.logger import BufferLogger28from lp.services.log.logger import BufferLogger
27from lp.services.mail.sendmail import format_address_for_person29from lp.services.mail.sendmail import format_address_for_person
28from lp.services.scripts import log30from lp.services.scripts import log
@@ -31,7 +33,33 @@
31 run_script,33 run_script,
32 TestCaseWithFactory,34 TestCaseWithFactory,
33 )35 )
34from lp.testing.layers import DatabaseFunctionalLayer36from lp.testing.layers import (
37 CeleryJobLayer,
38 DatabaseFunctionalLayer,
39 )
40
41
42def create_job(factory):
43 """Create a PersonMergeJob for testing purposes.
44
45 :param factory: A LaunchpadObjectFactory.
46 """
47 from_person = factory.makePerson(name='void')
48 to_person = factory.makePerson(name='gestalt')
49 return getUtility(IPersonMergeJobSource).create(
50 from_person=from_person, to_person=to_person)
51
52
53def transfer_email(job):
54 """Reassign email address using the people specified in the job.
55
56 IPersonSet.merge() does not (yet) promise to do this.
57 """
58 from_email = IMasterObject(job.from_person.preferredemail)
59 removeSecurityProxy(from_email).personID = job.to_person.id
60 removeSecurityProxy(from_email).accountID = job.to_person.accountID
61 removeSecurityProxy(from_email).status = EmailAddressStatus.NEW
62 IStore(from_email).flush()
3563
3664
37class TestPersonMergeJob(TestCaseWithFactory):65class TestPersonMergeJob(TestCaseWithFactory):
@@ -40,11 +68,10 @@
4068
41 def setUp(self):69 def setUp(self):
42 super(TestPersonMergeJob, self).setUp()70 super(TestPersonMergeJob, self).setUp()
43 self.from_person = self.factory.makePerson(name='void')
44 self.to_person = self.factory.makePerson(name='gestalt')
45 self.job_source = getUtility(IPersonMergeJobSource)71 self.job_source = getUtility(IPersonMergeJobSource)
46 self.job = self.job_source.create(72 self.job = create_job(self.factory)
47 from_person=self.from_person, to_person=self.to_person)73 self.from_person = self.job.from_person
74 self.to_person = self.job.to_person
4875
49 def test_interface(self):76 def test_interface(self):
50 # PersonMergeJob implements IPersonMergeJob.77 # PersonMergeJob implements IPersonMergeJob.
@@ -90,11 +117,7 @@
90 def transfer_email(self):117 def transfer_email(self):
91 # Reassign from_person's email address over to to_person because118 # Reassign from_person's email address over to to_person because
92 # IPersonSet.merge() does not (yet) promise to do that.119 # IPersonSet.merge() does not (yet) promise to do that.
93 from_email = IMasterObject(self.from_person.preferredemail)120 transfer_email(self.job)
94 removeSecurityProxy(from_email).personID = self.to_person.id
95 removeSecurityProxy(from_email).accountID = self.to_person.accountID
96 removeSecurityProxy(from_email).status = EmailAddressStatus.NEW
97 IStore(from_email).flush()
98121
99 def test_run(self):122 def test_run(self):
100 # When run it merges from_person into to_person.123 # When run it merges from_person into to_person.
@@ -179,3 +202,21 @@
179 self.assertEqual([self.job], self.find())202 self.assertEqual([self.job], self.find())
180 else:203 else:
181 self.assertEqual([], self.find())204 self.assertEqual([], self.find())
205
206
207class TestViaCelery(TestCaseWithFactory):
208 """Test that PersonMergeJob runs under Celery."""
209
210 layer = CeleryJobLayer
211
212 def test_run(self):
213 # When run it merges from_person into to_person.
214 self.useFixture(FeatureFixture({
215 'jobs.celery.enabled_classes': 'PersonMergeJob',
216 }))
217 job = create_job(self.factory)
218 transfer_email(job)
219 from_person = job.from_person
220 with block_on_job(self):
221 transaction.commit()
222 self.assertEqual(job.to_person, from_person.merged)
182223
=== modified file 'lib/lp/services/job/model/job.py'
--- lib/lp/services/job/model/job.py 2012-04-24 20:57:27 +0000
+++ lib/lp/services/job/model/job.py 2012-04-26 16:21:20 +0000
@@ -276,6 +276,7 @@
276 from lp.code.model.branchmergeproposaljob import (276 from lp.code.model.branchmergeproposaljob import (
277 BranchMergeProposalJob,277 BranchMergeProposalJob,
278 )278 )
279 from lp.registry.model.persontransferjob import PersonTransferJob
279 from lp.soyuz.model.distributionjob import DistributionJob280 from lp.soyuz.model.distributionjob import DistributionJob
280 from lp.translations.model.pofilestatsjob import POFileStatsJob281 from lp.translations.model.pofilestatsjob import POFileStatsJob
281 from lp.translations.model.translationsharingjob import (282 from lp.translations.model.translationsharingjob import (
@@ -286,7 +287,7 @@
286287
287 for baseclass in [288 for baseclass in [
288 ApportJob, BranchJob, BranchMergeProposalJob, DistributionJob,289 ApportJob, BranchJob, BranchMergeProposalJob, DistributionJob,
289 POFileStatsJob, TranslationSharingJob,290 PersonTransferJob, POFileStatsJob, TranslationSharingJob,
290 ]:291 ]:
291 derived, base_class, store = cls._getDerived(job_id, baseclass)292 derived, base_class, store = cls._getDerived(job_id, baseclass)
292 if derived is not None:293 if derived is not None: