Merge lp:~sinzui/launchpad/series-performance into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

This is my branch to address +series timeouts.

    lp:~sinzui/launchpad/series-performance
    Diff size: 120
    Launchpad bug: https://bugs.launchpad.net/bugs/396135
    Test command: ./bin/test -vvt "productseries-views|xx-productseries-series"
    Pre-implementation: Edwin
    Target release: 2.2.7

= +series timeouts =

The longest step is rendering the +series page is checking the privacy
of bugs. The +series page presents a count of all bug status for a series.
Since bug privacy is an expensive feature to re-implement, the +series page
must be more selective about which bugs to count.

Obsolete series are less interesting that other series. They do not need
bug status counts. In the bzr/+series example, there are 29 obsolete series
and 7 series that are other statuses. There are about 200 privacy checks
that can be avoided, as well as SQL calls to get that information.

== Rules ==

Add a condition to the productseries-status.pt template to display the bug
status only when the series is not obsolete

== QA ==

    * Visit https://staging.launchpad.net/bzr/+series
    * Verify that no obsolete series has a "Bugs targeted:" line
    * Load the page repeatedly to verify that the page does no timeout.

== Lint ==

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

Linting changed files:
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/productseries-views.txt
  lib/lp/registry/stories/productseries/xx-productseries-series.txt
  lib/lp/registry/templates/productseries-status.pt

== Test ==

    * lib/lp/registry/browser/tests/productseries-views.txt
      * Added a test to verify that the view provides an is_obsolete
        property.
    * lib/lp/registry/stories/productseries/xx-productseries-series.txt
      * Revised a test to show that obsolete series do not have a
        "Bugs targeted:" line.

== Implementation ==

    * lib/lp/registry/browser/productseries.py
      * Added the is_obsolete property for the template.
    * lib/lp/registry/templates/productseries-status.pt
      * Use the is_obsolete to avoid rendering the "Bugs targeted:" section.

[Brad Crittenden (bac)]

This fix looks great Curtis.

Two minor fixes:

36: typo: as other

106: indent the code surrounded by your new tag

review: Approve

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2009-07-17 00:26:05 +0000
+++ lib/lp/registry/browser/productseries.py	2009-07-21 16:30:28 +0000
@@ -47,14 +47,13 @@
 from lp.code.interfaces.codeimport import (
     ICodeImportSet)
 from lp.services.worlddata.interfaces.country import ICountry
-from lp.bugs.interfaces.bugtask import BugTaskSearchParams, IBugTaskSet
+from lp.bugs.interfaces.bugtask import IBugTaskSet
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.browser import StatusCount
 from lp.translations.interfaces.potemplate import IPOTemplateSet
 from lp.translations.interfaces.productserieslanguage import (
     IProductSeriesLanguageSet)
 from lp.services.worlddata.interfaces.language import ILanguageSet
-from canonical.launchpad.searchbuilder import any
 from canonical.launchpad.webapp import (
     action, ApplicationMenu, canonical_url, custom_widget,
     enabled_with_permission, LaunchpadEditFormView,
@@ -69,6 +68,7 @@
 
 from lp.registry.browser import (
     MilestoneOverlayMixin, RegistryDeleteViewMixin)
+from lp.registry.interfaces.distroseries import DistroSeriesStatus
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.sourcepackagename import (
     ISourcePackageNameSet)
@@ -417,6 +417,17 @@
         return (branch is not None and
                 check_permission('launchpad.View', branch))
 
+    @property
+    def is_obsolete(self):
+        """Return True if the series is OBSOLETE"
+
+        Obsolete series do not need to display as much information of other
+        series. Accessing private bugs is an expensive operation and showing
+        them for obsolete series can be a problem if many series are being
+        displayed.
+        """
+        return self.context.status == DistroSeriesStatus.OBSOLETE
+
     @cachedproperty
     def bugtask_status_counts(self):
         """A list StatusCounts summarising the targeted bugtasks."""
 
=== modified file 'lib/lp/registry/browser/tests/productseries-views.txt'
--- lib/lp/registry/browser/tests/productseries-views.txt	2009-07-07 11:24:01 +0000
+++ lib/lp/registry/browser/tests/productseries-views.txt	2009-07-21 16:30:28 +0000
@@ -60,6 +60,22 @@
     >>> print view.milestone_table_class
     listing
 
+Obsolete series are less interesting that other series. The ProductSeriesView
+has an is_obsolete property that templates can check when choosing the content
+to display.
+
+    >>> from lp.registry.interfaces.distroseries import DistroSeriesStatus
+
+    >>> print series.status
+    Active Development
+    >>> view.is_obsolete
+    False
+
+    >>> series.status = DistroSeriesStatus.OBSOLETE
+    >>> view = create_view(series, '+index')
+    >>> view.is_obsolete
+    True
+
 
 == Delete ProductSeries ==
 
 
=== modified file 'lib/lp/registry/stories/productseries/xx-productseries-series.txt'
--- lib/lp/registry/stories/productseries/xx-productseries-series.txt	2009-06-29 23:15:46 +0000
+++ lib/lp/registry/stories/productseries/xx-productseries-series.txt	2009-07-21 16:30:28 +0000
@@ -54,12 +54,12 @@
     >>> print series_1_0['class']
     unhighlighted series
 
-Any user can see that obsolete series are dimmed.
+Any user can see that obsolete series are dimmed. Obsolete series do not
+show bug status counts because it is expensive to retrieve the information.
 
     >>> series_xxx = find_tag_by_id(content, 'series-xxx')
     >>> print extract_text(series_xxx)
     xxx series Obsolete
-    Bugs targeted: None
     Blueprints targeted: None
     Use true GTK UI.
 
 
=== modified file 'lib/lp/registry/templates/productseries-status.pt'
--- lib/lp/registry/templates/productseries-status.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/productseries-status.pt	2009-07-21 16:30:28 +0000
@@ -6,12 +6,14 @@
   tal:define="
     series context;
     is_focus context/is_development_focus;
-    bug_count_status view/bugtask_status_counts;
     spec_count_status view/specification_status_counts;"
   >
   <metal:series use-macro="series/@@+macros/detailed_display">
     <div metal:fill-slot="extra">
       <div>
+        <tal:not-obsolete
+          condition="not: view/is_obsolete"
+          define="bug_count_status view/bugtask_status_counts;">
         Bugs targeted:
         <tal:statuses repeat="count_status bug_count_status">
           <span tal:attributes="class string:status${count_status/status/name}">
@@ -24,6 +26,7 @@
           None
         </tal:no-statuses>
         <br />
+        </tal:not-obsolete>
         Blueprints targeted:
         <tal:statuses repeat="count_status spec_count_status">
           <span tal:attributes="class string:specdelivery${count_status/status/name}">