Guruprasad has proposed merging ~lgp171188/launchpad:fix-snaps-description-new-snap-page into launchpad:master.
Commit message: Update the description for snaps in the new snap page Use the description that is currently in https://snapcraft.io/docs. LP: #2037033 Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #2037033 in Launchpad itself: "The +new-snap text does mention snappy and points to an invalid (404) URL for documentation" https://bugs.launchpad.net/launchpad/+bug/2037033 For more details, see: https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/451926 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:fix-snaps-description-new-snap-page into launchpad:master.
diff --git a/doc/reference/python.rst b/doc/reference/python.rst index a616784..5d877b3 100644 --- a/doc/reference/python.rst +++ b/doc/reference/python.rst @@ -264,6 +264,21 @@ 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 ab762aa..25274f1 100644 --- a/lib/lp/app/doc/batch-navigation.rst +++ b/lib/lp/app/doc/batch-navigation.rst @@ -8,7 +8,8 @@ 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. +storm.zope.interfaces.IResultSet and +storm.zope.interfaces.ISQLObjectResultSet. 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 15f20ea..3a45f35 100644 --- a/lib/lp/blueprints/model/sprint.py +++ b/lib/lp/blueprints/model/sprint.py @@ -425,7 +425,12 @@ class HasSprintsMixin: Subclasses must overwrite this method if it doesn't suit them. """ - table = getattr(self, "__storm_table__") + 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") 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 f619539..808e25b 100644 --- a/lib/lp/services/database/interfaces.py +++ b/lib/lp/services/database/interfaces.py @@ -9,6 +9,7 @@ __all__ = [ "IPrimaryObject", "IPrimaryStore", "IRequestExpired", + "ISQLBase", "IStandbyStore", "IStore", "IStoreSelector", @@ -20,6 +21,7 @@ __all__ = [ from zope.interface import Interface from zope.interface.common.interfaces import IRuntimeError +from zope.schema import Int class IRequestExpired(IRuntimeError): @@ -28,6 +30,15 @@ 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 f604f4c..10a2e1f 100644 --- a/lib/lp/services/database/sqlbase.py +++ b/lib/lp/services/database/sqlbase.py @@ -19,13 +19,19 @@ __all__ = [ "quote_identifier", "reset_store", "session_store", + "SQLBase", "sqlvalues", "StupidCache", ] +<<<<<<< lib/lp/services/database/sqlbase.py +======= + +>>>>>>> lib/lp/services/database/sqlbase.py from datetime import datetime, timezone import psycopg2 +import storm import transaction from psycopg2.extensions import ( ISOLATION_LEVEL_AUTOCOMMIT, @@ -38,18 +44,27 @@ 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. @@ -96,15 +111,168 @@ class StupidCache: return self._cache.keys() -def _get_main_default_store(): - """Return the main store using the default flavor. +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. - 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. + 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. """ - return getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR) + + 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) def get_transaction_timestamp(store): @@ -150,14 +318,26 @@ def quote(x): >>> from datetime import datetime, date, time >>> quote(datetime(2003, 12, 4, 13, 45, 50)) "'2003-12-04 13:45:50'" +<<<<<<< lib/lp/services/database/sqlbase.py >>> quote(datetime(2003, 12, 4, 13, 45, 50, 123456)) "'2003-12-04 13:45:50.123456'" +======= +>>>>>>> lib/lp/services/database/sqlbase.py >>> quote(date(2003, 12, 4)) "'2003-12-04'" >>> quote(time(13, 45, 50)) "'13:45:50'" - sqlrepr() also special-cases set objects. + 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. >>> quote(set([1, 2, 3])) '(1, 2, 3)' @@ -167,6 +347,15 @@ def quote(x): """ if isinstance(x, datetime): return "'%s'" % x +<<<<<<< lib/lp/services/database/sqlbase.py +======= + 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) +>>>>>>> lib/lp/services/database/sqlbase.py return sqlrepr(x, "postgres") @@ -354,7 +543,7 @@ def block_implicit_flushes(func): def block_implicit_flushes_decorator(*args, **kwargs): try: - store = _get_main_default_store() + store = _get_sqlobject_store() except DisallowedStore: return func(*args, **kwargs) store.block_implicit_flushes() @@ -373,7 +562,7 @@ def reset_store(func): try: return func(*args, **kwargs) finally: - _get_main_default_store().reset() + _get_sqlobject_store().reset() return mergeFunctionMetadata(func, reset_store_decorator) @@ -425,7 +614,7 @@ class cursor: """ def __init__(self): - self._connection = _get_main_default_store()._connection + self._connection = _get_sqlobject_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 bd5e645..13fabdc 100644 --- a/lib/lp/services/database/sqlobject/__init__.py +++ b/lib/lp/services/database/sqlobject/__init__.py @@ -7,6 +7,30 @@ 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 = [ ("\\", "\\\\"), @@ -45,7 +69,7 @@ def sqlrepr(value, dbname=None): return repr(value) elif value is None: return "NULL" - elif isinstance(value, (frozenset, list, set, tuple)): + elif isinstance(value, (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 5b61d74..ad25c53 100644 --- a/lib/lp/services/webapp/configure.zcml +++ b/lib/lp/services/webapp/configure.zcml @@ -50,6 +50,10 @@ factory='.batching.FiniteSequenceAdapter' /> <adapter + factory='.batching.FiniteSequenceAdapter' + for='storm.zope.interfaces.ISQLObjectResultSet' /> + + <adapter factory='.batching.BoundReferenceSetAdapter' for='storm.references.BoundReferenceSet' /> @@ -242,6 +246,10 @@ <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 7a5ae12..b7130a2 100644 --- a/lib/lp/services/webapp/database.zcml +++ b/lib/lp/services/webapp/database.zcml @@ -60,6 +60,9 @@ <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 fd6aaa9..2d46805 100644 --- a/lib/lp/services/webapp/snapshot.py +++ b/lib/lp/services/webapp/snapshot.py @@ -19,12 +19,13 @@ HARD_LIMIT_FOR_SNAPSHOT = 1000 @implementer(ISnapshotValueFactory) -@adapter(IResultSet) +@adapter(IResultSet) # And ISQLObjectResultSet. def snapshot_sql_result(value): """Snapshot adapter for the Storm result set.""" - # 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. + # 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. return shortlist( value, longest_expected=100, hardlimit=HARD_LIMIT_FOR_SNAPSHOT ) diff --git a/lib/lp/snappy/templates/snap-new.pt b/lib/lp/snappy/templates/snap-new.pt index 29e8f64..1f15132 100644 --- a/lib/lp/snappy/templates/snap-new.pt +++ b/lib/lp/snappy/templates/snap-new.pt @@ -10,9 +10,9 @@ <div metal:fill-slot="main"> <div> <p> - A snap package is a self-contained application that can be installed - on <a href="https://developer.ubuntu.com/en/snappy/">snappy Ubuntu - Core</a>. Launchpad can build snap packages using <a + Snaps are Linux app packages for desktop, cloud and IoT that are + simple to install, secure, cross-platform, and dependency-free. + Launchpad can build snap packages using <a href="https://developer.ubuntu.com/en/snappy/snapcraft/">snapcraft</a>, from any Git or Bazaar branch on Launchpad that has a <tt>snap/snapcraft.yaml</tt>, <tt>build-aux/snap/snapcraft.yaml</tt>, diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py index 78749c0..5c32866 100644 --- a/lib/lp/soyuz/model/publishing.py +++ b/lib/lp/soyuz/model/publishing.py @@ -34,6 +34,8 @@ 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 @@ -1584,6 +1586,8 @@ 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 : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp