Merge lp:~bac/launchpad/bug-488762-snapshot into lp:launchpad

Proposed by Brad Crittenden
Status: Merged
Approved by: Paul Hummer
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-488762-snapshot
Merge into: lp:launchpad
Diff against target: 240 lines (+80/-52)
6 files modified
lib/canonical/launchpad/webapp/snapshot.py (+1/-1)
lib/lp/registry/doc/person.txt (+1/-3)
lib/lp/registry/interfaces/person.py (+62/-45)
lib/lp/registry/model/product.py (+0/-2)
lib/lp/registry/tests/test_person.py (+15/-0)
versions.cfg (+1/-1)
To merge this branch: bzr merge lp:~bac/launchpad/bug-488762-snapshot
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Review via email: mp+15628@code.launchpad.net

Commit message

Remove large collections from IPerson snapshots.

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

= Summary =

Editing large teams fails because taking a snapshot of the team includes all of the
collections for that team, big stuff like all of the members. There is no reason for
those collections to be part of the snapshot.

== Proposed fix ==

lazr.lifecycle has a new method 'doNotSnapshot' that can encapuslate a Field and make
it not be included in the snapshot. On IPerson all of the collections have been
wrapped in that method to prevent their inclusion.

== Pre-implementation notes ==

Chats with Curtis.

== Implementation details ==

As above.

== Tests ==

I've added a test to show that the fields I currently expect to be omitted from the
snapshot are in fact omitted. It's not a very interesting test but there to prevent
regression.

bin/test -vvm lp.registry -t test_person_snapshot

== Demo and Q/A ==

On staging become an admin and try to edit the team in the bug report. It should
succeed.

= Launchpad lint =

Bah!

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/webapp/snapshot.py
  versions.cfg
  lib/lp/registry/doc/person.txt
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_person.py

== Pylint notices ==

lib/canonical/launchpad/webapp/snapshot.py
    11: [F0401] Unable to import 'lazr.lifecycle.interfaces' (No module named lifecycle)

