launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/optimise-getPublishedBinaries into lp:launchpad.

Commit message:
Stop SPPH.getPublishedBinaries materialising rows outside the current batch.

This fixes webservice timeouts for sources with large numbers of binaries.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1695113 in Launchpad itself: "OOPS when accessing getPublishedBinaries"
  https://bugs.launchpad.net/launchpad/+bug/1695113

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/optimise-getPublishedBinaries/+merge/325028
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/optimise-getPublishedBinaries into lp:launchpad.

=== modified file 'lib/lp/soyuz/doc/publishing.txt'
--- lib/lp/soyuz/doc/publishing.txt	2016-10-02 12:38:58 +0000
+++ lib/lp/soyuz/doc/publishing.txt	2017-06-02 21:53:54 +0000
@@ -512,13 +512,13 @@
 as active, SUPERSEDED, DELETED and OBSOLETE are excluded. Differently,
 getBuiltBinaries() follows binaries in any state.
 
-    >>> len(source.getPublishedBinaries())
+    >>> source.getPublishedBinaries().count()
     2
 
     >>> len(source.getBuiltBinaries())
     2
 
-Note that getPublishedBinaries() returns a SelectResult and
+Note that getPublishedBinaries() returns a DecoratedResultSet and
 getBuiltBinaries() returns a list.
 
 When we supersede one of the original binary publications, it gets
@@ -528,7 +528,7 @@
     >>> a_binary = source.getPublishedBinaries()[0]
     >>> a_binary.supersede()
 
-    >>> len(source.getPublishedBinaries())
+    >>> source.getPublishedBinaries().count()
     1
 
     >>> len(source.getBuiltBinaries())
@@ -541,7 +541,7 @@
     >>> deletable.requestDeletion(mark, "go")
     >>> deleted = deletable
 
-    >>> len(source.getPublishedBinaries())
+    >>> source.getPublishedBinaries().count()
     0
 
     >>> len(source.getBuiltBinaries())
@@ -550,13 +550,13 @@
 Finally we will mark both copied binary publication as obsolete and
 verify that the getPublishedBinaries() result is also empty after that.
 
-    >>> len(copied_source.getPublishedBinaries())
+    >>> copied_source.getPublishedBinaries().count()
     2
 
     >>> for bin in copied_source.getPublishedBinaries():
     ...     obsoleted = bin.requestObsolescence()
 
-    >>> len(copied_source.getPublishedBinaries())
+    >>> copied_source.getPublishedBinaries().count()
     0
 
     >>> len(copied_source.getBuiltBinaries())

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2017-04-12 11:31:44 +0000
+++ lib/lp/soyuz/model/publishing.py	2017-06-02 21:53:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -14,7 +14,10 @@
 
 from collections import defaultdict
 from datetime import datetime
-from operator import attrgetter
+from operator import (
+    attrgetter,
+    itemgetter,
+    )
 import os
 import sys
 
@@ -51,6 +54,7 @@
 from lp.services.database import bulk
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.datetimecol import UtcDateTimeCol
+from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.interfaces import (
     IMasterStore,
@@ -288,10 +292,7 @@
         """See `ISourcePackagePublishingHistory`."""
         publishing_set = getUtility(IPublishingSet)
         result_set = publishing_set.getBinaryPublicationsForSources(self)
-
-        return [binary_pub
-                for source, binary_pub, binary, binary_name, arch
-                in result_set]
+        return DecoratedResultSet(result_set, result_decorator=itemgetter(1))
 
     def getBuiltBinaries(self, want_files=False):
         """See `ISourcePackagePublishingHistory`."""