launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/calculateOldBinaries-use-denorm into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #909240 in Launchpad itself: "Can't display New queue due to timeouts"
  https://bugs.launchpad.net/launchpad/+bug/909240

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/calculateOldBinaries-use-denorm/+merge/88816

Rewrite the query in IBinaryPackageNameSet.getNotNewByName() to use IStore, and to no longer join against BinaryPackageRelease, and instead use the denormalised data that is directly in BinaryPackagePublishingHistory.

I also removed a stupid duplication effort in terms of the statuses that it checks, and removed a large amount of pointless comments.
-- 
https://code.launchpad.net/~stevenk/launchpad/calculateOldBinaries-use-denorm/+merge/88816
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/calculateOldBinaries-use-denorm into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/binarypackagename.py'
--- lib/lp/soyuz/model/binarypackagename.py	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/model/binarypackagename.py	2012-01-17 07:29:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -11,13 +11,12 @@
     'getBinaryPackageDescriptions',
 ]
 
-# SQLObject/SQLBase
 from sqlobject import (
     SQLObjectNotFound,
     StringCol,
     )
 from storm.store import EmptyResultSet
-# Zope imports
+from storm.expr import Join
 from zope.interface import implements
 from zope.schema.vocabulary import SimpleTerm
 
@@ -32,11 +31,11 @@
     BatchedCountableIterator,
     NamedSQLObjectHugeVocabulary,
     )
-from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.interfaces.binarypackagename import (
     IBinaryPackageName,
     IBinaryPackageNameSet,
     )
+from lp.soyuz.interfaces.publishing import active_publishing_status
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
 
 
@@ -97,33 +96,29 @@
 
     def getNotNewByNames(self, name_ids, distroseries, archive_ids):
         """See `IBinaryPackageNameSet`."""
-        # Here we're returning `BinaryPackageName`s where the records
-        # for the supplied `BinaryPackageName` IDs are published in the
-        # supplied distroseries.  If they're already published then they
-        # must not be new.
+        # Circular imports.
+        from lp.soyuz.model.distroarchseries import DistroArchSeries
+        from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
+
         if len(name_ids) == 0:
             return EmptyResultSet()
 
-        statuses = (
-            PackagePublishingStatus.PUBLISHED,
-            PackagePublishingStatus.PENDING,
-            )
-
-        return BinaryPackageName.select("""
-            BinaryPackagePublishingHistory.binarypackagerelease =
-                BinaryPackageRelease.id AND
-            BinaryPackagePublishingHistory.distroarchseries =
-                DistroArchSeries.id AND
-            DistroArchSeries.distroseries = %s AND
-            BinaryPackagePublishingHistory.status IN %s AND
-            BinaryPackagePublishingHistory.archive IN %s AND
-            BinaryPackageRelease.binarypackagename = BinaryPackageName.id AND
-            BinaryPackageName.id IN %s
-            """ % sqlvalues(distroseries, statuses, archive_ids, name_ids),
-            distinct=True,
-            clauseTables=["BinaryPackagePublishingHistory",
-                          "BinaryPackageRelease",
-                          "DistroArchSeries"])
+        return IStore(BinaryPackagePublishingHistory).using(
+            BinaryPackagePublishingHistory,
+            Join(BinaryPackageName,
+                BinaryPackagePublishingHistory.binarypackagenameID ==
+                BinaryPackageName.id),
+            Join(DistroArchSeries,
+                BinaryPackagePublishingHistory.distroarchseriesID ==
+                DistroArchSeries.id)
+            ).find(
+                BinaryPackageName,
+                DistroArchSeries.distroseries == distroseries,
+                BinaryPackagePublishingHistory.status.is_in(
+                    active_publishing_status),
+                BinaryPackagePublishingHistory.archiveID.is_in(archive_ids),
+                BinaryPackagePublishingHistory.binarypackagenameID.is_in(
+                    name_ids))
 
 
 class BinaryPackageNameIterator(BatchedCountableIterator):