Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-669724-projectgroup-milestones-timeout into lp:launchpad

  1. bug-669724-projectgroup-milestones-timeout
  2. Merge into devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 12238
Proposed branch: lp:~edwin-grubbs/launchpad/bug-669724-projectgroup-milestones-timeout
Merge into: lp:launchpad
Diff against target: 271 lines (+46/-31)
7 files modified
lib/lp/bugs/doc/official-bug-tags.txt (+10/-5)
lib/lp/registry/browser/configure.zcml (+2/-2)
lib/lp/registry/browser/milestone.py (+10/-6)
lib/lp/registry/browser/project.py (+9/-5)
lib/lp/registry/model/projectgroup.py (+9/-6)
lib/lp/registry/stories/milestone/object-milestones.txt (+1/-2)
lib/lp/registry/templates/object-milestones.pt (+5/-5)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-669724-projectgroup-milestones-timeout
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+46837@code.launchpad.net

Commit message

[r=sinzui][ui=none][bug=669724] Fixed timeout on $projectgroup/+milestones page.

Description of the change

Summary
-------

This branch fixes timeouts on the $projectgroup/+milestones page by
removing the counts for bugs and blueprints, since that information has
already been removed for projects and distributions.
It would be possible to re-add the bug and blueprints counts by writing
a query that gets the counts for all the milestones at once. However,
this will require rewriting the templates or adding a decorator on the
milestone object with count data for the various types of milestone
targets.

Implementation details
----------------------

The +pillar-table-row for IProjectGroupMilestone was using MilestoneView
instead of MilestonesView, which has show_series_context=True, and that
caused should_show_bugs_and_blueprints to be false for projects and
distributions, but not for project groups. I made this more explicit. I
renamed MilestonesView to avoid confusing it with MilestoneView, and it
also wasn't a very accurate name. Cache the milestones as a list so
that it can be referenced multiple times in the template without
evaluating the storm result set multiple times.
    lib/lp/registry/browser/configure.zcml
    lib/lp/registry/browser/milestone.py
    lib/lp/registry/stories/milestone/object-milestones.txt
    lib/lp/registry/templates/object-milestones.pt

Reduced the number of queries in the page by preventing the menu from
querying whether the project group has projects for each menu item.
    lib/lp/registry/browser/project.py

Instead of one query per project, use one query to get all bug tags for
the project group.
    lib/lp/registry/model/projectgroup.py

Fixed test for attribute that now returns storm result set.
    lib/lp/bugs/doc/official-bug-tags.txt

Tests
-----

./bin/test -vv -t 'official-bug-tags.txt|object-milestones.txt'

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

* Open https://launchpad.net/openobject/+milestones
  * It should not timeout.

Lint
----

