Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-490659-series-timeout-part2 into lp:launchpad/db-devel

  1. bug-490659-series-timeout-part2
  2. Merge into db-devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 9734
Proposed branch: lp:~edwin-grubbs/launchpad/bug-490659-series-timeout-part2
Merge into: lp:launchpad/db-devel
Diff against target: 288 lines (+120/-13)
10 files modified
lib/lp/registry/browser/distroseries.py (+4/-0)
lib/lp/registry/browser/productseries.py (+6/-1)
lib/lp/registry/browser/tests/test_series_views.py (+42/-4)
lib/lp/registry/interfaces/product.py (+4/-1)
lib/lp/registry/interfaces/productseries.py (+4/-1)
lib/lp/registry/model/milestone.py (+5/-2)
lib/lp/registry/model/productseries.py (+4/-1)
lib/lp/registry/templates/productseries-milestone-table-row.pt (+1/-1)
lib/lp/registry/templates/productseries-table-releases.pt (+6/-2)
lib/lp/registry/tests/test_product.py (+44/-0)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-490659-series-timeout-part2
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+34159@code.launchpad.net

Description of the change

Summary
-------

This branch is dependent on the milestone_sort_key() db function that
was recently landed on db-devel. The milestones/releases for a series
are now batched. I also limited the number of milestones/releases
displayed in the timeline graph.

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

Added batching:
    lib/lp/registry/browser/distroseries.py
    lib/lp/registry/browser/productseries.py
    lib/lp/registry/browser/tests/test_distroseries_views.py
    lib/lp/registry/browser/tests/test_productseries_views.py
    lib/lp/registry/model/milestone.py
    lib/lp/registry/templates/productseries-table-releases.pt

Limited the number of milestones/releases displayed in the timeline
graph.
    lib/lp/registry/interfaces/product.py
    lib/lp/registry/interfaces/productseries.py
    lib/lp/registry/model/productseries.py
    lib/lp/registry/tests/test_product.py

The cache TALes needs the third argument to ensure that it retrieves the
right cached value for a block in a loop.
    lib/lp/registry/templates/productseries-milestone-table-row.pt

Tests
-----

$ ./bin/test --list-tests -vv -t 'test_(distro|product)series_views|registry\.tests\.test_product\.'

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

* Open http://launchpad.dev/firefox/trunk

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

A meta-question; is batching here needed? I'm surprised we have enough
data that its a problem (or put another way, whats the scaling factors
at play here; how can we be sure batching will fix the issue).

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> A meta-question; is batching here needed? I'm surprised we have enough
> data that its a problem (or put another way, whats the scaling factors
> at play here; how can we be sure batching will fix the issue).

The libpng project has 777 releases on a single series. This is caused by the Release URL Pattern on the "main" series which automatically downloads new versions of libpng tarballs and creates a revision to hold the file.

Even projects that don't have that many milestones or releases may have problems displaying all of them since each milestone will display stats on how many bugs and blueprints are assigned to it.

Revision history for this message
Graham Binns (gmb) wrote :

<gmb> EdwinGrubbs, Those two tests are *awfully* similar. Any chance they could be unified in their own TestCase (e.g. TestFooBatchNavigators)
<EdwinGrubbs> gmb: sure
<gmb> EdwinGrubbs, Cool.
 EdwinGrubbs, Other than that, r=me.

review: Approve (code)
Revision history for this message
Robert Collins (lifeless) wrote :

On Wed, Sep 1, 2010 at 2:41 AM, Edwin Grubbs <email address hidden> wrote:

> The libpng project has 777 releases on a single series. This is caused by the Release URL Pattern on the "main" series which automatically downloads new versions of libpng tarballs and creates a revision to hold the file.
>
> Even projects that don't have that many milestones or releases may have problems displaying all of them since each milestone will display stats on how many bugs and blueprints are assigned to it.

Wow. We should be able to make the stats display reasonably fast;
might want to consider tuning the batch size up a bit if we can, e.g.
to 24 (2 years of monthly releases).

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

On Tue, Aug 31, 2010 at 4:28 PM, Robert Collins
<email address hidden> wrote:
> On Wed, Sep 1, 2010 at 2:41 AM, Edwin Grubbs <email address hidden> wrote:
>
>> The libpng project has 777 releases on a single series. This is caused by the Release URL Pattern on the "main" series which automatically downloads new versions of libpng tarballs and creates a revision to hold the file.
>>
>> Even projects that don't have that many milestones or releases may have problems displaying all of them since each milestone will display stats on how many bugs and blueprints are assigned to it.
>
> Wow. We should be able to make the stats display reasonably fast;
> might want to consider tuning the batch size up a bit if we can, e.g.
> to 24 (2 years of monthly releases).

