launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30490
[Merge] ~lgp171188/launchpad:fix-snaps-description-new-snap-page into launchpad:master
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
Follow ups
