← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~stevenk/launchpad/destroy-use-sqlbuilder into lp:launchpad

 

Steve Kowalik has proposed merging lp:~stevenk/launchpad/destroy-use-sqlbuilder into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/destroy-use-sqlbuilder/+merge/161694

Destroy all callsites of sqlobject.sqlbuilder, except for one which is sqlrepr and scared me, because quote is used all over the place.
-- 
https://code.launchpad.net/~stevenk/launchpad/destroy-use-sqlbuilder/+merge/161694
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/destroy-use-sqlbuilder into lp:launchpad.
=== modified file 'cronscripts/update-stats.py'
--- cronscripts/update-stats.py	2013-01-07 02:40:55 +0000
+++ cronscripts/update-stats.py	2013-04-30 20:25:32 +0000
@@ -31,7 +31,7 @@
         launchpad_stats = getUtility(ILaunchpadStatisticSet)
         launchpad_stats.updateStatistics(self.txn)
 
-        getUtility(IPersonSet).updateStatistics(self.txn)
+        getUtility(IPersonSet).updateStatistics()
 
         self.logger.debug('Finished the stats update')
 

=== modified file 'lib/lp/answers/model/faq.py'
--- lib/lp/answers/model/faq.py	2013-01-07 02:40:55 +0000
+++ lib/lp/answers/model/faq.py	2013-04-30 20:25:32 +0000
@@ -12,13 +12,13 @@
     ]
 
 from lazr.lifecycle.event import ObjectCreatedEvent
+from storm.locals import SQL
 from sqlobject import (
     ForeignKey,
     SQLMultipleJoin,
     SQLObjectNotFound,
     StringCol,
     )
-from sqlobject.sqlbuilder import SQLConstant
 from zope.event import notify
 from zope.interface import implements
 
@@ -136,7 +136,7 @@
         return FAQ.select(
             '%s AND FAQ.fti @@ %s' % (target_constraint, quote(fti_search)),
             orderBy=[
-                SQLConstant("-rank(FAQ.fti, %s::tsquery)" % quote(fti_search)),
+                SQL("-rank(FAQ.fti, %s::tsquery)" % quote(fti_search)),
                 "-FAQ.date_created"])
 
     @staticmethod
@@ -267,11 +267,9 @@
             return "FAQ.date_created"
         elif sort is FAQSort.RELEVANCY:
             if self.search_text:
-                # SQLConstant is a workaround for bug 53455.
-                return [SQLConstant(
-                            "-rank(FAQ.fti, ftq(%s))" % quote(
-                                self.search_text)),
-                        "-FAQ.date_created"]
+                return [
+                    SQL("-rank(FAQ.fti, ftq(%s))" % quote(self.search_text)),
+                    "-FAQ.date_created"]
             else:
                 return "-FAQ.date_created"
         else:

=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py	2012-10-30 16:59:58 +0000
+++ lib/lp/answers/model/question.py	2013-04-30 20:25:32 +0000
@@ -34,8 +34,10 @@
     SQLRelatedJoin,
     StringCol,
     )
-from sqlobject.sqlbuilder import SQLConstant
-from storm.expr import LeftJoin
+from storm.expr import (
+    LeftJoin,
+    SQL,
+    )
 from storm.store import Store
 from zope.component import getUtility
 from zope.event import notify
@@ -1020,17 +1022,16 @@
             return ["Question.status", "-Question.datecreated"]
         elif sort is QuestionSort.RELEVANCY:
             if self.search_text:
-                # SQLConstant is a workaround for bug 53455
                 if self.nl_phrase_used:
-                    return [SQLConstant(
-                                "-rank(Question.fti, %s::tsquery)" % quote(
-                                    self.search_text)),
-                            "-Question.datecreated"]
+                    return [
+                        SQL("-rank(Question.fti, %s::tsquery)" % quote(
+                            self.search_text)),
+                        "-Question.datecreated"]
                 else:
-                    return [SQLConstant(
-                                "-rank(Question.fti, ftq(%s))" % quote(
-                                    self.search_text)),
-                            "-Question.datecreated"]
+                    return [
+                        SQL("-rank(Question.fti, ftq(%s))" % quote(
+                            self.search_text)),
+                        "-Question.datecreated"]
             else:
                 return "-Question.datecreated"
         elif sort is QuestionSort.RECENT_OWNER_ACTIVITY:

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-11-15 22:22:38 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2013-04-30 20:25:32 +0000
@@ -12,7 +12,6 @@
     ]
 
 from lazr.enum import BaseItem
