From 14bc0d940841de31f055fa3170289e0fa671b7bb Mon Sep 17 00:00:00 2001 From: Boden R Date: Fri, 14 Dec 2018 12:05:29 -0700 Subject: [PATCH] rehome db api orm event listener functions The rehome/consumption of the db api caused some errors in consumer projects related to the ORM event listeners no longer getting initialized [1]. While the short term fix [1] was to import neutron's db api elsewhere, this doesn't work longer term as consumers need to decouple from neutron, thus not importing neutron modules. This patch rehomes the db api ORM event listeners into neutron-lib and initializes them upon import of neutron_lib (top-level). This change will allow consumers to load the event listeners by importing anything from neutron-lib, thus breaking the dependency on neutron. This patch also bumps the requirement for SQLAlchemy to match neutrons. [1] https://bugs.launchpad.net/neutron/+bug/1802369 Related-Bug: 1802369 Change-Id: I3e702b99fd5084e8090f93c384aa1f704edceaff --- lower-constraints.txt | 2 +- neutron_lib/__init__.py | 5 + neutron_lib/db/api.py | 121 ++++++++++++++++++ ...-api-event-listeners-2fb5256166e2a4e8.yaml | 5 + requirements.txt | 2 +- 5 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/rehome-db-api-event-listeners-2fb5256166e2a4e8.yaml diff --git a/lower-constraints.txt b/lower-constraints.txt index 9f8a00f12..af92a55bb 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -82,7 +82,7 @@ six==1.10.0 snowballstemmer==1.2.1 Sphinx==1.6.2 sphinxcontrib-websupport==1.0.1 -SQLAlchemy==1.0.10 +SQLAlchemy==1.2.0 sqlalchemy-migrate==0.11.0 sqlparse==0.2.2 statsd==3.2.1 diff --git a/neutron_lib/__init__.py b/neutron_lib/__init__.py index b1a9779a3..1f79bbc32 100644 --- a/neutron_lib/__init__.py +++ b/neutron_lib/__init__.py @@ -12,6 +12,11 @@ import pbr.version +from neutron_lib.db import api # noqa + +# NOTE(boden): neutron_lib.db.api is imported to ensure the ORM event listeners +# are registered upon importing any neutron-lib module. For more details see +# defect https://bugs.launchpad.net/networking-ovn/+bug/1802369 __version__ = pbr.version.VersionInfo( 'neutron_lib').version_string() diff --git a/neutron_lib/db/api.py b/neutron_lib/db/api.py index f66aa0178..874f0f822 100644 --- a/neutron_lib/db/api.py +++ b/neutron_lib/db/api.py @@ -32,6 +32,7 @@ from sqlalchemy import orm from sqlalchemy.orm import exc from neutron_lib._i18n import _ +from neutron_lib.db import model_base from neutron_lib import exceptions from neutron_lib.objects import exceptions as obj_exc @@ -349,3 +350,123 @@ def _load_one_to_manys(session): msg = ("Relationship %s attributes must be loaded in db " "object %s" % (relationship_attr.key, state.dict)) raise AssertionError(msg) + + +# Expire relationships when foreign key changes. +# +# NOTE(ihrachys) Arguably, it's a sqlalchemy anti-pattern to access child +# models directly and through parent relationships in the same session. But +# since OVO mechanism is built around synthetic fields that assume this mixed +# access is possible, we keep it here until we find a way to migrate OVO +# synthetic fields to better mechanism that would update child models via +# parents. Even with that, there are multiple places in plugin code where we +# mix access when using models directly; those occurrences would need to be +# fixed too to be able to remove this hook and explicit expire() calls. +# +# Adopted from the following recipe: +# https://bitbucket.org/zzzeek/sqlalchemy/wiki/UsageRecipes +# /ExpireRelationshipOnFKChange +# +# ...then massively changed to actually work for all neutron backref cases. +# +# TODO(ihrachys) at some point these event handlers should be extended to also +# automatically refresh values for expired attributes +def _expire_for_fk_change(target, fk_value, relationship_prop, column_attr): + """Expire relationship attributes when a many-to-one column changes.""" + + sess = orm.object_session(target) + + # subnets and network's many-to-one relationship is used as example in the + # comments in this function + if sess is not None: + # optional behavior #1 - expire the "Network.subnets" + # collection on the existing "network" object + if relationship_prop.back_populates and \ + relationship_prop.key in target.__dict__: + obj = getattr(target, relationship_prop.key) + if obj is not None and sqlalchemy.inspect(obj).persistent: + sess.expire(obj, [relationship_prop.back_populates]) + + # optional behavior #2 - expire the "Subnet.network" + if sqlalchemy.inspect(target).persistent: + sess.expire(target, [relationship_prop.key]) + + # optional behavior #3 - "trick" the ORM by actually + # setting the value ahead of time, then emitting a load + # for the attribute so that the *new* Subnet.network + # is loaded. Then, expire Network.subnets on *that*. + # Other techniques here including looking in the identity + # map for "value", if this is a simple many-to-one get. + if relationship_prop.back_populates: + target.__dict__[column_attr] = fk_value + new = getattr(target, relationship_prop.key) + if new is not None: + if sqlalchemy.inspect(new).persistent: + sess.expire(new, [relationship_prop.back_populates]) + else: + # no Session yet, do it later. This path is reached from the 'expire' + # listener setup by '_expire_prop_on_col' below, when a foreign key + # is directly assigned to in the many to one side of a relationship. + # i.e. assigning directly to Subnet.network_id before Subnet is added + # to the session + if target not in _emit_on_pending: + _emit_on_pending[target] = [] + _emit_on_pending[target].append( + (fk_value, relationship_prop, column_attr)) + + +_emit_on_pending = weakref.WeakKeyDictionary() + + +@event.listens_for(orm.session.Session, "pending_to_persistent") +def _pending_callables(session, obj): + """Expire relationships when a new object w/ a FK becomes persistent""" + if obj is None: + return + args = _emit_on_pending.pop(obj, []) + for a in args: + if a is not None: + _expire_for_fk_change(obj, *a) + + +@event.listens_for(orm.session.Session, "persistent_to_deleted") +def _persistent_to_deleted(session, obj): + """Expire relationships when an object w/ a foreign key becomes deleted""" + mapper = sqlalchemy.inspect(obj).mapper + for prop in mapper.relationships: + if prop.direction is orm.interfaces.MANYTOONE: + for col in prop.local_columns: + colkey = mapper.get_property_by_column(col).key + _expire_for_fk_change(obj, None, prop, colkey) + + +@event.listens_for(model_base.BASEV2, "attribute_instrument", propagate=True) +def _listen_for_changes(cls, key, inst): + mapper = sqlalchemy.inspect(cls) + if key not in mapper.relationships: + return + prop = inst.property + + if prop.direction is orm.interfaces.MANYTOONE: + for col in prop.local_columns: + colkey = mapper.get_property_by_column(col).key + _expire_prop_on_col(cls, prop, colkey) + elif prop.direction is orm.interfaces.ONETOMANY: + remote_mapper = prop.mapper + # the collection *has* to have a MANYTOONE backref so we + # can look up the parent. so here we make one if it doesn't + # have it already, as is the case in this example + if not prop.back_populates: + name = "_%s_backref" % prop.key + backref_prop = orm.relationship( + prop.parent, back_populates=prop.key) + + remote_mapper.add_property(name, backref_prop) + prop.back_populates = name + + +def _expire_prop_on_col(cls, prop, colkey): + @event.listens_for(getattr(cls, colkey), "set") + def expire(target, value, oldvalue, initiator): + """Expire relationships when FK attribute on an object changes""" + _expire_for_fk_change(target, value, prop, colkey) diff --git a/releasenotes/notes/rehome-db-api-event-listeners-2fb5256166e2a4e8.yaml b/releasenotes/notes/rehome-db-api-event-listeners-2fb5256166e2a4e8.yaml new file mode 100644 index 000000000..0b13f69b8 --- /dev/null +++ b/releasenotes/notes/rehome-db-api-event-listeners-2fb5256166e2a4e8.yaml @@ -0,0 +1,5 @@ +--- +features: + - The private ORM event listener functions from ``neutron.db.api`` are now in + ``neutron_lib.db.api`` and are automatically loaded when importing any + neutron-lib module. diff --git a/requirements.txt b/requirements.txt index 730fdaaba..8e09ff9e6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 -SQLAlchemy!=1.1.5,!=1.1.6,!=1.1.7,!=1.1.8,>=1.0.10 # MIT +SQLAlchemy>=1.2.0 # MIT pecan!=1.0.2,!=1.0.3,!=1.0.4,!=1.2,>=1.0.0 # BSD keystoneauth1>=3.4.0 # Apache-2.0 six>=1.10.0 # MIT