launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1095188 into lp:launchpad.

Commit message:
Traversing to a BugComment by index now efficiently slices to just the relevant comment, rather than loading details of all the bug's comments.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1095188 in Launchpad itself: "BugComment:EntryResource and BugComment:+index load all comments for the bug"
  https://bugs.launchpad.net/launchpad/+bug/1095188

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1095188/+merge/141579

Traversing to a BugComment was pulling in all of the bug's comments, when we can now just slice out the one we need. Let's do that to fix timeouts.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-1095188/+merge/141579
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1095188 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-12-20 14:55:13 +0000
+++ lib/lp/bugs/browser/bugtask.py	2013-01-02 03:52:20 +0000
@@ -589,25 +589,17 @@
 
     @stepthrough('comments')
     def traverse_comments(self, name):
-        """Traverse to a comment by id."""
+        """Traverse to a comment by index."""
         if not name.isdigit():
             return None
         index = int(name)
-        comments = get_comments_for_bugtask(self.context)
-        # I couldn't find a way of using index to restrict the queries
-        # in get_comments_for_bugtask in a way that wasn't horrible, and
-        # it wouldn't really save us a lot in terms of database time, so
-        # I have chosed to use this simple solution for now.
-        #   -- kiko, 2006-07-11
-        try:
-            comment = comments[index]
-            if (comment.visible
-                or check_permission('launchpad.Admin', self.context)):
-                return comment
-            else:
-                return None
-        except IndexError:
-            return None
+        # Ask the DB to slice out just the comment that we need.
+        comments = get_comments_for_bugtask(
+            self.context, slice_info=[slice(index, index + 1)])
+        if (comments and comments[0].visible
+            or check_permission('launchpad.Admin', self.context)):
+            return comments[0]
+        return None
 
     @stepthrough('nominations')
     def traverse_nominations(self, nomination_id):