lib/lp/registry/interfaces/person.py
    52: [F0401] Unable to import 'lazr.enum' (No module named enum)
    53: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module named lifecycle)
    54: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    55: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    62: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    406: [E1002, PersonNameField._validate] Use super on an old style class
    1401: [C0322, IPersonEditRestricted.addMember] Operator not preceded by a space
    status=copy_field(ITeamMembership['status']),
    ^
    comment=Text(required=False))
    @export_write_operation()
    def addMember(person, reviewer, status=TeamMembershipStatus.APPROVED,
    comment=None, force_team_add=False,
    may_subscribe_to_list=True):
    1433: [C0322, IPersonEditRestricted.acceptInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def acceptInvitationToBeMemberOf(team, comment):
    1445: [C0322, IPersonEditRestricted.declineInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def declineInvitationToBeMemberOf(team, comment):
    1737: [C0322, IPersonSet.newTeam] Operator not preceded by a space
    defaultmembershipperiod='default_membership_period',
    ^
    defaultrenewalperiod='default_renewal_period')
    @operation_parameters(
    subscriptionpolicy=Choice(
    title=_('Subscription policy'), vocabulary=TeamSubscriptionPolicy,
    required=False, default=TeamSubscriptionPolicy.MODERATED))
    @export_factory_operation(
    ITeam, ['name', 'displayname'...

Read more...

Revision history for this message
Paul Hummer (rockstar) wrote :

Please fix the "# DO NOT CHECK IN" code. Other than that, this is ready to land.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/snapshot.py'
2--- lib/canonical/launchpad/webapp/snapshot.py 2009-06-25 05:30:52 +0000
3+++ lib/canonical/launchpad/webapp/snapshot.py 2009-12-05 12:29:16 +0000
4@@ -19,4 +19,4 @@
5 # SelectResults, which doesn't really help the Snapshot
6 # object. We therefore list()ify the values; this isn't
7 # perfect but allows deltas to be generated reliably.
8- return shortlist(value, longest_expected=100, hardlimit=5000)
9+ return shortlist(value, longest_expected=100, hardlimit=1000)
10
11=== modified file 'lib/lp/registry/doc/person.txt'
12--- lib/lp/registry/doc/person.txt 2009-08-13 19:03:36 +0000
13+++ lib/lp/registry/doc/person.txt 2009-12-05 12:29:16 +0000
14@@ -626,9 +626,7 @@
15 {}
16
17 ** See lib/canonical/launchpad/doc/teammembership.txt for more
18-information
19-
20- about team membership/participation.
21+information about team membership/participation.
22
23
24 === Email notifications to teams ===
25
26=== modified file 'lib/lp/registry/interfaces/person.py'
27--- lib/lp/registry/interfaces/person.py 2009-12-04 22:24:18 +0000
28+++ lib/lp/registry/interfaces/person.py 2009-12-05 12:29:16 +0000
29@@ -50,7 +50,7 @@
30 from zope.interface.interface import invariant
31 from zope.component import getUtility
32 from lazr.enum import DBEnumeratedType, DBItem, EnumeratedType, Item
33-
34+from lazr.lifecycle.snapshot import doNotSnapshot
35 from lazr.restful.interface import copy_field
36 from lazr.restful.declarations import (
37 LAZR_WEBSERVICE_EXPORTED, REQUEST_USER, call_with,
38@@ -1198,75 +1198,92 @@
39 # activemembers.value_type.schema will be set to IPerson once
40 # IPerson is defined.
41 activemembers = exported(
42- CollectionField(
43- title=_("List of members with ADMIN or APPROVED status"),
44- value_type=Reference(schema=Interface)),
45+ doNotSnapshot(
46+ CollectionField(
47+ title=_("List of members with ADMIN or APPROVED status"),
48+ value_type=Reference(schema=Interface))),
49 exported_as='members')
50 adminmembers = exported(
51- CollectionField(
52- title=_("List of this team's admins."),
53- value_type=Reference(schema=Interface)),
54+ doNotSnapshot(
55+ CollectionField(
56+ title=_("List of this team's admins."),
57+ value_type=Reference(schema=Interface))),
58 exported_as='admins')
59 all_member_count = Attribute(
60 "The total number of real people who are members of this team, "
61 "including subteams.")
62 allmembers = exported(
63- CollectionField(
64- title=_("All participants of this team."),
65- description=_(
66- "List of all direct and indirect people and teams who, one "
67- "way or another, are a part of this team. If you want a "
68- "method to check if a given person is a member of a team, "
69- "you should probably look at IPerson.inTeam()."),
70- value_type=Reference(schema=Interface)),
71+ doNotSnapshot(
72+ CollectionField(
73+ title=_("All participants of this team."),
74+ description=_(
75+ "List of all direct and indirect people and teams who, "
76+ "one way or another, are a part of this team. If you "
77+ "want a method to check if a given person is a member "
78+ "of a team, you should probably look at "
79+ "IPerson.inTeam()."),
80+ value_type=Reference(schema=Interface))),
81 exported_as='participants')
82- approvedmembers = Attribute("List of members with APPROVED status")
83+ approvedmembers = doNotSnapshot(
84+ Attribute("List of members with APPROVED status"))
85 deactivated_member_count = Attribute("Number of deactivated members")
86- deactivatedmembers = Attribute("List of members with DEACTIVATED status")
87 deactivatedmembers = exported(
88- CollectionField(
89- title=_(
90- "All members whose membership is in the DEACTIVATED state"),
91- value_type=Reference(schema=Interface)),
92+ doNotSnapshot(
93+ CollectionField(
94+ title=_(
95+ "All members whose membership is in the "
96+ "DEACTIVATED state"),
97+ value_type=Reference(schema=Interface))),
98 exported_as='deactivated_members')
99 expired_member_count = Attribute("Number of EXPIRED members.")
100 expiredmembers = exported(
101- CollectionField(
102- title=_("All members whose membership is in the EXPIRED state"),
103- value_type=Reference(schema=Interface)),
104+ doNotSnapshot(
105+ CollectionField(
106+ title=_("All members whose membership is in the "
107+ "EXPIRED state"),
108+ value_type=Reference(schema=Interface))),
109 exported_as='expired_members')
110- inactivemembers = Attribute(
111- "List of members with EXPIRED or DEACTIVATED status")
112+ inactivemembers = doNotSnapshot(
113+ Attribute(
114+ "List of members with EXPIRED or DEACTIVATED status"))
115 inactive_member_count = Attribute("Number of inactive members")
116 invited_members = exported(
117- CollectionField(
118- title=_("All members whose membership is in the INVITED state"),
119- value_type=Reference(schema=Interface)))
120+ doNotSnapshot(
121+ CollectionField(
122+ title=_("All members whose membership is "
123+ "in the INVITED state"),
124+ value_type=Reference(schema=Interface))))
125+
126 invited_member_count = Attribute("Number of members with INVITED status")
127 member_memberships = exported(
128- CollectionField(
129- title=_("Active TeamMemberships for this object's members."),
130- description=_(
131- "Active TeamMemberships are the ones with the ADMIN or "
132- "APPROVED status. The results are ordered using "
133- "Person.sortingColumns."),
134- readonly=True, required=False,
135- value_type=Reference(schema=ITeamMembership)),
136+ doNotSnapshot(
137+ CollectionField(
138+ title=_("Active TeamMemberships for this object's members."),
139+ description=_(
140+ "Active TeamMemberships are the ones with the ADMIN or "
141+ "APPROVED status. The results are ordered using "
142+ "Person.sortingColumns."),
143+ readonly=True, required=False,
144+ value_type=Reference(schema=ITeamMembership))),
145 exported_as='members_details')
146- pendingmembers = Attribute(
147- "List of members with INVITED or PROPOSED status")
148+ pendingmembers = doNotSnapshot(
149+ Attribute(
150+ "List of members with INVITED or PROPOSED status"))
151 proposedmembers = exported(
152- CollectionField(
153- title=_("All members whose membership is in the PROPOSED state"),
154- value_type=Reference(schema=Interface)),
155+ doNotSnapshot(
156+ CollectionField(
157+ title=_("All members whose membership is in the "
158+ "PROPOSED state"),
159+ value_type=Reference(schema=Interface))),
160 exported_as='proposed_members')
161 proposed_member_count = Attribute("Number of PROPOSED members")
162
163 mapped_participants_count = Attribute(
164 "The number of mapped participants")
165- unmapped_participants = CollectionField(
166- title=_("List of participants with no coordinates recorded."),
167- value_type=Reference(schema=Interface))
168+ unmapped_participants = doNotSnapshot(
169+ CollectionField(
170+ title=_("List of participants with no coordinates recorded."),
171+ value_type=Reference(schema=Interface)))
172 unmapped_participants_count = Attribute(
173 "The number of unmapped participants")
174
175
176=== modified file 'lib/lp/registry/model/product.py'
177--- lib/lp/registry/model/product.py 2009-11-20 07:32:32 +0000
178+++ lib/lp/registry/model/product.py 2009-12-05 12:29:16 +0000
179@@ -546,8 +546,6 @@
180 """See `IBugTarget`."""
181 return get_bug_tags_open_count(BugTask.product == self, user)
182
183- branches = SQLMultipleJoin('Branch', joinColumn='product',
184- orderBy='id')
185 series = SQLMultipleJoin('ProductSeries', joinColumn='product',
186 orderBy='name')
187
188
189=== modified file 'lib/lp/registry/tests/test_person.py'
190--- lib/lp/registry/tests/test_person.py 2009-10-01 11:21:31 +0000
191+++ lib/lp/registry/tests/test_person.py 2009-12-05 12:29:16 +0000
192@@ -8,6 +8,7 @@
193 import pytz
194
195 from zope.component import getUtility
196+from zope.interface import providedBy
197 from zope.security.proxy import removeSecurityProxy
198
199 from canonical.database.sqlbase import cursor
200@@ -16,6 +17,7 @@
201 from lp.bugs.interfaces.bug import CreateBugParams, IBugSet
202 from canonical.launchpad.interfaces.emailaddress import (
203 EmailAddressAlreadyTaken, IEmailAddressSet, InvalidEmailAddress)
204+from lazr.lifecycle.snapshot import Snapshot
205 from lp.blueprints.interfaces.specification import ISpecificationSet
206 from lp.registry.interfaces.person import InvalidName
207 from lp.registry.interfaces.product import IProductSet
208@@ -406,6 +408,19 @@
209 self.assertEqual(self.otherteam.visibility,
210 PersonVisibility.PRIVATE_MEMBERSHIP)
211
212+ def test_person_snapshot(self):
213+ omitted = (
214+ 'activemembers', 'adminmembers', 'allmembers', 'approvedmembers',
215+ 'deactivatedmembers', 'expiredmembers', 'inactivemembers',
216+ 'invited_members', 'member_memberships', 'pendingmembers',
217+ 'proposedmembers', 'unmapped_participants',
218+ )
219+ snap = Snapshot(self.myteam, providing=providedBy(self.myteam))
220+ for name in omitted:
221+ self.assertFalse(
222+ hasattr(snap, name),
223+ "%s should be omitted from the snapshot but is not." % name)
224+
225
226 class TestPersonSet(unittest.TestCase):
227 """Test `IPersonSet`."""
228
229=== modified file 'versions.cfg'
230--- versions.cfg 2009-11-26 23:08:57 +0000
231+++ versions.cfg 2009-12-05 12:29:16 +0000
232@@ -25,7 +25,7 @@
233 lazr.config = 1.1.3
234 lazr.delegates = 1.1.0
235 lazr.enum = 1.1.2
236-lazr.lifecycle = 1.0
237+lazr.lifecycle = 1.1
238 lazr.restful = 0.9.17
239 lazr.restfulclient = 0.9.10
240 lazr.smtptest = 1.1