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
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
