Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-615654-fillTeamParticipation-timeout into lp:launchpad

  1. bug-615654-fillTeamParticipation-timeout
  2. Merge into devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Māris Fogels
Approved revision: no longer in the source branch.
Merged at revision: 11374
Proposed branch: lp:~edwin-grubbs/launchpad/bug-615654-fillTeamParticipation-timeout
Merge into: lp:launchpad
Diff against target: 102 lines (+44/-15)
1 file modified
lib/lp/registry/model/teammembership.py (+44/-15)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-615654-fillTeamParticipation-timeout
Reviewer Review Type Date Requested Status
Māris Fogels (community) Approve
Review via email: mp+32895@code.launchpad.net

Description of the change

Summary
-------

This branch optimizes the _fillTeamParticipation() method to help
prevent timeouts when approving lots of teams as members. A followup
branch will queue the sending of emails, which should provide a more
complete solution to timeouts.

Tests
-----

./bin/test -vv -t member

Demo and Q/A
------------

* Open http://staging.launchpad.net/~ubuntu-br/+editproposedmembers
  * Try approving 25 teams.
  * There shouldn't be a timeout.

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

Hi Edwin,

This change looks good. r=mars.

I wish there was some better way to load test changes like this. I imagine one could do something creative by pointing the API, LaunchpadObjectFactory, curl, and a stepper algorithm at staging >:)

Maris

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/teammembership.py'
2--- lib/lp/registry/model/teammembership.py 2010-07-16 14:58:17 +0000
3+++ lib/lp/registry/model/teammembership.py 2010-08-17 16:33:03 +0000
4@@ -11,7 +11,6 @@
5 ]
6
7 from datetime import datetime, timedelta
8-import itertools
9 import pytz
10
11 from storm.locals import Store
12@@ -167,8 +166,10 @@
13 simple_sendmail(from_addr, address, subject, msg)
14
15 def canChangeStatusSilently(self, user):
16- """Ensure that the user is in the Launchpad Administrators group before
17- silently making changes to their membership status."""
18+ """Ensure that the user is in the Launchpad Administrators group.
19+
20+ Then the user can silently make changes to their membership status.
21+ """
22 return user.inTeam(getUtility(ILaunchpadCelebrities).admin)
23
24 def canChangeExpirationDate(self, person):
25@@ -288,8 +289,8 @@
26
27 if silent and not self.canChangeStatusSilently(user):
28 raise UserCannotChangeMembershipSilently(
29- "Only Launchpad administrators may change membership statuses "
30- "silently.")
31+ "Only Launchpad administrators may change membership "
32+ "statuses silently.")
33
34 approved = TeamMembershipStatus.APPROVED
35 admin = TeamMembershipStatus.ADMIN
36@@ -536,7 +537,7 @@
37 conditions = [
38 TeamMembership.dateexpires <= when,
39 TeamMembership.status.is_in(
40- [TeamMembershipStatus.ADMIN, TeamMembershipStatus.APPROVED])
41+ [TeamMembershipStatus.ADMIN, TeamMembershipStatus.APPROVED]),
42 ]
43 if exclude_autorenewals:
44 # Avoid circular import.
45@@ -718,20 +719,48 @@
46 return Store.of(person).find(Person, "id IN (%s)" % query)
47
48
49-def _fillTeamParticipation(member, team):
50+def _fillTeamParticipation(member, accepting_team):
51 """Add relevant entries in TeamParticipation for given member and team.
52
53 Add a tuple "member, team" in TeamParticipation for the given team and all
54 of its superteams. More information on how to use the TeamParticipation
55 table can be found in the TeamParticipationUsage spec.
56 """
57- members = [member]
58 if member.isTeam():
59- # The given member is, in fact, a team, and in this case we must
60- # add all of its members to the given team and to its superteams.
61- members.extend(member.allmembers)
62+ # The submembers will be all the members of the team that is
63+ # being added as a member. The superteams will be all the teams
64+ # that the accepting_team belongs to, so all the members will
65+ # also be joining the superteams indirectly. It is important to
66+ # remember that teams are members of themselves, so the member
67+ # team will also be one of the submembers, and the
68+ # accepting_team will also be one of the superteams.
69+ query = """
70+ INSERT INTO TeamParticipation (person, team)
71+ SELECT submember.person, superteam.team
72+ FROM TeamParticipation submember
73+ JOIN TeamParticipation superteam ON TRUE
74+ WHERE submember.team = %(member)d
75+ AND superteam.person = %(accepting_team)d
76+ AND NOT EXISTS (
77+ SELECT 1
78+ FROM TeamParticipation
79+ WHERE person = submember.person
80+ AND team = superteam.team
81+ )
82+ """ % dict(member=member.id, accepting_team=accepting_team.id)
83+ else:
84+ query = """
85+ INSERT INTO TeamParticipation (person, team)
86+ SELECT %(member)d, superteam.team
87+ FROM TeamParticipation superteam
88+ WHERE superteam.person = %(accepting_team)d
89+ AND NOT EXISTS (
90+ SELECT 1
91+ FROM TeamParticipation
92+ WHERE person = %(member)d
93+ AND team = superteam.team
94+ )
95+ """ % dict(member=member.id, accepting_team=accepting_team.id)
96
97- for m in members:
98- for t in itertools.chain(team.super_teams, [team]):
99- if not m.hasParticipationEntryFor(t):
100- TeamParticipation(person=m, team=t)
101+ store = Store.of(member)
102+ store.execute(query)