launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/stormify-publisher into lp:launchpad.

Commit message:
Convert SQLObject-based queries in Publisher to Storm.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/stormify-publisher/+merge/226056

Convert SQLObject-based queries in Publisher to Storm.
-- 
https://code.launchpad.net/~cjwatson/launchpad/stormify-publisher/+merge/226056
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/stormify-publisher into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2014-07-08 23:22:48 +0000
+++ lib/lp/archivepublisher/publishing.py	2014-07-09 02:12:56 +0000
@@ -21,6 +21,7 @@
     _multivalued,
     Release,
     )
+from storm.expr import Desc
 from zope.component import getUtility
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -47,7 +48,7 @@
     )
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.sqlbase import sqlvalues
+from lp.services.database.interfaces import IStore
 from lp.services.librarian.client import LibrarianClient
 from lp.services.utils import file_exists
 from lp.soyuz.enums import (
@@ -61,6 +62,7 @@
     active_publishing_status,
     IPublishingSet,
     )
+from lp.soyuz.model.distroarchseries import DistroArchSeries
 from lp.soyuz.model.publishing import (
     BinaryPackagePublishingHistory,
     SourcePackagePublishingHistory,
@@ -316,20 +318,22 @@
         If the distroseries is already released, it automatically refuses
         to publish records to the RELEASE pocket.
         """
-        queries = ['distroseries = %s' % sqlvalues(distroseries)]
-
-        # Query main archive for this distroseries
-        queries.append('archive=%s' % sqlvalues(self.archive))
+        conditions = [
+            SourcePackagePublishingHistory.distroseries == distroseries,
+            # Query main archive for this distroseries
+            SourcePackagePublishingHistory.archive == self.archive,
+            ]
 
         # Careful publishing should include all PUBLISHED rows, normal run
         # only includes PENDING ones.
         statuses = [PackagePublishingStatus.PENDING]
         if is_careful:
             statuses.append(PackagePublishingStatus.PUBLISHED)
-        queries.append('status IN %s' % sqlvalues(statuses))
+        conditions.append(
+            SourcePackagePublishingHistory.status.is_in(statuses))
 
         # Restrict to a specific pocket.
-        queries.append('pocket = %s' % sqlvalues(pocket))
+        conditions.append(SourcePackagePublishingHistory.pocket == pocket)
 
         # Exclude RELEASE pocket if the distroseries was already released,
         # since it should not change for main archive.
@@ -337,13 +341,13 @@
         # We also allow RELEASE publishing for partner.
         if (not distroseries.isUnstable() and
             not self.archive.allowUpdatesToReleasePocket()):
-            queries.append(
-            'pocket != %s' % sqlvalues(PackagePublishingPocket.RELEASE))
-
-        publications = SourcePackagePublishingHistory.select(
-            " AND ".join(queries), orderBy="-id")
-
-        return publications
+            conditions.append(
+                SourcePackagePublishingHistory.pocket !=
+                    PackagePublishingPocket.RELEASE)
+
+        publications = IStore(SourcePackagePublishingHistory).find(
+            SourcePackagePublishingHistory, *conditions)
+        return publications.order_by(Desc(SourcePackagePublishingHistory.id))
 
     def publishSources(self, distroseries, pocket, is_careful=False):
         """Publish sources for a given distroseries and pocket.
@@ -372,30 +376,32 @@
         If the distroseries is already released, it automatically refuses
         to publish records to the RELEASE pocket.
         """
-        queries = [
-            "distroarchseries = %s AND archive = %s"
-            % sqlvalues(distroarchseries, self.archive)
+        conditions = [
+            BinaryPackagePublishingHistory.distroarchseries ==
+                distroarchseries,
+            BinaryPackagePublishingHistory.archive == self.archive,
             ]
 
-        target_status = [PackagePublishingStatus.PENDING]
+        statuses = [PackagePublishingStatus.PENDING]
         if is_careful:
-            target_status.append(PackagePublishingStatus.PUBLISHED)
-        queries.append("status IN %s" % sqlvalues(target_status))
+            statuses.append(PackagePublishingStatus.PUBLISHED)
+        conditions.append(
+            BinaryPackagePublishingHistory.status.is_in(statuses))
 
-        # restrict to a specific pocket.
-        queries.append('pocket = %s' % sqlvalues(pocket))
+        # Restrict to a specific pocket.
+        conditions.append(BinaryPackagePublishingHistory.pocket == pocket)
 
         # Exclude RELEASE pocket if the distroseries was already released,
         # since it should not change, unless the archive allows it.
         if (not distroarchseries.distroseries.isUnstable() and
             not self.archive.allowUpdatesToReleasePocket()):
-            queries.append(
-            'pocket != %s' % sqlvalues(PackagePublishingPocket.RELEASE))
-
-        publications = BinaryPackagePublishingHistory.select(
-                    " AND ".join(queries), orderBy=["-id"])
-
-        return publications
+            conditions.append(
+                BinaryPackagePublishingHistory.pocket !=
+                    PackagePublishingPocket.RELEASE)
+
+        publications = IStore(BinaryPackagePublishingHistory).find(
+            BinaryPackagePublishingHistory, *conditions)
+        return publications.order_by(Desc(BinaryPackagePublishingHistory.id))
 
     def publishBinaries(self, distroarchseries, pocket, is_careful=False):
         """Publish binaries for a given distroseries and pocket.
@@ -451,13 +457,13 @@
         self.log.debug("* Step A2: Mark pockets with deletions as dirty")
 
         # Query part that is common to both queries below.
-        base_query = """
-            archive = %s AND
-            status = %s AND
-            scheduleddeletiondate IS NULL AND
-            dateremoved is NULL
-            """ % sqlvalues(self.archive,
-                            PackagePublishingStatus.DELETED)
+        def base_conditions(table):
+            return [
+                table.archive == self.archive,
+                table.status == PackagePublishingStatus.DELETED,
+                table.scheduleddeletiondate == None,
+                table.dateremoved == None,
+                ]
 
         # We need to get a set of (distroseries, pocket) tuples that have
         # publications that are waiting to be deleted.  Each tuple is
@@ -472,13 +478,16 @@
                     # stable distroseries, no matter what other bugs
                     # that precede here have dirtied it.
                     continue
-                clauses = [base_query]
-                clauses.append("pocket = %s" % sqlvalues(pocket))
-                clauses.append("distroseries = %s" % sqlvalues(distroseries))
+                conditions = base_conditions(SourcePackagePublishingHistory)
+                conditions.extend([
+                    SourcePackagePublishingHistory.pocket == pocket,
+                    SourcePackagePublishingHistory.distroseries ==
+                        distroseries,
+                    ])
 
                 # Make the source publications query.
-                source_query = " AND ".join(clauses)
-                sources = SourcePackagePublishingHistory.select(source_query)
+                sources = IStore(SourcePackagePublishingHistory).find(
+                    SourcePackagePublishingHistory, *conditions)
                 if not sources.is_empty():
                     self.markPocketDirty(distroseries, pocket)
                     # No need to check binaries if the pocket is already
@@ -486,14 +495,15 @@
                     continue
 
                 # Make the binary publications query.
-                clauses = [base_query]
-                clauses.append("pocket = %s" % sqlvalues(pocket))
-                clauses.append("DistroArchSeries = DistroArchSeries.id")
-                clauses.append("DistroArchSeries.distroseries = %s" %
-                    sqlvalues(distroseries))
-                binary_query = " AND ".join(clauses)
-                binaries = BinaryPackagePublishingHistory.select(binary_query,
-                    clauseTables=['DistroArchSeries'])
+                conditions = base_conditions(BinaryPackagePublishingHistory)
+                conditions.extend([
+                    BinaryPackagePublishingHistory.pocket == pocket,
+                    BinaryPackagePublishingHistory.distroarchseriesID ==
+                        DistroArchSeries.id,
+                    DistroArchSeries.distroseries == distroseries,
+                    ])
+                binaries = IStore(BinaryPackagePublishingHistory).find(
+                    BinaryPackagePublishingHistory, *conditions)
                 if not binaries.is_empty():
                     self.markPocketDirty(distroseries, pocket)