Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-590840-getMembershipInformation-timeout-2 into lp:launchpad

  1. bug-590840-getMembershipInformation-timeout-2
  2. Merge into devel
Proposed by Edwin Grubbs
Status: Merged
Merged at revision: 10996
Proposed branch: lp:~edwin-grubbs/launchpad/bug-590840-getMembershipInformation-timeout-2
Merge into: lp:launchpad
Prerequisite: lp:~edwin-grubbs/launchpad/bug-590840-getMembershipInformation-timeout
Diff against target: 80 lines (+20/-14)
2 files modified
lib/lp/registry/doc/message-holds.txt (+10/-0)
lib/lp/registry/model/mailinglist.py (+10/-14)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-590840-getMembershipInformation-timeout-2
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+27394@code.launchpad.net

Description of the change

In the prerequisite branch bug-590840-getMembershipInformation-timeout, after I got the review, I discovered that it was testing both of the unioned queries. I added tests in this branch, and I also optimized MailingList.getSenderAddresses() like I did in the previous branch for MailingListSet.getSenderAddresses(team_names) by not selecting information that would be thrown away.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Looks fine, Edwin. Thanks for the clarifications on IRC.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/doc/message-holds.txt'
2--- lib/lp/registry/doc/message-holds.txt 2010-01-12 22:23:02 +0000
3+++ lib/lp/registry/doc/message-holds.txt 2010-06-11 18:56:33 +0000
4@@ -412,6 +412,12 @@
5 >>> sorted(email.email for email in list_three.getSenderAddresses())
6 [u'no-priv@canonical.com']
7
8+ >>> from lp.registry.interfaces.mailinglist import IMailingListSet
9+ >>> mailinglist_set = getUtility(IMailingListSet)
10+ >>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
11+ >>> sorted(email for name, email in bulk_addresses['test-three'])
12+ [u'no-priv@canonical.com']
13+
14 Salgado posts another message to the list, but this time it's approved.
15
16 >>> message = message_set.fromEmail("""\
17@@ -435,6 +441,10 @@
18 >>> sorted(email.email for email in list_three.getSenderAddresses())
19 [u'guilherme.salgado@canonical.com', u'no-priv@canonical.com']
20
21+ >>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
22+ >>> sorted(email for name, email in bulk_addresses['test-three'])
23+ [u'guilherme.salgado@canonical.com', u'no-priv@canonical.com']
24+
25 But he will still not get messages delivered to his address, since he's not
26 subscribed to the team mailing list.
27
28
29=== modified file 'lib/lp/registry/model/mailinglist.py'
30--- lib/lp/registry/model/mailinglist.py 2010-06-11 18:56:32 +0000
31+++ lib/lp/registry/model/mailinglist.py 2010-06-11 18:56:33 +0000
32@@ -467,15 +467,13 @@
33 # are allowed to post to the mailing list.
34 tables = (
35 Person,
36- LeftJoin(Account, Account.id == Person.accountID),
37- LeftJoin(EmailAddress, EmailAddress.personID == Person.id),
38- LeftJoin(TeamParticipation,
39- TeamParticipation.personID == Person.id),
40- LeftJoin(MailingList,
41- MailingList.teamID == TeamParticipation.teamID),
42+ Join(Account, Account.id == Person.accountID),
43+ Join(EmailAddress, EmailAddress.personID == Person.id),
44+ Join(TeamParticipation, TeamParticipation.personID == Person.id),
45+ Join(MailingList, MailingList.teamID == TeamParticipation.teamID),
46 )
47 team_members = store.using(*tables).find(
48- (EmailAddress, Person),
49+ EmailAddress,
50 And(TeamParticipation.team == self.team,
51 MailingList.status != MailingListStatus.INACTIVE,
52 Person.teamowner == None,
53@@ -489,13 +487,12 @@
54 # global approvals.
55 tables = (
56 Person,
57- LeftJoin(Account, Account.id == Person.accountID),
58- LeftJoin(EmailAddress, EmailAddress.personID == Person.id),
59- LeftJoin(MessageApproval,
60- MessageApproval.posted_byID == Person.id),
61+ Join(Account, Account.id == Person.accountID),
62+ Join(EmailAddress, EmailAddress.personID == Person.id),
63+ Join(MessageApproval, MessageApproval.posted_byID == Person.id),
64 )
65 approved_posters = store.using(*tables).find(
66- (EmailAddress, Person),
67+ EmailAddress,
68 And(MessageApproval.mailing_list == self,
69 MessageApproval.status.is_in(MESSAGE_APPROVAL_STATUSES),
70 EmailAddress.status.is_in(EMAIL_ADDRESS_STATUSES),
71@@ -509,8 +506,7 @@
72 # email_address.person.displayname, so the prejoin to Person is
73 # critical to acceptable performance. Indeed, without the prejoin, we
74 # were getting tons of timeout OOPSes. See bug 259440.
75- for email_address, person in team_members.union(approved_posters):
76- yield email_address
77+ return team_members.union(approved_posters)
78
79 def holdMessage(self, message):
80 """See `IMailingList`."""