launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/moar-preload-distroseries-queue-redux into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #835645 in Launchpad itself: "DistroSeries:+queue timeout paginating"
  https://bugs.launchpad.net/launchpad/+bug/835645
  Bug #1032176 in Launchpad itself: "DistroSeries.getPackageUploads timing out due to authorization checking "
  https://bugs.launchpad.net/launchpad/+bug/1032176

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/moar-preload-distroseries-queue-redux/+merge/142056

Preload all the things for IPackageUploadSet.getAll().

Turn ISourcePackageRelease.package_diffs into a cachedproperty, invalidate it when getDiffTo is called and set it inside the preloading magic for IPackageUploadSet.getAll().
-- 
https://code.launchpad.net/~stevenk/launchpad/moar-preload-distroseries-queue-redux/+merge/142056
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/moar-preload-distroseries-queue-redux into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py	2013-01-02 22:59:10 +0000
+++ lib/lp/soyuz/browser/queue.py	2013-01-07 04:45:27 +0000
@@ -55,10 +55,8 @@
     )
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.model.archive import Archive
-from lp.soyuz.model.component import Component
 from lp.soyuz.model.packagecopyjob import PackageCopyJob
 from lp.soyuz.model.queue import PackageUploadSource
-from lp.soyuz.model.section import Section
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 
@@ -237,9 +235,6 @@
             SourcePackageRelease, packageuploadsources,
             ['sourcepackagereleaseID'])
 
-        load_related(Section, source_sprs, ['sectionID'])
-        load_related(Component, source_sprs, ['componentID'])
-
         # Get a dictionary of lists of binary files keyed by upload ID.
         package_upload_builds_dict = self.builds_dict(upload_ids, binary_files)
 

=== modified file 'lib/lp/soyuz/browser/tests/test_queue.py'
--- lib/lp/soyuz/browser/tests/test_queue.py	2013-01-02 22:59:10 +0000
+++ lib/lp/soyuz/browser/tests/test_queue.py	2013-01-07 04:45:27 +0000
@@ -368,20 +368,37 @@
     def test_query_count(self):
         login(ADMIN_EMAIL)
         uploads = []
+        sprs = []
         distroseries = self.factory.makeDistroSeries()
+        dsc = self.factory.makeLibraryFileAlias(filename='foo_0.1.dsc')
+        deb = self.factory.makeLibraryFileAlias(filename='foo.deb')
+        transaction.commit()
         for i in range(5):
             uploads.append(self.factory.makeSourcePackageUpload(distroseries))
+            sprs.append(uploads[-1].sources[0].sourcepackagerelease)
+            sprs[-1].addFile(dsc)
             uploads.append(self.factory.makeCustomPackageUpload(distroseries))
             uploads.append(self.factory.makeCopyJobPackageUpload(distroseries))
+        self.factory.makePackageset(
+            packages=(sprs[0].sourcepackagename, sprs[2].sourcepackagename,
+                sprs[4].sourcepackagename),
+            distroseries=distroseries)
+        self.factory.makePackageset(
+            packages=(sprs[1].sourcepackagename,), distroseries=distroseries)
+        self.factory.makePackageset(
+            packages=(sprs[3].sourcepackagename,), distroseries=distroseries)
+        for i in (0, 2, 3):
+            self.factory.makePackageDiff(to_source=sprs[i])
         for i in range(15):
             uploads.append(self.factory.makeBuildPackageUpload(distroseries))
+            uploads[-1].builds[0].build.binarypackages[0].addFile(deb)
         queue_admin = self.factory.makeArchiveAdmin(distroseries.main_archive)
         Store.of(uploads[0]).invalidate()
         with person_logged_in(queue_admin):
             with StormStatementRecorder() as recorder:
                 view = self.makeView(distroseries, queue_admin)
                 view()   
-        self.assertThat(recorder, HasQueryCount(Equals(52)))
+        self.assertThat(recorder, HasQueryCount(Equals(51)))
 
 
 class TestCompletePackageUpload(TestCaseWithFactory):

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2013-01-02 22:59:10 +0000
+++ lib/lp/soyuz/model/queue.py	2013-01-07 04:45:27 +0000
@@ -48,6 +48,7 @@
 from lp.archivepublisher.customupload import CustomUploadError
 from lp.archivepublisher.debversion import Version
 from lp.archiveuploader.tagfiles import parse_tagfile_content
