launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03847
[Merge] lp:~lifeless/launchpad/bug-793809 into lp:launchpad
Robert Collins has proposed merging lp:~lifeless/launchpad/bug-793809 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #793809 in Launchpad itself: "Distribution:+bugtarget-portlet-tags-content timeouts"
https://bugs.launchpad.net/launchpad/+bug/793809
For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-793809/+merge/63635
Per the linked bug generating bug tag portlets is extraordinarily slow. This is meant to be addressed by querying bugsummary, which this branch implements.
I have changed the interface to match our needs allowing a faster query (the with clause on teams for visibility).
Other than that I can see a path to reduce the duplicate code that we use here, but its unrelated to fixing this bug, so I've focused on getting the bug fixed ;)
--
https://code.launchpad.net/~lifeless/launchpad/bug-793809/+merge/63635
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-793809 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py 2011-06-06 06:44:23 +0000
+++ lib/lp/bugs/browser/bugtarget.py 2011-06-07 03:55:55 +0000
@@ -1405,19 +1405,8 @@
def tags_cloud_data(self):
"""The data for rendering a tags cloud"""
official_tags = self.context.official_bug_tags
-
- # Construct a dict of official and top 10 tags.
- # getUsedBugTagsWithOpenCounts is expensive, so do the union in
- # SQL. Also preseed with 0 for all the official tags, as gUBTWOC
- # won't return unused ones.
- top_ten = removeSecurityProxy(
- self.context.getUsedBugTagsWithOpenCounts(self.user)[:10])
- official = removeSecurityProxy(
- self.context.getUsedBugTagsWithOpenCounts(
- self.user, official_tags))
- tags = dict((tag, 0) for tag in official_tags)
- tags.update(dict(top_ten.union(official)))
-
+ tags = self.context.getUsedBugTagsWithOpenCounts(self.user, 10,
+ official_tags)
max_count = float(max([1] + tags.values()))
return sorted(
@@ -1462,8 +1451,8 @@
@property
def tags_js_data(self):
"""Return the JSON representation of the bug tags."""
- used_tags = dict(self.context.getUsedBugTagsWithOpenCounts(self.user))
- official_tags = list(self.context.official_bug_tags)
+ used_tags = self.context.getUsedBugTagsWithOpenCounts(self.user)
+ official_tags = self.context.official_bug_tags
return """<script type="text/javascript">
var used_bug_tags = %s;
var official_bug_tags = %s;
=== modified file 'lib/lp/bugs/doc/bug-tags.txt'
--- lib/lp/bugs/doc/bug-tags.txt 2011-04-12 06:40:51 +0000
+++ lib/lp/bugs/doc/bug-tags.txt 2011-06-07 03:55:55 +0000
@@ -332,35 +332,34 @@
We can also get all the used tags, together with the number of open
bugs each tag has. Only tags having open bugs are returned.
- >>> list(firefox.getUsedBugTagsWithOpenCounts(None))
- [(u'doc', 1L), (u'sco', 1L), (u'svg', 1L)]
-
- >>> list(mozilla.getUsedBugTagsWithOpenCounts(None))
- [(u'doc', 1L), (u'sco', 1L), (u'svg', 1L)]
-
- >>> list(ubuntu.getUsedBugTagsWithOpenCounts(None))
+ >>> sorted(firefox.getUsedBugTagsWithOpenCounts(None).items())
+ [(u'doc', 1L), (u'sco', 1L), (u'svg', 1L)]
+
+ >>> sorted(mozilla.getUsedBugTagsWithOpenCounts(None).items())
+ [(u'doc', 1L), (u'sco', 1L), (u'svg', 1L)]
+
+ >>> sorted(ubuntu.getUsedBugTagsWithOpenCounts(None).items())
[(u'crash', 2L), (u'dataloss', 1L), (u'pebcak', 1L),
(u'sco', 1L), (u'svg', 1L)]
-We can even ask for the counts for a particular set of tags.
+We can require that some tags be included in the output even when limiting the
+results.
- >>> list(ubuntu.getUsedBugTagsWithOpenCounts(
- ... None, wanted_tags=['pebcak', 'svg', 'fake']))
- [(u'pebcak', 1L), (u'svg', 1L)]
- >>> list(ubuntu.getUsedBugTagsWithOpenCounts(None, wanted_tags=[]))
- []
+ >>> sorted(ubuntu.getUsedBugTagsWithOpenCounts(None,
+ ... tag_limit=1, include_tags=[u'pebcak', u'svg', u'fake']).items())
+ [(u'crash', 2L), (u'fake', 0), (u'pebcak', 1L), (u'svg', 1L)]
Source packages are a bit special, they return all the tags that are
used in the whole distribution, while the bug count includes only bugs
in the specific package.
- >>> list(ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None))
- [(u'crash', 1L)]
+ >>> ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None)
+ {u'crash': 1L}
- >>> list(debian_woody.getUsedBugTagsWithOpenCounts(None))
+ >>> sorted(debian_woody.getUsedBugTagsWithOpenCounts(None).items())
[(u'dataloss', 1L), (u'layout-test', 1L), (u'pebcak', 1L)]
- >>> list(debian_woody_firefox.getUsedBugTagsWithOpenCounts(None))
+ >>> sorted(debian_woody_firefox.getUsedBugTagsWithOpenCounts(None).items())
[(u'dataloss', 1L), (u'layout-test', 1L), (u'pebcak', 1L)]
Only bugs that the supplied user has access to will be counted:
@@ -370,14 +369,14 @@
True
>>> flush_database_updates()
- >>> list(ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None))
- []
+ >>> ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None)
+ {}
>>> sample_person = getUtility(ILaunchBag).user
>>> bug_nine.isSubscribed(sample_person)
True
- >>> list(ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(sample_person))
- [(u'crash', 1L)]
+ >>> ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(sample_person)
+ {u'crash': 1L}
When context doesn't have any tags getUsedBugTags() returns a empty list.
=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
--- lib/lp/bugs/interfaces/bugtarget.py 2011-04-12 06:21:39 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py 2011-06-07 03:55:55 +0000
@@ -403,15 +403,17 @@
def getUsedBugTags():
"""Return the tags used by the context as a sorted list of strings."""
- def getUsedBugTagsWithOpenCounts(user, wanted_tags=None):
+ def getUsedBugTagsWithOpenCounts(user, tag_limit=0, include_tags=None):
"""Return name and bug count of tags having open bugs.
- It returns a list of tuples contining the tag name, and the
- number of open bugs having that tag. Only the bugs that the user
- has permission to see are counted, and only tags having open
- bugs will be returned.
-
- If wanted_tags is specified, only those tags will be returned.
+ :param user: The user who wants the report.
+ :param tag_limit: The number of tags to return (excludes those found by
+ matching include_tags). If 0 then all tags are returned. If
+ non-zero then the most frequently used tags are returned.
+ :param include_tags: A list of string tags to return irrespective of
+ usage. Tags in this list that have no open bugs are returned with a
+ count of 0. May be None if there are tags to require inclusion of.
+ :return: A dict from tag -> count.
"""
def _getOfficialTagClause():
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-06-03 10:38:25 +0000
+++ lib/lp/bugs/model/bug.py 2011-06-07 03:55:55 +0000
@@ -63,6 +63,7 @@
Select,
SQL,
SQLRaw,
+ Sum,
Union,
)
from storm.info import ClassAlias
@@ -242,11 +243,6 @@
tag = StringCol(notNull=True)
-# We need to always use the same Count instance or the
-# get_bug_tags_open_count is not UNIONable.
-tag_count_columns = (BugTag.tag, Count())
-
-
def get_bug_tags(context_clause):
"""Return all the bug tags as a list of strings.
@@ -266,39 +262,56 @@
return shortlist([row[0] for row in cur.fetchall()])
-def get_bug_tags_open_count(context_condition, user, wanted_tags=None):
- """Return all the used bug tags with their open bug count.
-
+def get_bug_tags_open_count(context_condition, user, tag_limit=0,
+ include_tags=None):
+ """Worker for IBugTarget.getUsedBugTagsWithOpenCounts.
+
+ See `IBugTarget` for details.
+
+ The only change is that this function takes a SQL expression for limiting
+ the found tags.
:param context_condition: A Storm SQL expression, limiting the
used tags to a specific context. Only the BugTask table may be
used to choose the context.
- :param user: The user performing the search.
- :param wanted_tags: A set of tags within which to restrict the search.
-
- :return: A list of tuples, (tag name, open bug count).
"""
- tables = (
- BugTag,
- Join(BugTask, BugTask.bugID == BugTag.bugID),
- )
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
+ tags = {}
+ if include_tags:
+ tags = dict((tag, 0) for tag in include_tags)
+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+ admin_team = getUtility(ILaunchpadCelebrities).admin
+ if user is not None and not user.inTeam(admin_team):
+ store = store.with_(SQL(
+ "teams AS ("
+ "SELECT team from TeamParticipation WHERE person=%s)" % user.id))
where_conditions = [
- BugTask.status.is_in(UNRESOLVED_BUGTASK_STATUSES),
+ BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES),
+ BugSummary.tag != None,
context_condition,
]
- if wanted_tags is not None:
- where_conditions.append(BugTag.tag.is_in(wanted_tags))
- privacy_filter = get_bug_privacy_filter(user)
- if privacy_filter:
- # The EXISTS sub-select avoids a join against Bug, improving
- # performance significantly.
+ if user is None:
+ where_conditions.append(BugSummary.viewed_by_id == None)
+ elif not user.inTeam(admin_team):
where_conditions.append(
- Exists(Select(
- columns=[True], tables=[Bug],
- where=And(Bug.id == BugTag.bugID, SQLRaw(privacy_filter)))))
- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
- return store.using(*tables).find(
- tag_count_columns, *where_conditions).group_by(BugTag.tag).order_by(
- Desc(Count()), BugTag.tag)
+ Or(
+ BugSummary.viewed_by_id == None,
+ BugSummary.viewed_by_id.is_in(SQL("SELECT team FROM teams"))
+ ))
+ tag_count_columns = (BugSummary.tag, Sum(BugSummary.count))
+ # Always query for used
+ def _query(*args):
+ return store.find(tag_count_columns, *(where_conditions + list(args))
+ ).group_by(BugSummary.tag).order_by(
+ Desc(Sum(BugSummary.count)), BugSummary.tag)
+ used = _query()
+ if tag_limit:
+ used = used[:tag_limit]
+ if include_tags:
+ # Union in a query for just include_tags.
+ used = used.union(_query(BugSummary.tag.is_in(include_tags)))
+ tags.update(dict(used))
+ return tags
class BugBecameQuestionEvent:
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2011-05-27 21:12:25 +0000
+++ lib/lp/registry/model/distribution.py 2011-06-07 03:55:55 +0000
@@ -646,10 +646,14 @@
"""See `IBugTarget`."""
return get_bug_tags("BugTask.distribution = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
- """See `IBugTarget`."""
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ """See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
return get_bug_tags_open_count(
- BugTask.distribution == self, user, wanted_tags=wanted_tags)
+ And(BugSummary.distribution_id == self.id,
+ BugSummary.sourcepackagename_id == None),
+ user, tag_limit=tag_limit, include_tags=include_tags)
def getMirrorByName(self, name):
"""See `IDistribution`."""
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2011-05-12 04:18:32 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2011-06-07 03:55:55 +0000
@@ -486,12 +486,14 @@
"""See `IBugTarget`."""
return self.distribution.getUsedBugTags()
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
- """See `IBugTarget`."""
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ """See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
return get_bug_tags_open_count(
- And(BugTask.distribution == self.distribution,
- BugTask.sourcepackagename == self.sourcepackagename),
- user, wanted_tags=wanted_tags)
+ And(BugSummary.distribution == self.distribution,
+ BugSummary.sourcepackagename == self.sourcepackagename),
+ user, tag_limit=tag_limit, include_tags=include_tags)
def _getOfficialTagClause(self):
return self.distribution._getOfficialTagClause()
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2011-05-31 15:45:19 +0000
+++ lib/lp/registry/model/distroseries.py 2011-06-07 03:55:55 +0000
@@ -28,6 +28,7 @@
StringCol,
)
from storm.locals import (
+ And,
Desc,
Join,
SQL,
@@ -853,10 +854,16 @@
"""See `IHasBugs`."""
return get_bug_tags("BugTask.distroseries = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
- """See `IHasBugs`."""
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ """See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
return get_bug_tags_open_count(
- BugTask.distroseries == self, user, wanted_tags=wanted_tags)
+ And(
+ BugSummary.distroseries_id == self.id,
+ BugSummary.sourcepackagename_id == None
+ ),
+ user, tag_limit=tag_limit, include_tags=include_tags)
@property
def has_any_specifications(self):
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2011-05-27 21:12:25 +0000
+++ lib/lp/registry/model/product.py 2011-06-07 03:55:55 +0000
@@ -799,10 +799,13 @@
"""See `IBugTarget`."""
return get_bug_tags("BugTask.product = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
- """See `IBugTarget`."""
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ """See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
return get_bug_tags_open_count(
- BugTask.product == self, user, wanted_tags=wanted_tags)
+ BugSummary.product_id == self.id,
+ user, tag_limit=tag_limit, include_tags=include_tags)
series = SQLMultipleJoin('ProductSeries', joinColumn='product',
orderBy='name')
=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2011-05-27 21:12:25 +0000
+++ lib/lp/registry/model/productseries.py 2011-06-07 03:55:55 +0000
@@ -446,10 +446,13 @@
"""See IBugTarget."""
return get_bug_tags("BugTask.productseries = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
"""See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
return get_bug_tags_open_count(
- BugTask.productseries == self, user, wanted_tags=wanted_tags)
+ BugSummary.productseries_id == self.id, user, tag_limit=tag_limit,
+ include_tags=include_tags)
def createBug(self, bug_params):
"""See IBugTarget."""
=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py 2011-04-26 16:22:11 +0000
+++ lib/lp/registry/model/projectgroup.py 2011-06-07 03:55:55 +0000
@@ -343,14 +343,16 @@
return get_bug_tags(
"BugTask.product IN (%s)" % ",".join(product_ids))
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
- """See `IHasBugs`."""
- if not self.products:
- return []
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ """See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
product_ids = [product.id for product in self.products]
+ if not product_ids:
+ return {}
return get_bug_tags_open_count(
- BugTask.productID.is_in(product_ids), user,
- wanted_tags=wanted_tags)
+ BugSummary.product_id.is_in(product_ids),
+ user, tag_limit=tag_limit, include_tags=include_tags)
def _getBugTaskContextClause(self):
"""See `HasBugsBase`."""
=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py 2011-05-14 15:03:04 +0000
+++ lib/lp/registry/model/sourcepackage.py 2011-06-07 03:55:55 +0000
@@ -494,12 +494,14 @@
"""See `IBugTarget`."""
return self.distroseries.getUsedBugTags()
- def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
- """See `IBugTarget`."""
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ """See IBugTarget."""
+ # Circular fail.
+ from lp.bugs.model.bugsummary import BugSummary
return get_bug_tags_open_count(
- And(BugTask.distroseries == self.distroseries,
- BugTask.sourcepackagename == self.sourcepackagename),
- user, wanted_tags=wanted_tags)
+ And(BugSummary.distroseries == self.distroseries,
+ BugSummary.sourcepackagename == self.sourcepackagename),
+ user, tag_limit=tag_limit, include_tags=include_tags)
@property
def max_bug_heat(self):
