launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30484
[Merge] ~cjwatson/launchpad:remove-sqlbase into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:remove-sqlbase into launchpad:master.
Commit message:
Remove SQLBase
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/451833
It's finally unused. I also simplified `lp.services.database.sqlbase.quote` slightly in the process.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-sqlbase into launchpad:master.
diff --git a/doc/reference/python.rst b/doc/reference/python.rst
index 5d877b3..a616784 100644
--- a/doc/reference/python.rst
+++ b/doc/reference/python.rst
@@ -264,21 +264,6 @@ passes and returns them easier to debug.
Database-related
================
-Storm
------
-
-We use two database ORM (object-relational mapper) APIs in Launchpad, the
-older and deprecated SQLObject API and the new and improved `Storm
-<https://storm.canonical.com>`_ API. All new code should use the Storm API,
-and you are encouraged to convert existing code to Storm as part of your
-tech-debt payments.
-
-.. note::
-
- The SQLObject and Storm ``ResultSet`` interfaces are not compatible, so
- e.g. if you need to ``UNION`` between these two, you will run into
- trouble. We are looking into ways to address this.
-
Field attributes
----------------
diff --git a/lib/lp/app/doc/batch-navigation.rst b/lib/lp/app/doc/batch-navigation.rst
index 25274f1..ab762aa 100644
--- a/lib/lp/app/doc/batch-navigation.rst
+++ b/lib/lp/app/doc/batch-navigation.rst
@@ -8,8 +8,7 @@ This documents and tests the Launchpad-specific elements of its usage.
Note that our use of the batching code relies on the registration of
lp.services.webapp.batching.FiniteSequenceAdapter for
-storm.zope.interfaces.IResultSet and
-storm.zope.interfaces.ISQLObjectResultSet.
+storm.zope.interfaces.IResultSet.
Batch navigation provides a way to navigate batch results in a web
page by providing URL links to the next, previous and numbered pages
diff --git a/lib/lp/blueprints/model/sprint.py b/lib/lp/blueprints/model/sprint.py
index 3a45f35..15f20ea 100644
--- a/lib/lp/blueprints/model/sprint.py
+++ b/lib/lp/blueprints/model/sprint.py
@@ -425,12 +425,7 @@ class HasSprintsMixin:
Subclasses must overwrite this method if it doesn't suit them.
"""
- try:
- table = getattr(self, "__storm_table__")
- except AttributeError:
- # XXX cjwatson 2020-09-10: Remove this once all inheritors have
- # been converted from SQLObject to Storm.
- table = getattr(self, "_table")
+ table = getattr(self, "__storm_table__")
return [
getattr(Specification, table.lower()) == self,
Specification.id == SprintSpecification.specification_id,
diff --git a/lib/lp/services/database/interfaces.py b/lib/lp/services/database/interfaces.py
index 808e25b..f619539 100644
--- a/lib/lp/services/database/interfaces.py
+++ b/lib/lp/services/database/interfaces.py
@@ -9,7 +9,6 @@ __all__ = [
"IPrimaryObject",
"IPrimaryStore",
"IRequestExpired",
- "ISQLBase",
"IStandbyStore",
"IStore",
"IStoreSelector",
@@ -21,7 +20,6 @@ __all__ = [
from zope.interface import Interface
from zope.interface.common.interfaces import IRuntimeError
-from zope.schema import Int
class IRequestExpired(IRuntimeError):
@@ -30,15 +28,6 @@ class IRequestExpired(IRuntimeError):
"""
-# XXX 2007-02-09 jamesh:
-# This derived from sqlos.interfaces.ISQLObject before hand. I don't
-# think it is ever used though ...
-class ISQLBase(Interface):
- """An extension of ISQLObject that provides an ID."""
-
- id = Int(title="The integer ID for the instance")
-
-
#
# Database policies
#
diff --git a/lib/lp/services/database/sqlbase.py b/lib/lp/services/database/sqlbase.py
index 3a2f903..d24dd95 100644
--- a/lib/lp/services/database/sqlbase.py
+++ b/lib/lp/services/database/sqlbase.py
@@ -19,16 +19,13 @@ __all__ = [
"quote_identifier",
"reset_store",
"session_store",
- "SQLBase",
"sqlvalues",
"StupidCache",
]
-
-from datetime import datetime, timezone
+from datetime import timezone
import psycopg2
-import storm
import transaction
from psycopg2.extensions import (
ISOLATION_LEVEL_AUTOCOMMIT,
@@ -41,27 +38,18 @@ from psycopg2.extensions import (
from storm.databases.postgres import compile as postgres_compile
from storm.expr import State
from storm.expr import compile as storm_compile
-from storm.locals import Storm # noqa: B1
-from storm.locals import Store
from storm.zope.interfaces import IZStorm
from twisted.python.util import mergeFunctionMetadata
from zope.component import getUtility
-from zope.interface import implementer
-from zope.security.proxy import removeSecurityProxy
from lp.services.config import dbconfig
from lp.services.database.interfaces import (
DEFAULT_FLAVOR,
MAIN_STORE,
DisallowedStore,
- IPrimaryObject,
- IPrimaryStore,
- ISQLBase,
- IStore,
IStoreSelector,
)
from lp.services.database.sqlobject import sqlrepr
-from lp.services.propertycache import clear_property_cache
# Default we want for scripts, and the PostgreSQL default. Note psycopg1 will
# use SERIALIZABLE unless we override, but psycopg2 will not.
@@ -108,168 +96,15 @@ class StupidCache:
return self._cache.keys()
-def _get_sqlobject_store():
- """Return the store used by the SQLObject compatibility layer."""
- return getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-
-
-class LaunchpadStyle(storm.sqlobject.SQLObjectStyle):
- """A SQLObject style for launchpad.
+def _get_main_default_store():
+ """Return the main store using the default flavor.
- Python attributes and database columns are lowercase.
- Class names and database tables are MixedCase. Using this style should
- simplify SQLBase class definitions since more defaults will be correct.
+ For web requests, the default flavor uses a primary or standby database
+ depending on the type of request (see
+ `lp.services.database.policy.LaunchpadDatabasePolicy`); in all other
+ situations, it uses the primary database.
"""
-
- def pythonAttrToDBColumn(self, attr):
- return attr
-
- def dbColumnToPythonAttr(self, col):
- return col
-
- def pythonClassToDBTable(self, className):
- return className
-
- def dbTableToPythonClass(self, table):
- return table
-
- def idForTable(self, table):
- return "id"
-
- def pythonClassToAttr(self, className):
- return className.lower()
-
- # dsilvers: 20050322: If you take this method out; then RelativeJoin
- # instances in our SQLObject classes cause the following error:
- # AttributeError: 'LaunchpadStyle' object has no attribute
- # 'tableReference'
- def tableReference(self, table):
- """Return the tablename mapped for use in RelativeJoin statements."""
- return table.__str__()
-
-
-@implementer(ISQLBase)
-class SQLBase(storm.sqlobject.SQLObjectBase):
- """Base class emulating SQLObject for legacy database classes."""
-
- _style = LaunchpadStyle()
-
- # Silence warnings in linter script, which complains about all
- # SQLBase-derived objects missing an id.
- id = None
-
- def __init__(self, *args, **kwargs):
- """Extended version of the SQLObjectBase constructor.
-
- We force use of the primary Store.
-
- We refetch any parameters from different stores from the
- correct primary Store.
- """
- # Make it simple to write dumb-invalidators - initialized
- # _cached_properties to a valid list rather than just-in-time
- # creation.
- self._cached_properties = []
- store = IPrimaryStore(self.__class__)
-
- # The constructor will fail if objects from a different Store
- # are passed in. We need to refetch these objects from the correct
- # primary Store if necessary so the foreign key references can be
- # constructed.
- # XXX StuartBishop 2009-03-02 bug=336867: We probably want to remove
- # this code - there are enough other places developers have to be
- # aware of the replication # set boundaries. Why should
- # Person(..., account=an_account) work but
- # some_person.account = an_account fail?
- for key, argument in kwargs.items():
- argument = removeSecurityProxy(argument)
- if not isinstance(argument, Storm): # noqa: B1
- continue
- argument_store = Store.of(argument)
- if argument_store is not store:
- new_argument = store.find(
- argument.__class__, id=argument.id
- ).one()
- assert (
- new_argument is not None
- ), "%s not yet synced to this store" % repr(argument)
- kwargs[key] = new_argument
-
- store.add(self)
- try:
- self._create(None, **kwargs)
- except Exception:
- store.remove(self)
- raise
-
- @classmethod
- def _get_store(cls):
- return IStore(cls)
-
- def __repr__(self):
- return "<%s object>" % (self.__class__.__name__)
-
- def destroySelf(self):
- my_primary = IPrimaryObject(self)
- if self is my_primary:
- super().destroySelf()
- else:
- my_primary.destroySelf()
-
- def __eq__(self, other):
- """Equality operator.
-
- Objects compare equal if they have the same class and id, and the id
- is not None.
-
- This rule allows objects retrieved from different stores to compare
- equal. Newly-created objects may not yet have an id; in such cases
- we flush the store so that we can find out their id.
- """
- naked_self = removeSecurityProxy(self)
- naked_other = removeSecurityProxy(other)
- if naked_self.__class__ != naked_other.__class__:
- return False
- try:
- self_id = naked_self.id
- except KeyError:
- self.syncUpdate()
- self_id = naked_self.id
- if self_id is None:
- return False
- try:
- other_id = naked_other.id
- except KeyError:
- other.syncUpdate()
- other_id = naked_other.id
- return self_id == other_id
-
- def __ne__(self, other):
- """Inverse of __eq__."""
- return not (self == other)
-
- def __hash__(self):
- """Hash operator.
-
- We must define __hash__ since we define __eq__ (Python 3 requires
- this), but we need to take care to preserve the invariant that
- objects that compare equal have the same hash value. Newly-created
- objects may not yet have an id; in such cases we flush the store so
- that we can find out their id.
- """
- try:
- id = self.id
- except KeyError:
- self.syncUpdate()
- id = self.id
- return hash((self.__class__, id))
-
- def __storm_invalidated__(self):
- """Flush cached properties."""
- # XXX: RobertCollins 2010-08-16 bug=622648: Note this is not directly
- # tested, but the entire test suite blows up awesomely if it's broken.
- # It's entirely unclear where tests for this should be.
- clear_property_cache(self)
+ return getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
def get_transaction_timestamp(store):
@@ -314,22 +149,13 @@ def quote(x):
>>> from datetime import datetime, date, time
>>> quote(datetime(2003, 12, 4, 13, 45, 50))
- "'2003-12-04 13:45:50'"
+ "'2003-12-04T13:45:50'"
>>> quote(date(2003, 12, 4))
"'2003-12-04'"
>>> quote(time(13, 45, 50))
"'13:45:50'"
- This function special cases datetime objects, due to a bug that has
- since been fixed in SQLOS (it installed an SQLObject converter that
- stripped the time component from the value). By itself, the sqlrepr
- function has the following output:
-
- >>> sqlrepr(datetime(2003, 12, 4, 13, 45, 50), "postgres")
- "'2003-12-04T13:45:50'"
-
- This function also special cases set objects, which SQLObject's
- sqlrepr() doesn't know how to handle.
+ sqlrepr() also special-cases set objects.
>>> quote(set([1, 2, 3]))
'(1, 2, 3)'
@@ -337,14 +163,6 @@ def quote(x):
>>> quote(frozenset([1, 2, 3]))
'(1, 2, 3)'
"""
- if isinstance(x, datetime):
- return "'%s'" % x
- elif ISQLBase(x, None) is not None:
- return str(x.id)
- elif isinstance(x, (set, frozenset)):
- # SQLObject can't cope with sets, so convert to a list, which it
- # /does/ know how to handle.
- x = list(x)
return sqlrepr(x, "postgres")
@@ -532,7 +350,7 @@ def block_implicit_flushes(func):
def block_implicit_flushes_decorator(*args, **kwargs):
try:
- store = _get_sqlobject_store()
+ store = _get_main_default_store()
except DisallowedStore:
return func(*args, **kwargs)
store.block_implicit_flushes()
@@ -551,7 +369,7 @@ def reset_store(func):
try:
return func(*args, **kwargs)
finally:
- _get_sqlobject_store().reset()
+ _get_main_default_store().reset()
return mergeFunctionMetadata(func, reset_store_decorator)
@@ -603,7 +421,7 @@ class cursor:
"""
def __init__(self):
- self._connection = _get_sqlobject_store()._connection
+ self._connection = _get_main_default_store()._connection
self._result = None
def execute(self, query, params=None):
diff --git a/lib/lp/services/database/sqlobject/__init__.py b/lib/lp/services/database/sqlobject/__init__.py
index 13fabdc..bd5e645 100644
--- a/lib/lp/services/database/sqlobject/__init__.py
+++ b/lib/lp/services/database/sqlobject/__init__.py
@@ -7,30 +7,6 @@
import datetime
from storm.expr import SQL
-from storm.sqlobject import ( # noqa: F401
- AND,
- CONTAINSSTRING,
- DESC,
- IN,
- LIKE,
- NOT,
- OR,
- BoolCol,
- DateCol,
- FloatCol,
- ForeignKey,
- IntCol,
- IntervalCol,
- SingleJoin,
- SQLConstant,
- SQLMultipleJoin,
- SQLObjectBase,
- SQLObjectMoreThanOneResultError,
- SQLObjectResultSet,
- SQLRelatedJoin,
- StringCol,
- UtcDateTimeCol,
-)
_sqlStringReplace = [
("\\", "\\\\"),
@@ -69,7 +45,7 @@ def sqlrepr(value, dbname=None):
return repr(value)
elif value is None:
return "NULL"
- elif isinstance(value, (list, set, tuple)):
+ elif isinstance(value, (frozenset, list, set, tuple)):
return "(%s)" % ", ".join(sqlrepr(v, dbname) for v in value)
elif isinstance(value, datetime.datetime):
return value.strftime("'%Y-%m-%dT%H:%M:%S'")
diff --git a/lib/lp/services/webapp/configure.zcml b/lib/lp/services/webapp/configure.zcml
index ad25c53..5b61d74 100644
--- a/lib/lp/services/webapp/configure.zcml
+++ b/lib/lp/services/webapp/configure.zcml
@@ -50,10 +50,6 @@
factory='.batching.FiniteSequenceAdapter' />
<adapter
- factory='.batching.FiniteSequenceAdapter'
- for='storm.zope.interfaces.ISQLObjectResultSet' />
-
- <adapter
factory='.batching.BoundReferenceSetAdapter'
for='storm.references.BoundReferenceSet' />
@@ -246,10 +242,6 @@
<adapter
factory="lp.services.webapp.snapshot.snapshot_sql_result" />
- <!-- It also works for the legacy SQLObject interface. -->
- <adapter
- factory="lp.services.webapp.snapshot.snapshot_sql_result"
- for="storm.zope.interfaces.ISQLObjectResultSet" />
<class class="lp.services.webapp.publisher.RenamedView">
<allow interface="zope.publisher.interfaces.browser.IBrowserPublisher"
diff --git a/lib/lp/services/webapp/database.zcml b/lib/lp/services/webapp/database.zcml
index b7130a2..7a5ae12 100644
--- a/lib/lp/services/webapp/database.zcml
+++ b/lib/lp/services/webapp/database.zcml
@@ -60,9 +60,6 @@
<implements interface="lp.services.database.interfaces.IStore" />
<allow attributes="get" />
</class>
- <class class="lp.services.database.sqlbase.SQLBase">
- <implements interface="lp.services.database.interfaces.IDBObject" />
- </class>
<class class="lp.services.database.stormbase.StormBase">
<implements interface="lp.services.database.interfaces.IDBObject" />
</class>
diff --git a/lib/lp/services/webapp/snapshot.py b/lib/lp/services/webapp/snapshot.py
index 2d46805..fd6aaa9 100644
--- a/lib/lp/services/webapp/snapshot.py
+++ b/lib/lp/services/webapp/snapshot.py
@@ -19,13 +19,12 @@ HARD_LIMIT_FOR_SNAPSHOT = 1000
@implementer(ISnapshotValueFactory)
-@adapter(IResultSet) # And ISQLObjectResultSet.
+@adapter(IResultSet)
def snapshot_sql_result(value):
"""Snapshot adapter for the Storm result set."""
- # SQLMultipleJoin and SQLRelatedJoin return
- # SelectResults, which doesn't really help the Snapshot
- # object. We therefore list()ify the values; this isn't
- # perfect but allows deltas to be generated reliably.
+ # ReferenceSet returns ResultSets, which doesn't really help the
+ # Snapshot object. We therefore list()ify the values; this isn't perfect
+ # but allows deltas to be generated reliably.
return shortlist(
value, longest_expected=100, hardlimit=HARD_LIMIT_FOR_SNAPSHOT
)
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index 5c32866..78749c0 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -34,8 +34,6 @@ from storm.info import ClassAlias
from storm.properties import DateTime, Int, Unicode
from storm.references import Reference
from storm.store import Store
-from storm.zope import IResultSet
-from storm.zope.interfaces import ISQLObjectResultSet
from zope.component import getUtility
from zope.interface import implementer
from zope.security.proxy import isinstance as zope_isinstance
@@ -1586,8 +1584,6 @@ class PublishingSet:
if len(bpphs) == 0:
return
else:
- if ISQLObjectResultSet.providedBy(bpphs):
- bpphs = IResultSet(bpphs)
if bpphs.is_empty():
return
