launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15527
[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