-from sqlobject.sqlbuilder import SQLConstant
 from storm.expr import (
     Alias,
     And,
@@ -794,9 +793,6 @@
             pass
 
     for orderby_col in orderby:
-        if isinstance(orderby_col, SQLConstant):
-            orderby_arg.append(orderby_col)
-            continue
         desc_search = False
         if orderby_col.startswith("-"):
             orderby_col = orderby_col[1:]

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2013-01-07 02:40:55 +0000
+++ lib/lp/bugs/model/bugtracker.py	2013-04-30 20:25:32 +0000
@@ -32,7 +32,6 @@
     SQLObjectNotFound,
     StringCol,
     )
-from sqlobject.sqlbuilder import AND
 from storm.expr import (
     Count,
     Desc,
@@ -510,12 +509,12 @@
         # are already watching a remote bug.
         if self.bugtrackertype == BugTrackerType.EMAILADDRESS:
             return []
-
-        return shortlist(Bug.select(AND(BugWatch.q.bugID == Bug.q.id,
-                                        BugWatch.q.bugtrackerID == self.id,
-                                        BugWatch.q.remotebug == remotebug),
-                                    distinct=True,
-                                    orderBy=['datecreated']))
+        return shortlist(
+            Store.of(self).find(
+                Bug,
+                BugWatch.bugID == Bug.id, BugWatch.bugtrackerID == self.id,
+                BugWatch.remotebug == remotebug).config(
+                    distinct=True).order_by(Bug.datecreated))
 
     @property
     def watches_ready_to_check(self):
@@ -591,10 +590,10 @@
     @property
     def imported_bug_messages(self):
         """See `IBugTracker`."""
-        return BugMessage.select(
-            AND((BugMessage.q.bugwatchID == BugWatch.q.id),
-                (BugWatch.q.bugtrackerID == self.id)),
-            orderBy=BugMessage.q.id)
+        return Store.of(self).find(
+            BugMessage,
+            BugMessage.bugwatchID == BugWatch.id,
+            BugWatch.bugtrackerID == self.id).order_by(BugMessage.id)
 
     def getLinkedPersonByName(self, name):
         """Return the Person with a given name on this bugtracker."""

=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py	2012-06-29 08:40:05 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py	2013-04-30 20:25:32 +0000
@@ -14,7 +14,7 @@
 import unittest
 
 from pytz import UTC
-from sqlobject.sqlbuilder import SQLConstant
+from storm.locals import SQL
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
@@ -121,9 +121,7 @@
         if state == CodeImportJobState.RUNNING:
             getUtility(ICodeImportJobWorkflow).startJob(job, self.machine)
         naked_job = removeSecurityProxy(job)
-        naked_job.date_due = SQLConstant(
-            "CURRENT_TIMESTAMP AT TIME ZONE 'UTC' + '%d days'"
-            % date_due_delta)
+        naked_job.date_due = UTC_NOW + SQL('%d days' % date_due_delta)
         naked_job.requesting_user = requesting_user
         return job
 
@@ -231,9 +229,7 @@
     def makeJobWithHeartbeatInPast(self, seconds_in_past):
         code_import = make_running_import(factory=self.factory)
         naked_job = removeSecurityProxy(code_import.import_job)
-        naked_job.heartbeat = SQLConstant(
-            "CURRENT_TIMESTAMP AT TIME ZONE 'UTC' + '%d seconds'"
-            % (-seconds_in_past,))
+        naked_job.date_due = UTC_NOW + SQL('%d seconds' % (-seconds_in_past,))
         return code_import.import_job
 
     def assertReclaimableJobs(self, jobs):

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2013-03-12 05:51:28 +0000
+++ lib/lp/registry/interfaces/person.py	2013-04-30 20:25:32 +0000
@@ -2186,7 +2186,7 @@
     def getByAccount(account):
         """Return the `IPerson` with the given account, or None."""
 
-    def updateStatistics(ztm):
+    def updateStatistics():
         """Update statistics caches and commit."""
 
     def peopleCount():

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2013-02-25 04:24:16 +0000
+++ lib/lp/registry/model/distribution.py	2013-04-30 20:25:32 +0000
@@ -18,7 +18,6 @@
     SQLObjectNotFound,
     StringCol,
     )
