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
=== modified file 'lib/lp/registry/doc/message-holds.txt'
--- lib/lp/registry/doc/message-holds.txt 2010-01-12 22:23:02 +0000
+++ lib/lp/registry/doc/message-holds.txt 2010-06-11 18:56:33 +0000
@@ -412,6 +412,12 @@
412 >>> sorted(email.email for email in list_three.getSenderAddresses())412 >>> sorted(email.email for email in list_three.getSenderAddresses())
413 [u'no-priv@canonical.com']413 [u'no-priv@canonical.com']
414414
415 >>> from lp.registry.interfaces.mailinglist import IMailingListSet
416 >>> mailinglist_set = getUtility(IMailingListSet)
417 >>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
418 >>> sorted(email for name, email in bulk_addresses['test-three'])
419 [u'no-priv@canonical.com']
420
415Salgado posts another message to the list, but this time it's approved.421Salgado posts another message to the list, but this time it's approved.
416422
417 >>> message = message_set.fromEmail("""\423 >>> message = message_set.fromEmail("""\
@@ -435,6 +441,10 @@
435 >>> sorted(email.email for email in list_three.getSenderAddresses())441 >>> sorted(email.email for email in list_three.getSenderAddresses())
436 [u'guilherme.salgado@canonical.com', u'no-priv@canonical.com']442 [u'guilherme.salgado@canonical.com', u'no-priv@canonical.com']
437443
444 >>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
445 >>> sorted(email for name, email in bulk_addresses['test-three'])
446 [u'guilherme.salgado@canonical.com', u'no-priv@canonical.com']
447
438But he will still not get messages delivered to his address, since he's not448But he will still not get messages delivered to his address, since he's not
439subscribed to the team mailing list.449subscribed to the team mailing list.
440450
441451
=== modified file 'lib/lp/registry/model/mailinglist.py'
--- lib/lp/registry/model/mailinglist.py 2010-06-11 18:56:32 +0000
+++ lib/lp/registry/model/mailinglist.py 2010-06-11 18:56:33 +0000
@@ -467,15 +467,13 @@
467 # are allowed to post to the mailing list.467 # are allowed to post to the mailing list.
468 tables = (468 tables = (
469 Person,469 Person,
470 LeftJoin(Account, Account.id == Person.accountID),470 Join(Account, Account.id == Person.accountID),
471 LeftJoin(EmailAddress, EmailAddress.personID == Person.id),471 Join(EmailAddress, EmailAddress.personID == Person.id),
472 LeftJoin(TeamParticipation,472 Join(TeamParticipation, TeamParticipation.personID == Person.id),
473 TeamParticipation.personID == Person.id),473 Join(MailingList, MailingList.teamID == TeamParticipation.teamID),
474 LeftJoin(MailingList,
475 MailingList.teamID == TeamParticipation.teamID),
476 )474 )
477 team_members = store.using(*tables).find(475 team_members = store.using(*tables).find(
478 (EmailAddress, Person),476 EmailAddress,
479 And(TeamParticipation.team == self.team,477 And(TeamParticipation.team == self.team,
480 MailingList.status != MailingListStatus.INACTIVE,478 MailingList.status != MailingListStatus.INACTIVE,
481 Person.teamowner == None,479 Person.teamowner == None,
@@ -489,13 +487,12 @@
489 # global approvals.487 # global approvals.
490 tables = (488 tables = (
491 Person,489 Person,
492 LeftJoin(Account, Account.id == Person.accountID),490 Join(Account, Account.id == Person.accountID),
493 LeftJoin(EmailAddress, EmailAddress.personID == Person.id),491 Join(EmailAddress, EmailAddress.personID == Person.id),
494 LeftJoin(MessageApproval,492 Join(MessageApproval, MessageApproval.posted_byID == Person.id),
495 MessageApproval.posted_byID == Person.id),
496 )493 )
497 approved_posters = store.using(*tables).find(494 approved_posters = store.using(*tables).find(
498 (EmailAddress, Person),495 EmailAddress,
499 And(MessageApproval.mailing_list == self,496 And(MessageApproval.mailing_list == self,
500 MessageApproval.status.is_in(MESSAGE_APPROVAL_STATUSES),497 MessageApproval.status.is_in(MESSAGE_APPROVAL_STATUSES),
501 EmailAddress.status.is_in(EMAIL_ADDRESS_STATUSES),498 EmailAddress.status.is_in(EMAIL_ADDRESS_STATUSES),
@@ -509,8 +506,7 @@
509 # email_address.person.displayname, so the prejoin to Person is506 # email_address.person.displayname, so the prejoin to Person is
510 # critical to acceptable performance. Indeed, without the prejoin, we507 # critical to acceptable performance. Indeed, without the prejoin, we
511 # were getting tons of timeout OOPSes. See bug 259440.508 # were getting tons of timeout OOPSes. See bug 259440.
512 for email_address, person in team_members.union(approved_posters):509 return team_members.union(approved_posters)
513 yield email_address
514510
515 def holdMessage(self, message):511 def holdMessage(self, message):
516 """See `IMailingList`."""512 """See `IMailingList`."""