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-load2 into lp:launchpad with lp:~rvb/launchpad/builders-timeout-bug-887078-eager-load-cachedproperty as a prerequisite.

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-load2/+merge/82200

This branch is a followup on ~rvb/launchpad/builders-timeout-bug-887078-eager-load-cachedproperty.
It eager loads the objects related to a translation template build to avoid issuing one query per translation template build displayed on builder:+history.

= Tests =

./bin/test -vvc test_builder_views test_build_history_queries_count_translation_template_builds

= Q/A =

None.
-- 
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078-eager-load2/+merge/82200
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/builders-timeout-bug-887078-eager-load2 into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/tests/test_builder_views.py'
--- lib/lp/soyuz/browser/tests/test_builder_views.py	2011-11-14 17:38:34 +0000
+++ lib/lp/soyuz/browser/tests/test_builder_views.py	2011-11-14 17:38:38 +0000
@@ -5,12 +5,15 @@
 
 from storm.locals import Store
 from testtools.matchers import Equals
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.database.sqlbase import flush_database_caches
 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.interfaces.buildfarmjob import IBuildFarmJobSource
 from lp.soyuz.browser.builder import BuilderEditView
 from lp.testing import (
     StormStatementRecorder,
@@ -20,6 +23,9 @@
 from lp.testing.matchers import HasQueryCount
 from lp.testing.sampledata import ADMIN_EMAIL
 from lp.testing.views import create_initialized_view
+from lp.translations.interfaces.translationtemplatesbuild import (
+    ITranslationTemplatesBuildSource,
+    )
 
 
 class TestBuilderEditView(TestCaseWithFactory):
@@ -69,6 +75,16 @@
         super(TestBuilderHistoryView, self).setUp()
         self.builder = self.factory.makeBuilder()
 
+    def createTranslationTemplateBuildWithBuilder(self):
+        build_farm_job_source = getUtility(IBuildFarmJobSource)
+        build_farm_job = build_farm_job_source.new(
+            BuildFarmJobType.TRANSLATIONTEMPLATESBUILD)
+        source = getUtility(ITranslationTemplatesBuildSource)
+        branch = self.factory.makeBranch()
+        build = source.create(build_farm_job, branch)
+        removeSecurityProxy(build).builder = self.builder
+        return build
+
     def createRecipeBuildWithBuilder(self):
         build = self.factory.makeSourcePackageRecipeBuild()
         Store.of(build).flush()
@@ -127,3 +143,14 @@
             self.createBinaryPackageBuild)
 
         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.
+        def call_history_render():
+            create_initialized_view(self.builder, '+history').render()
+        recorder1, recorder2 = self._record_queries_count(
+            call_history_render,
+            self.createTranslationTemplateBuildWithBuilder)
+
+        self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))

=== modified file 'lib/lp/translations/model/translationtemplatesbuild.py'
--- lib/lp/translations/model/translationtemplatesbuild.py	2011-11-09 11:36:44 +0000
+++ lib/lp/translations/model/translationtemplatesbuild.py	2011-11-14 17:38:38 +0000
@@ -18,12 +18,19 @@
     implements,
     )
 
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.buildmaster.model.buildfarmjob import BuildFarmJobDerived
+from lp.code.model.branch import Branch
+from lp.code.model.branchcollection import GenericBranchCollection
 from lp.code.model.branchjob import (
     BranchJob,
     BranchJobType,
     )
+from lp.registry.model.product import Product
+from lp.services.database.bulk import load_related
 from lp.translations.interfaces.translationtemplatesbuild import (
     ITranslationTemplatesBuild,
     ITranslationTemplatesBuildSource,
@@ -111,10 +118,20 @@
         buildfarmjob_ids = [buildfarmjob.id for buildfarmjob in buildfarmjobs]
         """See `ITranslationTemplatesBuildSource`."""
         store = cls._getStore(store)
-        return store.find(
+
+        def eager_load(rows):
+            branches = load_related(
+                Branch, rows, ['branch_id'])
+            load_related(
+                Product, branches, ['productID'])
+            branch_collection = GenericBranchCollection()
+            branch_collection._preloadDataForBranches(branches)
+
+        resultset = store.find(
             TranslationTemplatesBuild,
             TranslationTemplatesBuild.build_farm_job_id.is_in(
                 buildfarmjob_ids))
+        return DecoratedResultSet(resultset, pre_iter_hook=eager_load)
 
     @classmethod
     def findByBranch(cls, branch, store=None):