launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/launchpad/builders-timeout-bug-887078-eager-load3 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #887078 in Launchpad itself: "Builder:+history timeout"
  https://bugs.launchpad.net/launchpad/+bug/887078

For more details, see:
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078-eager-load3/+merge/83576

This branch aims at reducing the number of repeated statements issued by builder:+history.  It preloads the owners of the build archives and the build logs.

= Tests =

./bin/test -vvc test_builder_views  test_build_history_queries_count_binary_package_builds_in_ppa

= Q/A =

Check out the repeated statements section in the oops for:
https://qastaging.launchpad.net/builders/roseapple/+history/++oops++
(Compare it with the same section in OOPS-0115a140f6ba8ff7a8e10767b961722a.) The only repeated statements left should be the ones related to fetching session data & team participation stuff.
-- 
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078-eager-load3/+merge/83576
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/builders-timeout-bug-887078-eager-load3 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/database/librarian.py'
--- lib/canonical/launchpad/database/librarian.py	2011-03-28 05:11:12 +0000
+++ lib/canonical/launchpad/database/librarian.py	2011-11-28 10:35:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -96,8 +96,8 @@
     _table = 'LibraryFileAlias'
     date_created = UtcDateTimeCol(notNull=False, default=DEFAULT)
     content = ForeignKey(
-            foreignKey='LibraryFileContent', dbName='content', notNull=False,
-            )
+        foreignKey='LibraryFileContent', dbName='content', notNull=False,
+        )
     filename = StringCol(notNull=True)
     mimetype = StringCol(notNull=True)
     expires = UtcDateTimeCol(notNull=False, default=None)

=== modified file 'lib/lp/soyuz/browser/tests/test_builder_views.py'
--- lib/lp/soyuz/browser/tests/test_builder_views.py	2011-11-15 11:08:01 +0000
+++ lib/lp/soyuz/browser/tests/test_builder_views.py	2011-11-28 10:35:26 +0000
@@ -3,6 +3,8 @@
 
 __metaclass__ = type
 
+from functools import partial
+
 from storm.locals import Store
 from testtools.matchers import Equals
 from zope.component import getUtility
@@ -12,7 +14,10 @@
 from canonical.launchpad.ftests import login
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing.layers import LaunchpadFunctionalLayer
-from lp.buildmaster.enums import BuildFarmJobType
+from lp.buildmaster.enums import (
+    BuildFarmJobType,
+    BuildStatus,
+    )
 from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
 from lp.soyuz.browser.builder import BuilderEditView
 from lp.testing import (
@@ -83,6 +88,7 @@
         branch = self.factory.makeBranch()
         build = source.create(build_farm_job, branch)
         removeSecurityProxy(build).builder = self.builder
+        self.addFakeBuildLog(build)
         return build
 
     def createRecipeBuildWithBuilder(self):
@@ -93,11 +99,26 @@
                 branches=[branch1, branch2]))
         Store.of(build).flush()
         removeSecurityProxy(build).builder = self.builder
+        self.addFakeBuildLog(build)
         return build
 
-    def createBinaryPackageBuild(self):
-        build = self.factory.makeBinaryPackageBuild()
-        removeSecurityProxy(build).builder = self.builder
+    def addFakeBuildLog(self, build):
+        lfa = self.factory.makeLibraryFileAlias('mybuildlog.txt')
+        removeSecurityProxy(build).log = lfa
+        import transaction
+        transaction.commit()
+
+    def createBinaryPackageBuild(self, in_ppa=False):
+        archive = None
+        if in_ppa:
+            archive = self.factory.makeArchive()
+        build = self.factory.makeBinaryPackageBuild(
+            archive=archive, status=BuildStatus.FULLYBUILT)
+        naked_build = removeSecurityProxy(build)
+        naked_build.builder = self.builder
+        naked_build.date_started = self.factory.getUniqueDate()
+        naked_build.date_finished = self.factory.getUniqueDate()
+        self.addFakeBuildLog(build)
         return build
 
     def _record_queries_count(self, tested_method, item_creator):
@@ -148,6 +169,18 @@
 
         self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
 
+    def test_build_history_queries_count_binary_package_builds_in_ppa(self):
+        # Rendering to builder's history issues a constant number of queries
+        # when ppa binary builds are displayed.
+        def builder_history_render():
+            create_initialized_view(self.builder, '+history').render()
+        createBinaryPackageBuildInPPA = partial(
+            self.createBinaryPackageBuild, in_ppa=True)
+        recorder1, recorder2 = self._record_queries_count(
+            builder_history_render, createBinaryPackageBuildInPPA)
+
+        self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
+
     def test_build_history_queries_count_translation_template_builds(self):
         # Rendering to builder's history issues a constant number of queries
         # when translation template builds are displayed.

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2011-11-17 12:48:05 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2011-11-28 10:35:26 +0000
@@ -877,12 +877,6 @@
 
         def eager_load(rows):
             # Circular imports.
-            from lp.registry.model.sourcepackagename import (
-                SourcePackageName
-                )
-            from lp.soyuz.model.sourcepackagerelease import (
-                SourcePackageRelease
-                )
             from lp.soyuz.model.distroarchseries import (
                 DistroArchSeries
                 )
@@ -893,16 +887,14 @@
                 Distribution
                 )
             from lp.soyuz.model.archive import Archive
-            sprs = load_related(
-                SourcePackageRelease, rows, ['source_package_release_id'])
-            load_related(
-                SourcePackageName, sprs, ['sourcepackagenameID'])
+            from lp.registry.model.person import Person
+            self._prefetchBuildData(rows)
             distro_arch_series = load_related(
                 DistroArchSeries, rows, ['distro_arch_series_id'])
             package_builds = load_related(
                 PackageBuild, rows, ['package_build_id'])
-            load_related(
-                Archive, package_builds, ['archive_id'])
+            archives = load_related(Archive, package_builds, ['archive_id'])
+            load_related(Person, archives, ['ownerID'])
             distroseries = load_related(
                 DistroSeries, distro_arch_series, ['distroseriesID'])
             load_related(