No lint.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you for discovering my typo. The renaming and clean ups are very appreciated.

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/bugs/doc/official-bug-tags.txt'
2--- lib/lp/bugs/doc/official-bug-tags.txt 2010-10-20 19:41:34 +0000
3+++ lib/lp/bugs/doc/official-bug-tags.txt 2011-01-19 21:51:20 +0000
4@@ -1,4 +1,5 @@
5-= Official Bug Tags =
6+Official Bug Tags
7+=================
8
9 Distributions and products can define official bug tags.
10
11@@ -49,7 +50,8 @@
12 >>> transaction.abort()
13
14
15-== Targets of official bug tags ==
16+Targets of official bug tags
17+----------------------------
18
19 Distribution owners and other persons with the permission launchpad.Edit
20 can add and remove offical bug tags by calling addOfficialBugTag()
21@@ -169,7 +171,8 @@
22 >>> ubuntu.official_bug_tags = [u'foo', u'bar']
23 Traceback (most recent call last):
24 ...
25- Unauthorized: (<Distribution 'Ubuntu' (ubuntu)>, 'official_bug_tags', 'launchpad.BugSupervisor')
26+ Unauthorized: (<Distribution 'Ubuntu' (ubuntu)>,
27+ 'official_bug_tags', 'launchpad.BugSupervisor')
28
29 The same is available for products.
30
31@@ -180,7 +183,8 @@
32 [u'bar', u'foo']
33
34
35-== Official tags for additional bug targets ==
36+Official tags for additional bug targets
37+----------------------------------------
38
39 All IHasBugs implementations provide an official_bug_tags property. They are
40 taken from the relevant distribution or product.
41@@ -213,7 +217,8 @@
42 >>> thunderbird.official_bug_tags = [u'baz']
43 >>> logout()
44 >>> login('no-priv@canonical.com')
45- >>> getUtility(IProjectGroupSet).getByName('mozilla').official_bug_tags
46+ >>> mozilla = getUtility(IProjectGroupSet).getByName('mozilla')
47+ >>> list(mozilla.official_bug_tags)
48 [u'bar', u'baz', u'foo']
49 >>> logout()
50
51
52=== modified file 'lib/lp/registry/browser/configure.zcml'
53--- lib/lp/registry/browser/configure.zcml 2011-01-18 20:49:35 +0000
54+++ lib/lp/registry/browser/configure.zcml 2011-01-19 21:51:20 +0000
55@@ -1330,14 +1330,14 @@
56 <browser:page
57 name="+pillar-table-row"
58 for="lp.registry.interfaces.milestone.IMilestone"
59- class="lp.registry.browser.milestone.MilestonesView"
60+ class="lp.registry.browser.milestone.MilestoneWithoutCountsView"
61 facet="overview"
62 permission="zope.Public"
63 template="../templates/productseries-milestone-table-row.pt"/>
64 <browser:page
65 name="+pillar-table-row"
66 for="lp.registry.interfaces.milestone.IProjectGroupMilestone"
67- class="lp.registry.browser.milestone.MilestoneView"
68+ class="lp.registry.browser.milestone.MilestoneWithoutCountsView"
69 facet="overview"
70 permission="zope.Public"
71 template="../templates/productseries-milestone-table-row.pt"/>
72
73=== modified file 'lib/lp/registry/browser/milestone.py'
74--- lib/lp/registry/browser/milestone.py 2010-12-10 15:06:12 +0000
75+++ lib/lp/registry/browser/milestone.py 2011-01-19 21:51:20 +0000
76@@ -15,7 +15,7 @@
77 'MilestoneNavigation',
78 'MilestoneOverviewNavigationMenu',
79 'MilestoneSetNavigation',
80- 'MilestonesView',
81+ 'MilestoneWithoutCountsView',
82 'MilestoneView',
83 'ObjectMilestonesView',
84 ]
85@@ -55,9 +55,7 @@
86 LaunchpadFormView,
87 )
88 from lp.bugs.browser.bugtask import BugTaskListingItem
89-from lp.bugs.interfaces.bugtask import (
90- IBugTaskSet,
91- )
92+from lp.bugs.interfaces.bugtask import IBugTaskSet
93 from lp.registry.browser import (
94 get_status_counts,
95 RegistryDeleteViewMixin,
96@@ -213,7 +211,7 @@
97 @property
98 def should_show_bugs_and_blueprints(self):
99 """Display the summary of bugs/blueprints for this milestone?"""
100- return (not self.show_series_context) and self.milestone.active
101+ return self.milestone.active
102
103 @property
104 def page_title(self):
105@@ -368,9 +366,11 @@
106 return len(self.bugtasks) > 0 or len(self.specifications) > 0
107
108
109-class MilestonesView(MilestoneView):
110+class MilestoneWithoutCountsView(MilestoneView):
111 """Show a milestone in a list of milestones."""
112+
113 show_series_context = True
114+ should_show_bugs_and_blueprints = False
115
116
117 class MilestoneAddView(LaunchpadFormView):
118@@ -516,3 +516,7 @@
119 """A view for listing the milestones for any `IHasMilestones` object"""
120
121 label = 'Milestones'
122+
123+ @cachedproperty
124+ def milestones(self):
125+ return list(self.context.all_milestones)
126
127=== modified file 'lib/lp/registry/browser/project.py'
128--- lib/lp/registry/browser/project.py 2010-12-13 21:41:15 +0000
129+++ lib/lp/registry/browser/project.py 2011-01-19 21:51:20 +0000
130@@ -164,29 +164,33 @@
131 enable_only = ['overview', 'branches', 'bugs', 'specifications',
132 'answers', 'translations']
133
134+ @cachedproperty
135+ def has_products(self):
136+ return self.context.hasProducts()
137+
138 def branches(self):
139 text = 'Code'
140- return Link('', text, enabled=self.context.hasProducts())
141+ return Link('', text, enabled=self.has_products)
142
143 def bugs(self):
144 site = 'bugs'
145 text = 'Bugs'
146- return Link('', text, enabled=self.context.hasProducts(), site=site)
147+ return Link('', text, enabled=self.has_products, site=site)
148
149 def answers(self):
150 site = 'answers'
151 text = 'Answers'
152- return Link('', text, enabled=self.context.hasProducts(), site=site)
153+ return Link('', text, enabled=self.has_products, site=site)
154
155 def specifications(self):
156 site = 'blueprints'
157 text = 'Blueprints'
158- return Link('', text, enabled=self.context.hasProducts(), site=site)
159+ return Link('', text, enabled=self.has_products, site=site)
160
161 def translations(self):
162 site = 'translations'
163 text = 'Translations'
164- return Link('', text, enabled=self.context.hasProducts(), site=site)
165+ return Link('', text, enabled=self.has_products, site=site)
166
167
168 class ProjectAdminMenuMixin:
169
170=== modified file 'lib/lp/registry/model/projectgroup.py'
171--- lib/lp/registry/model/projectgroup.py 2010-12-25 21:28:14 +0000
172+++ lib/lp/registry/model/projectgroup.py 2011-01-19 21:51:20 +0000
173@@ -20,8 +20,8 @@
174 )
175 from storm.expr import (
176 And,
177+ Join,
178 SQL,
179- Join,
180 )
181 from storm.locals import Int
182 from storm.store import Store
183@@ -91,6 +91,7 @@
184 IProjectGroupSet,
185 )
186 from lp.registry.model.announcement import MakesAnnouncements
187+from lp.registry.model.hasdrivers import HasDriversMixin
188 from lp.registry.model.karma import KarmaContextMixin
189 from lp.registry.model.milestone import (
190 HasMilestonesMixin,
191@@ -100,7 +101,6 @@
192 from lp.registry.model.pillar import HasAliasMixin
193 from lp.registry.model.product import Product
194 from lp.registry.model.productseries import ProductSeries
195-from lp.registry.model.hasdrivers import HasDriversMixin
196 from lp.registry.model.structuralsubscription import (
197 StructuralSubscriptionTargetMixin,
198 )
199@@ -327,10 +327,13 @@
200 @property
201 def official_bug_tags(self):
202 """See `IHasBugs`."""
203- official_bug_tags = set()
204- for product in self.products:
205- official_bug_tags.update(product.official_bug_tags)
206- return sorted(official_bug_tags)
207+ store = Store.of(self)
208+ result = store.find(
209+ OfficialBugTag.tag,
210+ OfficialBugTag.product == Product.id,
211+ Product.project == self.id).order_by(OfficialBugTag.tag)
212+ result.config(distinct=True)
213+ return result
214
215 def getUsedBugTags(self):
216 """See `IHasBugs`."""
217
218=== modified file 'lib/lp/registry/stories/milestone/object-milestones.txt'
219--- lib/lp/registry/stories/milestone/object-milestones.txt 2010-12-21 23:49:34 +0000
220+++ lib/lp/registry/stories/milestone/object-milestones.txt 2011-01-19 21:51:20 +0000
221@@ -129,8 +129,7 @@
222 GNOME 1.3 A date This is an inactive milestone
223 GNOME 1.2 A date not yet released
224 GNOME 1.1. A date not yet released
225- GNOME 1.1 A date not yet released Bugs targeted: 3 Confirmed
226- Blueprints targeted: 2 Unknown
227+ GNOME 1.1 A date not yet released
228
229
230 Individual milestones
231
232=== modified file 'lib/lp/registry/templates/object-milestones.pt'
233--- lib/lp/registry/templates/object-milestones.pt 2010-08-20 04:14:22 +0000
234+++ lib/lp/registry/templates/object-milestones.pt 2011-01-19 21:51:20 +0000
235@@ -14,7 +14,7 @@
236 Milestones belong to a series and can be created from the series page
237 by a project owner or series release manager.
238 </p>
239-
240+
241 <ul class="horizontal"
242 tal:condition="context/project|nothing">
243 <li>
244@@ -25,10 +25,10 @@
245 </ul>
246 </div>
247
248- <tal:milestones define="milestones context/all_milestones">
249+ <tal:milestones>
250 <table id="milestones" class="listing"
251 tal:define="has_series context/series|nothing"
252- tal:condition="context/has_milestones">
253+ tal:condition="view/milestones">
254 <thead>
255 <tr>
256 <th>Version</th>
257@@ -40,12 +40,12 @@
258 </thead>
259 <tbody>
260 <tal:row
261- repeat="milestone milestones"
262+ repeat="milestone view/milestones"
263 replace="structure milestone/@@+pillar-table-row" />
264 </tbody>
265 </table>
266
267- <tal:no-milestones condition="not: context/has_milestones">
268+ <tal:no-milestones condition="not: view/milestones">
269 <p>There are no milestones associated with
270 <span tal:replace="context/title" />
271 </p>