-from sqlobject.sqlbuilder import SQLConstant
 from storm.expr import (
     And,
     Desc,
@@ -1229,12 +1228,9 @@
 
         if text:
             orderBy.insert(
-                0, SQLConstant(
-                    'rank(Archive.fti, ftq(%s)) DESC' % quote(text)))
+                0, SQL('rank(Archive.fti, ftq(%s)) DESC' % quote(text)))
 
-            clauses.append("""
-                Archive.fti @@ ftq(%s)
-            """ % sqlvalues(text))
+            clauses.append("Archive.fti @@ ftq(%s)" % sqlvalues(text))
 
         if user is not None:
             if not user.inTeam(getUtility(ILaunchpadCelebrities).admin):

=== modified file 'lib/lp/registry/model/distributionmirror.py'
--- lib/lp/registry/model/distributionmirror.py	2013-01-07 02:40:55 +0000
+++ lib/lp/registry/model/distributionmirror.py	2013-04-30 20:25:32 +0000
@@ -25,7 +25,6 @@
     ForeignKey,
     StringCol,
     )
-from sqlobject.sqlbuilder import AND
 from storm.expr import (
     And,
     Desc,
@@ -589,13 +588,14 @@
         country_id = None
         if country is not None:
             country_id = country.id
-        base_query = AND(
-            DistributionMirror.q.content == mirror_type,
-            DistributionMirror.q.enabled == True,
-            DistributionMirror.q.http_base_url != None,
-            DistributionMirror.q.official_candidate == True,
-            DistributionMirror.q.status == MirrorStatus.OFFICIAL)
-        query = AND(DistributionMirror.q.countryID == country_id, base_query)
+        base_query = And(
+            DistributionMirror.content == mirror_type,
+            DistributionMirror.enabled == True,
+            DistributionMirror.http_base_url != None,
+            DistributionMirror.official_candidate == True,
+            DistributionMirror.status == MirrorStatus.OFFICIAL,
+            DistributionMirror.countryID == country_id)
+        query = And(DistributionMirror.countryID == country_id, base_query)
         # The list of mirrors returned by this method is fed to apt through
         # launchpad.net, so we order the results randomly in a lame attempt to
         # balance the load on the mirrors.
@@ -606,9 +606,9 @@
 
         if not mirrors and country is not None:
             continent = country.continent
-            query = AND(
+            query = And(
                 Country.q.continentID == continent.id,
-                DistributionMirror.q.countryID == Country.q.id,
+                DistributionMirror.countryID == Country.q.id,
                 base_query)
             mirrors.extend(shortlist(
                 DistributionMirror.select(query, orderBy=order_by),

=== modified file 'lib/lp/registry/model/karma.py'
--- lib/lp/registry/model/karma.py	2013-01-07 02:40:55 +0000
+++ lib/lp/registry/model/karma.py	2013-04-30 20:25:32 +0000
@@ -21,7 +21,6 @@
     SQLObjectNotFound,
     StringCol,
     )
-from sqlobject.sqlbuilder import AND
 from storm.expr import Desc
 from zope.interface import implements
 
@@ -183,16 +182,14 @@
 
         Return None if it's not found.
         """
-        # Can't use selectBy() because product/distribution/sourcepackagename
-        # may be None.
-        query = AND(
-            KarmaCache.q.personID == person_id,
-            KarmaCache.q.categoryID == category_id,
-            KarmaCache.q.productID == product_id,
-            KarmaCache.q.projectID == project_id,
-            KarmaCache.q.distributionID == distribution_id,
-            KarmaCache.q.sourcepackagenameID == sourcepackagename_id)
-        return KarmaCache.selectOne(query)
+        return IStore(KarmaCache).find(
+            KarmaCache, 
+            KarmaCache.personID == person_id,
+            KarmaCache.categoryID == category_id,
+            KarmaCache.productID == product_id,
+            KarmaCache.projectID == project_id,
+            KarmaCache.distributionID == distribution_id,
+            KarmaCache.sourcepackagenameID == sourcepackagename_id)
 
 
 class KarmaTotalCache(SQLBase):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2013-04-16 06:49:48 +0000
+++ lib/lp/registry/model/person.py	2013-04-30 20:25:32 +0000
@@ -36,6 +36,7 @@
 import random
 import re
 import subprocess
+import transaction
 import weakref
 
 from lazr.delegates import delegates
@@ -52,11 +53,6 @@
     SQLObjectNotFound,
     StringCol,
     )
-from sqlobject.sqlbuilder import (
-    AND,
-    OR,
-    SQLConstant,
-    )
 from storm.base import Storm
 from storm.expr import (
     Alias,
@@ -524,14 +520,13 @@
 
     delegates(IPersonSettings, context='_person_settings')
 
-    sortingColumns = SQLConstant(
-        "person_sort_key(Person.displayname, Person.name)")
+    sortingColumns = SQL("person_sort_key(Person.displayname, Person.name)")
     # Redefine the default ordering into Storm syntax.
     _storm_sortingColumns = ('Person.displayname', 'Person.name')
     # When doing any sort of set operations (union, intersect, except_) with
     # SQLObject we can't use sortingColumns because the table name Person is
     # not available in that context, so we use this one.
-    _sortingColumnsForSetOperations = SQLConstant(
+    _sortingColumnsForSetOperations = SQL(
         "person_sort_key(displayname, name)")
     _defaultOrder = sortingColumns
     _visibility_warning_marker = object()
@@ -941,13 +936,13 @@
         person participates in, the one with the oldest creation date is
         returned.
         """
-        query = AND(
-            TeamMembership.q.teamID == team.id,
-            TeamMembership.q.personID == Person.q.id,
-            OR(TeamMembership.q.status == TeamMembershipStatus.ADMIN,
-               TeamMembership.q.status == TeamMembershipStatus.APPROVED),
-            TeamParticipation.q.teamID == Person.q.id,
-            TeamParticipation.q.personID == self.id)
+        query = And(
+            TeamMembership.teamID == team.id,
+            TeamMembership.personID == Person.q.id,
+            Or(TeamMembership.status == TeamMembershipStatus.ADMIN,
+               TeamMembership.status == TeamMembershipStatus.APPROVED),
+            TeamParticipation.teamID == Person.id,
+            TeamParticipation.personID == self.id)
         clauseTables = ['TeamMembership', 'TeamParticipation']
         member = Person.selectFirst(
             query, clauseTables=clauseTables, orderBy='datecreated')
@@ -3487,28 +3482,28 @@
 
     def getByName(self, name, ignore_merged=True):
         """See `IPersonSet`."""
-        query = (Person.q.name == name)
+        query = (Person.name == name)
         if ignore_merged:
-            query = AND(query, Person.q.mergedID == None)
+            query = And(query, Person.mergedID == None)
         return Person.selectOne(query)
 
     def getByAccount(self, account):
         """See `IPersonSet`."""
         return Person.selectOne(Person.q.accountID == account.id)
 
-    def updateStatistics(self, ztm):
+    def updateStatistics(self):
         """See `IPersonSet`."""
         stats = getUtility(ILaunchpadStatisticSet)
         people_count = Person.select(
-            AND(Person.q.teamownerID == None,
-                Person.q.mergedID == None)).count()
+            And(Person.teamownerID == None,
+                Person.mergedID == None)).count()
         stats.update('people_count', people_count)
-        ztm.commit()
+        transaction.commit()
         teams_count = Person.select(
-            AND(Person.q.teamownerID != None,
+            And(Person.q.teamownerID != None,
                 Person.q.mergedID == None)).count()
         stats.update('teams_count', teams_count)
-        ztm.commit()
+        transaction.commit()
 
     def peopleCount(self):
         """See `IPersonSet`."""

=== modified file 'lib/lp/services/librarianserver/db.py'
--- lib/lp/services/librarianserver/db.py	2012-04-16 23:02:44 +0000
+++ lib/lp/services/librarianserver/db.py	2013-04-30 20:25:32 +0000
@@ -8,8 +8,10 @@
     'Library',
     ]
 
-from sqlobject.sqlbuilder import AND
-from storm.expr import SQL
+from storm.expr import (
+    And,
+    SQL,
+    )
 
 from lp.services.database.sqlbase import session_store
 from lp.services.librarian.model import (
@@ -63,11 +65,10 @@
                 raise LookupError("Token stale/pruned/path mismatch")
             else:
                 restricted = True
-        alias = LibraryFileAlias.selectOne(AND(
-            LibraryFileAlias.q.id==aliasid,
-            LibraryFileAlias.q.contentID==LibraryFileContent.q.id,
-            LibraryFileAlias.q.restricted==restricted,
-            ))
+        alias = LibraryFileAlias.selectOne(And(
+            LibraryFileAlias.id == aliasid,
+            LibraryFileAlias.contentID == LibraryFileContent.q.id,
+            LibraryFileAlias.restricted == restricted))
         if alias is None:
             raise LookupError("No file alias with LibraryFileContent")
         return alias


Follow ups