launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03290
[Merge] lp:~wgrant/launchpad/bug-736002 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-736002 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #736002 in Launchpad itself: "Distribution:+bugtarget-portlet-tags-content timeouts"
https://bugs.launchpad.net/launchpad/+bug/736002
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-736002/+merge/57278
This branch fixes tags_cloud_data to to issue only two queries, hopefully minimising timeouts on +bugtarget-portlet-tags-content.
It used to display the official tags and the top 10 unofficial tags. This proves to be fairly expensive, so it will now display the official tags and the top 10 tags, some of which may duplicate the official tag set.
I've changed it to query the context's official tags, then perform a single UNION query to get the top 10 and official tag counts. Both sides of the union are done together in a single pass, and I've also avoided a slow join over Bug, letting the query complete in <4s for Ubuntu on staging.
--
https://code.launchpad.net/~wgrant/launchpad/bug-736002/+merge/57278
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-736002 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py 2011-03-31 08:05:07 +0000
+++ lib/lp/bugs/browser/bugtarget.py 2011-04-12 07:28:38 +0000
@@ -1373,62 +1373,33 @@
# Use path_only here to reduce the size of the rendered page.
return "+bugs?field.tag=%s" % urllib.quote(tag)
- def getUsedBugTagsWithURLs(self):
- """Return the bug tags and their search URLs."""
- bug_tag_counts = self.context.getUsedBugTagsWithOpenCounts(self.user)
- return [
- {'tag': tag, 'count': count, 'url': self._getSearchURL(tag)}
- for tag, count in bug_tag_counts]
-
- @property
- def official_tags(self):
- """Get the official tags to diplay."""
- official_tags = set(self.context.official_bug_tags)
- tags = [tag for tag in self.getUsedBugTagsWithURLs()
- if tag['tag'] in official_tags]
- used_tags = set(tag['tag'] for tag in tags)
- tags.sort(key=itemgetter('count'), reverse=True)
- for tag in sorted(official_tags - used_tags):
- tags.append(
- {'tag': tag, 'count': 0, 'url': self._getSearchURL(tag)})
- return tags
-
- @property
- def other_tags(self):
- """Get the unofficial tags to diplay."""
- official_tags = set(self.context.official_bug_tags)
- tags = [tag for tag in self.getUsedBugTagsWithURLs()
- if tag['tag'] not in official_tags]
- tags.sort(key=itemgetter('count'), reverse=True)
- return tags[:10]
-
@property
def tags_cloud_data(self):
"""The data for rendering a tags cloud"""
- official_tags = list(self.context.official_bug_tags)
- tags = self.getUsedBugTagsWithURLs()
- other_tags = [tag for tag in tags if tag['tag'] not in official_tags]
- popular_tags = [tag['tag'] for tag in sorted(
- other_tags, key=itemgetter('count'), reverse=True)[:10]]
- tags = [
- tag for tag in tags
- if tag['tag'] in official_tags + popular_tags]
- all_tag_dicts = [tag['tag'] for tag in tags]
- for official_tag in official_tags:
- if official_tag not in all_tag_dicts:
- tags.append({
- 'tag': official_tag,
- 'count': 0,
- 'url': "+bugs?field.tag=%s" % urllib.quote(official_tag)})
- max_count = float(max([1] + [tag['count'] for tag in tags]))
- for tag in tags:
- if tag['tag'] in official_tags:
- if tag['count'] == 0:
- tag['factor'] = 1.5
- else:
- tag['factor'] = 1.5 + (tag['count'] / max_count)
- else:
- tag['factor'] = 1 + (tag['count'] / max_count)
+ 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.
+ raw_tags = dict((tag, 0) for tag in official_tags)
+ top_ten = removeSecurityProxy(
+ self.context.getUsedBugTagsWithOpenCounts(self.user)[:10])
+ official = removeSecurityProxy(
+ self.context.getUsedBugTagsWithOpenCounts(
+ self.user, official_tags))
+ raw_tags.update(dict(top_ten.union(official)))
+
+ max_count = float(max([1] + raw_tags.values()))
+
+ tags = []
+ for tag, count in raw_tags.iteritems():
+ tag_dict = dict(
+ tag=tag, count=count, url=self._getSearchURL(tag),
+ factor=1 + (count / max_count))
+ if tag in official_tags:
+ tag_dict['factor'] += 0.5
+ tags.append(tag_dict)
return sorted(tags, key=itemgetter('tag'))
@property
=== modified file 'lib/lp/bugs/doc/bug-tags.txt'
--- lib/lp/bugs/doc/bug-tags.txt 2011-01-11 09:40:05 +0000
+++ lib/lp/bugs/doc/bug-tags.txt 2011-04-12 07:28:38 +0000
@@ -332,27 +332,35 @@
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.
- >>> firefox.getUsedBugTagsWithOpenCounts(None)
- [(u'doc', 1L), (u'sco', 1L), (u'svg', 1L)]
-
- >>> mozilla.getUsedBugTagsWithOpenCounts(None)
- [(u'doc', 1L), (u'sco', 1L), (u'svg', 1L)]
-
- >>> ubuntu.getUsedBugTagsWithOpenCounts(None)
+ >>> 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))
[(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.
+
+ >>> list(ubuntu.getUsedBugTagsWithOpenCounts(
+ ... None, wanted_tags=['pebcak', 'svg', 'fake']))
+ [(u'pebcak', 1L), (u'svg', 1L)]
+ >>> list(ubuntu.getUsedBugTagsWithOpenCounts(None, wanted_tags=[]))
+ []
+
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.
- >>> ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None)
+ >>> list(ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None))
[(u'crash', 1L)]
- >>> debian_woody.getUsedBugTagsWithOpenCounts(None)
+ >>> list(debian_woody.getUsedBugTagsWithOpenCounts(None))
[(u'dataloss', 1L), (u'layout-test', 1L), (u'pebcak', 1L)]
- >>> debian_woody_firefox.getUsedBugTagsWithOpenCounts(None)
+ >>> list(debian_woody_firefox.getUsedBugTagsWithOpenCounts(None))
[(u'dataloss', 1L), (u'layout-test', 1L), (u'pebcak', 1L)]
Only bugs that the supplied user has access to will be counted:
@@ -362,13 +370,13 @@
True
>>> flush_database_updates()
- >>> ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None)
+ >>> list(ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(None))
[]
>>> sample_person = getUtility(ILaunchBag).user
>>> bug_nine.isSubscribed(sample_person)
True
- >>> ubuntu_thunderbird.getUsedBugTagsWithOpenCounts(sample_person)
+ >>> list(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-03-28 20:54:50 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py 2011-04-12 07:28:38 +0000
@@ -403,13 +403,15 @@
def getUsedBugTags():
"""Return the tags used by the context as a sorted list of strings."""
- def getUsedBugTagsWithOpenCounts(user):
+ def getUsedBugTagsWithOpenCounts(user, wanted_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.
"""
def _getOfficialTagClause():
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-04-07 16:23:24 +0000
+++ lib/lp/bugs/model/bug.py 2011-04-12 07:28:38 +0000
@@ -50,6 +50,9 @@
from storm.expr import (
And,
Count,
+ Desc,
+ Exists,
+ Join,
LeftJoin,
Max,
Not,
@@ -211,6 +214,29 @@
%(condition)s GROUP BY BugTag.tag ORDER BY BugTag.tag"""
+def snapshot_bug_params(bug_params):
+ """Return a snapshot of a `CreateBugParams` object."""
+ return Snapshot(
+ bug_params, names=[
+ "owner", "title", "comment", "description", "msg",
+ "datecreated", "security_related", "private",
+ "distribution", "sourcepackagename", "binarypackagename",
+ "product", "status", "subscribers", "tags",
+ "subscribe_owner", "filed_by"])
+
+
+class BugTag(SQLBase):
+ """A tag belonging to a bug."""
+
+ bug = ForeignKey(dbName='bug', foreignKey='Bug', notNull=True)
+ 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.
@@ -230,59 +256,39 @@
return shortlist([row[0] for row in cur.fetchall()])
-def get_bug_tags_open_count(context_condition, user):
+def get_bug_tags_open_count(context_condition, user, wanted_tags=None):
"""Return all the used bug tags with their open bug count.
: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).
"""
- open_statuses_condition = BugTask.status.is_in(
- UNRESOLVED_BUGTASK_STATUSES)
- columns = [
- BugTag.tag,
- Count(),
- ]
- tables = [
+ tables = (
BugTag,
- LeftJoin(Bug, Bug.id == BugTag.bugID),
- LeftJoin(
- BugTask,
- And(BugTask.bugID == Bug.id, open_statuses_condition)),
- ]
+ Join(BugTask, BugTask.bugID == BugTag.bugID),
+ )
where_conditions = [
- open_statuses_condition,
+ BugTask.status.is_in(UNRESOLVED_BUGTASK_STATUSES),
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:
- where_conditions.append(SQLRaw(privacy_filter))
+ # The EXISTS sub-select avoids a join against Bug, improving
+ # performance significantly.
+ 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)
- result = store.execute(Select(
- columns=columns, where=And(*where_conditions), tables=tables,
- group_by=BugTag.tag, order_by=BugTag.tag))
- return shortlist([(row[0], row[1]) for row in result.get_all()])
-
-
-def snapshot_bug_params(bug_params):
- """Return a snapshot of a `CreateBugParams` object."""
- return Snapshot(
- bug_params, names=[
- "owner", "title", "comment", "description", "msg",
- "datecreated", "security_related", "private",
- "distribution", "sourcepackagename", "binarypackagename",
- "product", "status", "subscribers", "tags",
- "subscribe_owner", "filed_by"])
-
-
-class BugTag(SQLBase):
- """A tag belonging to a bug."""
-
- bug = ForeignKey(dbName='bug', foreignKey='Bug', notNull=True)
- tag = StringCol(notNull=True)
+ return store.using(*tables).find(
+ tag_count_columns, *where_conditions).group_by(BugTag.tag).order_by(
+ Desc(Count()), BugTag.tag)
class BugBecameQuestionEvent:
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2011-04-08 15:44:02 +0000
+++ lib/lp/registry/model/distribution.py 2011-04-12 07:28:38 +0000
@@ -649,9 +649,10 @@
"""See `IBugTarget`."""
return get_bug_tags("BugTask.distribution = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See `IBugTarget`."""
- return get_bug_tags_open_count(BugTask.distribution == self, user)
+ return get_bug_tags_open_count(
+ BugTask.distribution == self, user, wanted_tags=wanted_tags)
def getMirrorByName(self, name):
"""See `IDistribution`."""
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2011-04-12 07:28:38 +0000
@@ -488,12 +488,12 @@
"""See `IBugTarget`."""
return self.distribution.getUsedBugTags()
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See `IBugTarget`."""
return get_bug_tags_open_count(
And(BugTask.distribution == self.distribution,
BugTask.sourcepackagename == self.sourcepackagename),
- user)
+ user, wanted_tags=wanted_tags)
def _getOfficialTagClause(self):
return self.distribution._getOfficialTagClause()
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2011-04-08 15:44:02 +0000
+++ lib/lp/registry/model/distroseries.py 2011-04-12 07:28:38 +0000
@@ -833,9 +833,10 @@
"""See `IHasBugs`."""
return get_bug_tags("BugTask.distroseries = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See `IHasBugs`."""
- return get_bug_tags_open_count(BugTask.distroseries == self, user)
+ return get_bug_tags_open_count(
+ BugTask.distroseries == self, user, wanted_tags=wanted_tags)
@property
def has_any_specifications(self):
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2011-03-28 20:54:50 +0000
+++ lib/lp/registry/model/product.py 2011-04-12 07:28:38 +0000
@@ -800,9 +800,10 @@
"""See `IBugTarget`."""
return get_bug_tags("BugTask.product = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See `IBugTarget`."""
- return get_bug_tags_open_count(BugTask.product == self, user)
+ return get_bug_tags_open_count(
+ BugTask.product == self, user, wanted_tags=wanted_tags)
series = SQLMultipleJoin('ProductSeries', joinColumn='product',
orderBy='name')
=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2011-03-28 20:54:50 +0000
+++ lib/lp/registry/model/productseries.py 2011-04-12 07:28:38 +0000
@@ -433,9 +433,10 @@
"""See IBugTarget."""
return get_bug_tags("BugTask.productseries = %s" % sqlvalues(self))
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See IBugTarget."""
- return get_bug_tags_open_count(BugTask.productseries == self, user)
+ return get_bug_tags_open_count(
+ BugTask.productseries == self, user, wanted_tags=wanted_tags)
def createBug(self, bug_params):
"""See IBugTarget."""
=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py 2011-01-21 08:30:55 +0000
+++ lib/lp/registry/model/projectgroup.py 2011-04-12 07:28:38 +0000
@@ -343,13 +343,14 @@
return get_bug_tags(
"BugTask.product IN (%s)" % ",".join(product_ids))
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See `IHasBugs`."""
if not self.products:
return []
product_ids = [product.id for product in self.products]
return get_bug_tags_open_count(
- BugTask.productID.is_in(product_ids), user)
+ BugTask.productID.is_in(product_ids), user,
+ wanted_tags=wanted_tags)
def _getBugTaskContextClause(self):
"""See `HasBugsBase`."""
=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py 2011-04-07 19:08:45 +0000
+++ lib/lp/registry/model/sourcepackage.py 2011-04-12 07:28:38 +0000
@@ -495,12 +495,12 @@
"""See `IBugTarget`."""
return self.distroseries.getUsedBugTags()
- def getUsedBugTagsWithOpenCounts(self, user):
+ def getUsedBugTagsWithOpenCounts(self, user, wanted_tags=None):
"""See `IBugTarget`."""
return get_bug_tags_open_count(
And(BugTask.distroseries == self.distroseries,
BugTask.sourcepackagename == self.sourcepackagename),
- user)
+ user, wanted_tags=wanted_tags)
@property
def max_bug_heat(self):
