launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13351
[Merge] lp:~wallyworld/launchpad/projectgroup-timeout-1016156 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/projectgroup-timeout-1016156 into lp:launchpad.
Commit message:
Rework some entity properties to reduce query counts on the projectgroup index page and others.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1016156 in Launchpad itself: "ProjectGroup:+index timeout due to slow query of subprojects"
https://bugs.launchpad.net/launchpad/+bug/1016156
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/projectgroup-timeout-1016156/+merge/129337
== Implementation ==
The entity domain objects had (non-cached) properties which return result sets. So every time the property was referenced via a TAL expression, the query was re-run. I identified some core properties on various classes which are implicated on the project group index page, and also other pillar pages. These were converted to cached properties and listified. The result set creation was moved off to a separate getter if required.
As an example, for one project group index page, query count drops in half from the original number of about 70.
== Tests ==
A generic, internal optimisation, so rely on existing tests.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/answers/browser/faqcollection.py
lib/lp/answers/browser/questiontarget.py
lib/lp/blueprints/model/sprint.py
lib/lp/registry/browser/announcement.py
lib/lp/registry/browser/project.py
lib/lp/registry/doc/projectgroup.txt
lib/lp/registry/interfaces/projectgroup.py
lib/lp/registry/model/projectgroup.py
lib/lp/registry/templates/hasannouncements-portlet-latest.pt
--
https://code.launchpad.net/~wallyworld/launchpad/projectgroup-timeout-1016156/+merge/129337
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/projectgroup-timeout-1016156 into lp:launchpad.
=== modified file 'lib/lp/answers/browser/faqcollection.py'
--- lib/lp/answers/browser/faqcollection.py 2012-01-01 02:58:52 +0000
+++ lib/lp/answers/browser/faqcollection.py 2012-10-12 02:42:21 +0000
@@ -127,7 +127,7 @@
quantity = 5
faqs = self.context.searchFAQs(
search_text=self.search_text, sort=FAQSort.NEWEST_FIRST)
- return faqs[:quantity]
+ return list(faqs)[:quantity]
@safe_action
@action(_('Search'), name='search')
=== modified file 'lib/lp/answers/browser/questiontarget.py'
--- lib/lp/answers/browser/questiontarget.py 2012-07-05 00:47:31 +0000
+++ lib/lp/answers/browser/questiontarget.py 2012-10-12 02:42:21 +0000
@@ -167,7 +167,7 @@
is used by the +portlet-latestquestions view.
"""
question_collection = IQuestionCollection(self.context)
- return question_collection.searchQuestions()[:quantity]
+ return list(question_collection.searchQuestions())[:quantity]
class QuestionCollectionOpenCountView:
=== modified file 'lib/lp/blueprints/model/sprint.py'
--- lib/lp/blueprints/model/sprint.py 2012-09-27 18:50:38 +0000
+++ lib/lp/blueprints/model/sprint.py 2012-10-12 02:42:21 +0000
@@ -60,6 +60,7 @@
SQLBase,
)
from lp.services.database.stormexpr import fti_search
+from lp.services.propertycache import cachedproperty
class Sprint(SQLBase, HasDriversMixin, HasSpecificationsMixin):
@@ -361,22 +362,28 @@
quote(SprintSpecificationStatus.ACCEPTED))
return query, ['Specification', 'SprintSpecification']
- @property
+ def getSprints(self):
+ query, tables = self._getBaseQueryAndClauseTablesForQueryingSprints()
+ return Sprint.select(
+ query, clauseTables=tables, orderBy='-time_starts', distinct=True)
+
+ @cachedproperty
def sprints(self):
"""See IHasSprints."""
- query, tables = self._getBaseQueryAndClauseTablesForQueryingSprints()
- return Sprint.select(
- query, clauseTables=tables, orderBy='-time_starts', distinct=True)
+ return list(self.getSprints())
- @property
- def coming_sprints(self):
- """See IHasSprints."""
+ def getComingSprings(self):
query, tables = self._getBaseQueryAndClauseTablesForQueryingSprints()
query += " AND Sprint.time_ends > 'NOW'"
return Sprint.select(
query, clauseTables=tables, orderBy='time_starts',
distinct=True, limit=5)
+ @cachedproperty
+ def coming_sprints(self):
+ """See IHasSprints."""
+ return list(self.getComingSprings())
+
@property
def past_sprints(self):
"""See IHasSprints."""
=== modified file 'lib/lp/registry/browser/announcement.py'
--- lib/lp/registry/browser/announcement.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/announcement.py 2012-10-12 02:42:21 +0000
@@ -298,18 +298,22 @@
@cachedproperty
def announcements(self):
published_only = not check_permission('launchpad.Edit', self.context)
- return self.context.getAnnouncements(
- limit=None, published_only=published_only)
+ return list(self.context.getAnnouncements(
+ limit=None, published_only=published_only))
@cachedproperty
def latest_announcements(self):
published_only = not check_permission('launchpad.Edit', self.context)
- return self.context.getAnnouncements(
- limit=5, published_only=published_only)
+ return list(self.context.getAnnouncements(
+ limit=5, published_only=published_only))
+
+ @cachedproperty
+ def has_announcements(self):
+ return len(self.latest_announcements) > 0
@cachedproperty
def show_announcements(self):
- return (self.latest_announcements.count() > 0
+ return (len(self.latest_announcements) > 0
or check_permission('launchpad.Edit', self.context))
@cachedproperty
=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py 2012-10-08 01:55:25 +0000
+++ lib/lp/registry/browser/project.py 2012-10-12 02:42:21 +0000
@@ -390,7 +390,7 @@
The number of sub projects can break the preferred layout so the
template may want to plan for a long list.
"""
- return self.context.products.count() > 10
+ return len(self.context.products) > 10
@property
def project_group_milestone_tag(self):
=== modified file 'lib/lp/registry/doc/projectgroup.txt'
--- lib/lp/registry/doc/projectgroup.txt 2012-09-26 20:56:43 +0000
+++ lib/lp/registry/doc/projectgroup.txt 2012-10-12 02:42:21 +0000
@@ -356,7 +356,7 @@
>>> gnome = getUtility(IProjectGroupSet)['gnome']
>>> gnome.has_translatable()
True
- >>> translatables = gnome.translatables()
+ >>> translatables = gnome.translatables
>>> translatables.count()
1
=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
--- lib/lp/registry/interfaces/projectgroup.py 2012-10-08 02:42:05 +0000
+++ lib/lp/registry/interfaces/projectgroup.py 2012-10-12 02:42:21 +0000
@@ -326,19 +326,15 @@
title=u"Search for possible duplicate bugs when a new bug is filed",
required=False, readonly=True)
+ translatables = Attribute(
+ "An iterator over products that are translatable in LP")
+
def getProduct(name):
"""Get a product with name `name`."""
def getConfigurableProducts():
"""Get all products that can be edited by user."""
- def translatables():
- """Return an iterator over products that are translatable in LP.
-
- Only products with IProduct.translations_usage set to
- ServiceUsage.LAUNCHPAD are considered translatable.
- """
-
def has_translatable():
"""Return a boolean showing the existance of translatables products.
"""
=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py 2012-10-08 02:42:05 +0000
+++ lib/lp/registry/model/projectgroup.py 2012-10-12 02:42:21 +0000
@@ -3,6 +3,7 @@
# pylint: disable-msg=E0611,W0212
"""Launchpad ProjectGroup-related Database Table Objects."""
+from lp.services.propertycache import cachedproperty
__metaclass__ = type
__all__ = [
@@ -168,11 +169,14 @@
"""See `IPillar`."""
return "Project Group"
- @property
- def products(self):
+ def getProducts(self):
return Product.selectBy(
project=self, active=True, orderBy='displayname')
+ @cachedproperty
+ def products(self):
+ return list(self.getProducts())
+
def getProduct(self, name):
return Product.selectOneBy(project=self, name=name)
@@ -187,8 +191,12 @@
return [self.driver]
return []
- def translatables(self):
- """See `IProjectGroup`."""
+ def getTranslatables(self):
+ """Return an iterator over products that are translatable in LP.
+
+ Only products with IProduct.translations_usage set to
+ ServiceUsage.LAUNCHPAD are considered translatable.
+ """
store = Store.of(self)
origin = [
Product,
@@ -201,9 +209,13 @@
Product.translations_usage == ServiceUsage.LAUNCHPAD,
).config(distinct=True)
+ @cachedproperty
+ def translatables(self):
+ return list(self.getTranslatables())
+
def has_translatable(self):
"""See `IProjectGroup`."""
- return not self.translatables().is_empty()
+ return self.translatables > 0
def sharesTranslationsWithOtherSide(self, person, language,
sourcepackage=None,
@@ -383,7 +395,7 @@
This is to avoid situations where users try to file bugs against
empty project groups (Malone bug #106523).
"""
- return self.products.count() != 0
+ return len(self.products) != 0
def _getMilestoneCondition(self):
"""See `HasMilestonesMixin`."""
=== modified file 'lib/lp/registry/templates/hasannouncements-portlet-latest.pt'
--- lib/lp/registry/templates/hasannouncements-portlet-latest.pt 2009-08-11 04:35:26 +0000
+++ lib/lp/registry/templates/hasannouncements-portlet-latest.pt 2012-10-12 02:42:21 +0000
@@ -4,7 +4,7 @@
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
omit-tag="">
<tal:news define="announcements view/latest_announcements;
- has_announcements announcements/count;
+ has_announcements view/has_announcements;
overview_menu context/menu:overview">
<div id="portlet-latest-announcements" class="portlet announcements"
tal:condition="view/show_announcements">
Follow ups