+from lp.buildmaster.model.packagebuild import PackageBuild
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.auditor.client import AuditorClient
@@ -75,7 +76,10 @@
 from lp.services.features import getFeatureFlag
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.librarian.interfaces.client import DownloadFailed
-from lp.services.librarian.model import LibraryFileAlias
+from lp.services.librarian.model import (
+    LibraryFileAlias,
+    LibraryFileContent,
+    )
 from lp.services.librarian.utils import copy_and_close
 from lp.services.mail.signedmessage import strip_pgp_signature
 from lp.services.propertycache import (
@@ -96,6 +100,7 @@
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.packagecopyjob import IPackageCopyJobSource
+from lp.soyuz.interfaces.packagediff import IPackageDiffSet
 from lp.soyuz.interfaces.publishing import (
     IPublishingSet,
     name_priority_map,
@@ -115,8 +120,15 @@
     QueueStateWriteProtectedError,
     )
 from lp.soyuz.interfaces.section import ISectionSet
+from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
+from lp.soyuz.model.component import Component
 from lp.soyuz.model.distroarchseries import DistroArchSeries
+from lp.soyuz.model.files import (
+    BinaryPackageFile,
+    SourcePackageReleaseFile,
+    )
 from lp.soyuz.pas import BuildDaemonPackagesArchSpecific
+from lp.soyuz.model.section import Section
 
 # There are imports below in PackageUploadCustom for various bits
 # of the archivepublisher which cause circular import errors if they
@@ -1728,18 +1740,30 @@
     source_sprs = load_related(
         SourcePackageRelease, puses, ['sourcepackagereleaseID'])
     bpbs = load_related(BinaryPackageBuild, pubs, ['buildID'])
+    load_related(PackageBuild, bpbs, ['package_build_id'])
     load_related(DistroArchSeries, bpbs, ['distro_arch_series_id'])
     binary_sprs = load_related(
         SourcePackageRelease, bpbs, ['source_package_release_id'])
     sprs = source_sprs + binary_sprs
 
     load_related(SourcePackageName, sprs, ['sourcepackagenameID'])
+    load_related(Section, sprs, ['sectionID'])
+    load_related(Component, sprs, ['componentID'])
     load_related(LibraryFileAlias, uploads, ['changes_file_id'])
     publications = load_referencing(
         SourcePackagePublishingHistory, sprs, ['sourcepackagereleaseID'])
     load_related(Archive, publications, ['archiveID'])
+    diffs = getUtility(IPackageDiffSet).getDiffsToReleases(
+        sprs, preload_for_display=True)
+
+    puc_lfas = load_related(LibraryFileAlias, pucs, ['libraryfilealiasID'])
+    load_related(LibraryFileContent, puc_lfas, ['contentID'])
+
     for spr_cache in sprs:
         get_property_cache(spr_cache).published_archives = []
+        get_property_cache(spr_cache).package_diffs = []
     for publication in publications:
         spr_cache = get_property_cache(publication.sourcepackagerelease)
         spr_cache.published_archives.append(publication.archive)
+    for diff in diffs:
+        get_property_cache(diff.to_source).package_diffs.append(diff)

=== modified file 'lib/lp/soyuz/model/sourcepackagerelease.py'
--- lib/lp/soyuz/model/sourcepackagerelease.py	2013-01-02 22:59:10 +0000
+++ lib/lp/soyuz/model/sourcepackagerelease.py	2013-01-07 04:45:27 +0000
@@ -30,6 +30,7 @@
 from storm.info import ClassAlias
 from storm.locals import (
     Int,
+    Desc,
     Reference,
     )
 from storm.store import Store
@@ -59,7 +60,10 @@
     LibraryFileAlias,
     LibraryFileContent,
     )
-from lp.services.propertycache import cachedproperty
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.soyuz.enums import PackageDiffStatus
 from lp.soyuz.interfaces.archive import MAIN_ARCHIVE_PURPOSES
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
@@ -165,8 +169,6 @@
         joinColumn='sourcepackagerelease', orderBy="libraryfile")
     publishings = SQLMultipleJoin('SourcePackagePublishingHistory',
         joinColumn='sourcepackagerelease', orderBy="-datecreated")
-    package_diffs = SQLMultipleJoin(
-        'PackageDiff', joinColumn='to_source', orderBy="-date_requested")
 
     _user_defined_fields = StringCol(dbName='user_defined_fields')
 
@@ -209,6 +211,12 @@
             return []
         return simplejson.loads(self._user_defined_fields)
 
+    @cachedproperty
+    def package_diffs(self):
+        return list(Store.of(self).find(
+            PackageDiff, to_source=self).order_by(
+                Desc(PackageDiff.date_requested)))
+
     @property
     def builds(self):
         """See `ISourcePackageRelease`."""
@@ -564,6 +572,8 @@
 
     def getDiffTo(self, to_sourcepackagerelease):
         """See ISourcePackageRelease."""
+        Store.of(to_sourcepackagerelease).flush()
+        del get_property_cache(to_sourcepackagerelease).package_diffs
         return PackageDiff.selectOneBy(
             from_source=self, to_source=to_sourcepackagerelease)
 

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2013-01-02 06:26:34 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2013-01-07 04:45:27 +0000
@@ -1304,4 +1304,4 @@
         IStore(uploads[0].__class__).invalidate()
         with StormStatementRecorder() as recorder:
             ws_distroseries.getPackageUploads()
-        self.assertThat(recorder, HasQueryCount(Equals(27)))
+        self.assertThat(recorder, HasQueryCount(Equals(31)))