The default batch size on launchpad.dev is 5, but the default batch
size on production is 50. This default is part of the BatchNavigator,
so it doesn't just apply to this page.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/distroseries.py 2010-09-01 00:31:00 +0000
@@ -360,6 +360,10 @@
360360
361 milestone_can_release = False361 milestone_can_release = False
362362
363 @cachedproperty
364 def milestone_batch_navigator(self):
365 return BatchNavigator(self.context.all_milestones, self.request)
366
363367
364class DistroSeriesEditView(LaunchpadEditFormView, SeriesStatusMixin):368class DistroSeriesEditView(LaunchpadEditFormView, SeriesStatusMixin):
365 """View class that lets you edit a DistroSeries object.369 """View class that lets you edit a DistroSeries object.
366370
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py 2010-08-24 19:06:33 +0000
+++ lib/lp/registry/browser/productseries.py 2010-09-01 00:31:00 +0000
@@ -59,6 +59,7 @@
59from canonical.launchpad import _59from canonical.launchpad import _
60from canonical.launchpad.helpers import browserLanguages60from canonical.launchpad.helpers import browserLanguages
61from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities61from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
62from canonical.launchpad.webapp.batching import BatchNavigator
62from canonical.launchpad.webapp import (63from canonical.launchpad.webapp import (
63 ApplicationMenu,64 ApplicationMenu,
64 canonical_url,65 canonical_url,
@@ -475,13 +476,17 @@
475 for status in sorted(status_counts,476 for status in sorted(status_counts,
476 key=attrgetter('sortkey'))]477 key=attrgetter('sortkey'))]
477478
478 @property479 @cachedproperty
479 def latest_release_with_download_files(self):480 def latest_release_with_download_files(self):
480 for release in self.context.releases:481 for release in self.context.releases:
481 if len(list(release.files)) > 0:482 if len(list(release.files)) > 0:
482 return release483 return release
483 return None484 return None
484485
486 @cachedproperty
487 def milestone_batch_navigator(self):
488 return BatchNavigator(self.context.all_milestones, self.request)
489
485490
486class ProductSeriesDetailedDisplayView(ProductSeriesView):491class ProductSeriesDetailedDisplayView(ProductSeriesView):
487492
488493
=== renamed file 'lib/lp/registry/browser/tests/test_distroseries_views.py' => 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_distroseries_views.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2010-09-01 00:31:00 +0000
@@ -3,13 +3,13 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6import unittest
7
8from storm.zope.interfaces import IResultSet6from storm.zope.interfaces import IResultSet
9from zope.component import getUtility7from zope.component import getUtility
10from zope.security.proxy import removeSecurityProxy8from zope.security.proxy import removeSecurityProxy
119
10from canonical.config import config
12from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities11from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
12from canonical.launchpad.webapp.batching import BatchNavigator
13from canonical.testing import LaunchpadZopelessLayer13from canonical.testing import LaunchpadZopelessLayer
14from lp.testing import TestCaseWithFactory14from lp.testing import TestCaseWithFactory
15from lp.testing.views import create_initialized_view15from lp.testing.views import create_initialized_view
@@ -44,5 +44,43 @@
44 self.assertEqual(view.needs_linking, None)44 self.assertEqual(view.needs_linking, None)
4545
4646
47def test_suite():47class TestMilestoneBatchNavigatorAttribute(TestCaseWithFactory):
48 return unittest.TestLoader().loadTestsFromName(__name__)48 """Test the series.milestone_batch_navigator attribute."""
49
50 layer = LaunchpadZopelessLayer
51
52 def test_distroseries_milestone_batch_navigator(self):
53 ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
54 distroseries = self.factory.makeDistroSeries(distribution=ubuntu)
55 for name in ('a', 'b', 'c', 'd'):
56 distroseries.newMilestone(name)
57 view = create_initialized_view(distroseries, name='+index')
58 self._check_milestone_batch_navigator(view)
59
60 def test_productseries_milestone_batch_navigator(self):
61 product = self.factory.makeProduct()
62 for name in ('a', 'b', 'c', 'd'):
63 product.development_focus.newMilestone(name)
64 view = create_initialized_view(
65 product.development_focus, name='+index')
66 self._check_milestone_batch_navigator(view)
67
68 def _check_milestone_batch_navigator(self, view):
69 config.push('default-batch-size', """
70 [launchpad]
71 default_batch_size: 2
72 """)
73 self.assert_(
74 isinstance(view.milestone_batch_navigator, BatchNavigator),
75 'milestone_batch_navigator is not a BatchNavigator object: %r'
76 % view.milestone_batch_navigator)
77 self.assertEqual(4, view.milestone_batch_navigator.batch.total())
78 expected = [
79 'd',
80 'c',
81 ]
82 milestone_names = [
83 item.name
84 for item in view.milestone_batch_navigator.currentBatch()]
85 self.assertEqual(expected, milestone_names)
86 config.pop('default-batch-size')
4987
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2010-08-26 23:54:09 +0000
+++ lib/lp/registry/interfaces/product.py 2010-09-01 00:31:00 +0000
@@ -786,7 +786,10 @@
786 @export_read_operation()786 @export_read_operation()
787 @export_operation_as('get_timeline')787 @export_operation_as('get_timeline')
788 def getTimeline(include_inactive):788 def getTimeline(include_inactive):
789 """Return basic timeline data useful for creating a diagram."""789 """Return basic timeline data useful for creating a diagram.
790
791 The number of milestones returned per series is limited.
792 """
790793
791794
792class IProduct(795class IProduct(
793796
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/interfaces/productseries.py 2010-09-01 00:31:00 +0000
@@ -308,7 +308,10 @@
308 @export_read_operation()308 @export_read_operation()
309 @export_operation_as('get_timeline')309 @export_operation_as('get_timeline')
310 def getTimeline(include_inactive):310 def getTimeline(include_inactive):
311 """Return basic timeline data useful for creating a diagram."""311 """Return basic timeline data useful for creating a diagram.
312
313 The number of milestones returned is limited.
314 """
312315
313316
314class IProductSeries(IProductSeriesEditRestricted, IProductSeriesPublic,317class IProductSeries(IProductSeriesEditRestricted, IProductSeriesPublic,
315318
=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py 2010-08-24 19:06:33 +0000
+++ lib/lp/registry/model/milestone.py 2010-09-01 00:31:00 +0000
@@ -97,7 +97,7 @@
9797
98 @property98 @property
99 def has_milestones(self):99 def has_milestones(self):
100 return self.all_milestones.any() is not None100 return not self.all_milestones.is_empty()
101101
102 @property102 @property
103 def all_milestones(self):103 def all_milestones(self):
@@ -293,7 +293,10 @@
293 def __init__(self, target, name, dateexpected, active):293 def __init__(self, target, name, dateexpected, active):
294 self.name = name294 self.name = name
295 self.code_name = None295 self.code_name = None
296 self.id = None296 # The id is necessary for generating a unique memcache key
297 # in a page template loop. The ProjectMilestone.id is passed
298 # in as the third argument to the "cache" TALes.
299 self.id = 'ProjectGroup:%s/Milestone:%s' % (target.name, name)
297 self.code_name = None300 self.code_name = None
298 self.product = None301 self.product = None
299 self.distribution = None302 self.distribution = None
300303
=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/model/productseries.py 2010-09-01 00:31:00 +0000
@@ -97,6 +97,9 @@
97 )97 )
9898
9999
100MAX_TIMELINE_MILESTONES = 20
101
102
100def landmark_key(landmark):103def landmark_key(landmark):
101 """Sorts landmarks by date and name."""104 """Sorts landmarks by date and name."""
102 if landmark['date'] is None:105 if landmark['date'] is None:
@@ -543,7 +546,7 @@
543546
544 def getTimeline(self, include_inactive=False):547 def getTimeline(self, include_inactive=False):
545 landmarks = []548 landmarks = []
546 for milestone in self.all_milestones:549 for milestone in self.all_milestones[:MAX_TIMELINE_MILESTONES]:
547 if milestone.product_release is None:550 if milestone.product_release is None:
548 # Skip inactive milestones, but include releases,551 # Skip inactive milestones, but include releases,
549 # even if include_inactive is False.552 # even if include_inactive is False.
550553
=== modified file 'lib/lp/registry/templates/productseries-milestone-table-row.pt'
--- lib/lp/registry/templates/productseries-milestone-table-row.pt 2010-05-27 04:10:17 +0000
+++ lib/lp/registry/templates/productseries-milestone-table-row.pt 2010-09-01 00:31:00 +0000
@@ -5,7 +5,7 @@
5 define="milestone_menu view/milestone/menu:overview;5 define="milestone_menu view/milestone/menu:overview;
6 milestone view/milestone;6 milestone view/milestone;
7 release view/release"7 release view/release"
8 tal:content="cache:private, 1 hour">8 tal:content="cache:private, 1 hour, milestone/id">
9 <tr>9 <tr>
10 <td>10 <td>
11 <img src="/@@/milestone" alt="" />11 <img src="/@@/milestone" alt="" />
1212
=== modified file 'lib/lp/registry/templates/productseries-table-releases.pt'
--- lib/lp/registry/templates/productseries-table-releases.pt 2010-05-17 20:09:03 +0000
+++ lib/lp/registry/templates/productseries-table-releases.pt 2010-09-01 00:31:00 +0000
@@ -2,8 +2,10 @@
2 xmlns:tal="http://xml.zope.org/namespaces/tal"2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
5 omit-tag="">5 tal:content="cache:anonymous, 1 hour">
66
7<tal:navigation_top
8 replace="structure view/milestone_batch_navigator/@@+navigation-links-upper" />
7<table class="listing"9<table class="listing"
8 tal:attributes="id context/name/fmt:css-id/series-;10 tal:attributes="id context/name/fmt:css-id/series-;
9 class view/milestone_table_class">11 class view/milestone_table_class">
@@ -16,9 +18,11 @@
16 </tr>18 </tr>
17 </thead>19 </thead>
18 <tbody id="milestone-rows">20 <tbody id="milestone-rows">
19 <tal:row repeat="milestone context/all_milestones">21 <tal:row repeat="milestone view/milestone_batch_navigator/currentBatch">
20 <tal:milestone replace="structure milestone/@@+productseries-table-row" />22 <tal:milestone replace="structure milestone/@@+productseries-table-row" />
21 </tal:row>23 </tal:row>
22 </tbody>24 </tbody>
23</table>25</table>
26<tal:navigation_bottom
27 replace="structure view/milestone_batch_navigator/@@+navigation-links-lower" />
24</tal:root>28</tal:root>
2529
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2010-08-23 16:05:36 +0000
+++ lib/lp/registry/tests/test_product.py 2010-09-01 00:31:00 +0000
@@ -92,6 +92,50 @@
92 expected,92 expected,
93 list(product.getMilestonesAndReleases()))93 list(product.getMilestonesAndReleases()))
9494
95 def test_getTimeline_limit(self):
96 # Only 20 milestones/releases per series should be included in the
97 # getTimeline() results. The results are sorted by
98 # descending dateexpected and name, so the presumed latest
99 # milestones should be included.
100 product = self.factory.makeProduct(name='foo')
101 for i in range(25):
102 milestone_list = self.factory.makeMilestone(
103 product=product,
104 productseries=product.development_focus,
105 name=str(i))
106
107 # 0 through 4 should not be in the list.
108 expected_milestones = [
109 '/foo/+milestone/24',
110 '/foo/+milestone/23',
111 '/foo/+milestone/22',
112 '/foo/+milestone/21',
113 '/foo/+milestone/20',
114 '/foo/+milestone/19',
115 '/foo/+milestone/18',
116 '/foo/+milestone/17',
117 '/foo/+milestone/16',
118 '/foo/+milestone/15',
119 '/foo/+milestone/14',
120 '/foo/+milestone/13',
121 '/foo/+milestone/12',
122 '/foo/+milestone/11',
123 '/foo/+milestone/10',
124 '/foo/+milestone/9',
125 '/foo/+milestone/8',
126 '/foo/+milestone/7',
127 '/foo/+milestone/6',
128 '/foo/+milestone/5',
129 ]
130
131 [series] = product.getTimeline()
132 timeline_milestones = [
133 landmark['uri']
134 for landmark in series['landmarks']]
135 self.assertEqual(
136 expected_milestones,
137 timeline_milestones)
138
95139
96class TestProductFiles(unittest.TestCase):140class TestProductFiles(unittest.TestCase):
97 """Tests for downloadable product files."""141 """Tests for downloadable product files."""

Subscribers

People subscribed via source and target branches

to status/vote changes: