From 6dee4c5942684128f2df4eb6b1a1f0ec978c7224 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Mon, 20 Feb 2017 01:37:04 -0800 Subject: [PATCH] Cleanup _find_related_obj All of the additional lookup logic was essentially duplicating a relationship property of 'load_on_pending=True', which tells SQLAlchemy to lookup the relationship during object creation [1]. So we can dump all of this logic and just use that option. 1. http://docs.sqlalchemy.org/en/latest/orm/relationship_api.html #sqlalchemy.orm.relationship.params.load_on_pending Related: blueprint push-notifications Change-Id: I0e495a50f5cab9b6449825039d7683d77de1e763 --- neutron/db/extra_dhcp_opt/models.py | 2 +- neutron/db/models/allowed_address_pair.py | 2 +- neutron/db/models/dns.py | 3 ++ neutron/db/models/external_net.py | 2 +- neutron/db/models/l3.py | 4 +-- neutron/db/models/l3_attrs.py | 2 +- neutron/db/models/portbinding.py | 2 +- neutron/db/models/securitygroup.py | 4 +-- neutron/db/models/segment.py | 7 ++-- neutron/db/models/subnet_service_type.py | 2 +- neutron/db/models/tag.py | 2 +- neutron/db/models_v2.py | 5 ++- neutron/db/port_security/models.py | 4 +-- neutron/db/qos/models.py | 10 +++--- neutron/services/revisions/revision_plugin.py | 34 ++++++------------- 15 files changed, 40 insertions(+), 45 deletions(-) diff --git a/neutron/db/extra_dhcp_opt/models.py b/neutron/db/extra_dhcp_opt/models.py index 4913ad20730..9510b6f1e08 100644 --- a/neutron/db/extra_dhcp_opt/models.py +++ b/neutron/db/extra_dhcp_opt/models.py @@ -41,6 +41,6 @@ class ExtraDhcpOpt(model_base.BASEV2, model_base.HasId): # Add a relationship to the Port model in order to instruct SQLAlchemy to # eagerly load extra_dhcp_opts bindings ports = orm.relationship( - models_v2.Port, + models_v2.Port, load_on_pending=True, backref=orm.backref("dhcp_opts", lazy='subquery', cascade='delete')) revises_on_change = ('ports', ) diff --git a/neutron/db/models/allowed_address_pair.py b/neutron/db/models/allowed_address_pair.py index 0210b5fef4b..5e7763aff59 100644 --- a/neutron/db/models/allowed_address_pair.py +++ b/neutron/db/models/allowed_address_pair.py @@ -25,7 +25,7 @@ class AllowedAddressPair(model_base.BASEV2): ip_address = sa.Column(sa.String(64), nullable=False, primary_key=True) port = orm.relationship( - models_v2.Port, + models_v2.Port, load_on_pending=True, backref=orm.backref("allowed_address_pairs", lazy="subquery", cascade="delete")) revises_on_change = ('port', ) diff --git a/neutron/db/models/dns.py b/neutron/db/models/dns.py index 0c1a40324c3..b05ee288179 100644 --- a/neutron/db/models/dns.py +++ b/neutron/db/models/dns.py @@ -29,6 +29,7 @@ class NetworkDNSDomain(model_base.BASEV2): # Add a relationship to the Network model in order to instruct # SQLAlchemy to eagerly load this association network = orm.relationship(models_v2.Network, + load_on_pending=True, backref=orm.backref("dns_domain", lazy='joined', uselist=False, @@ -57,6 +58,7 @@ class FloatingIPDNS(model_base.BASEV2): # Add a relationship to the FloatingIP model in order to instruct # SQLAlchemy to eagerly load this association floatingip = orm.relationship(l3_models.FloatingIP, + load_on_pending=True, backref=orm.backref("dns", lazy='joined', uselist=False, @@ -85,6 +87,7 @@ class PortDNS(model_base.BASEV2): # Add a relationship to the Port model in order to instruct # SQLAlchemy to eagerly load this association port = orm.relationship(models_v2.Port, + load_on_pending=True, backref=orm.backref("dns", lazy='joined', uselist=False, diff --git a/neutron/db/models/external_net.py b/neutron/db/models/external_net.py index d132fe7c145..ae8081b90f0 100644 --- a/neutron/db/models/external_net.py +++ b/neutron/db/models/external_net.py @@ -32,7 +32,7 @@ class ExternalNetwork(model_base.BASEV2): # Add a relationship to the Network model in order to instruct # SQLAlchemy to eagerly load this association network = orm.relationship( - models_v2.Network, + models_v2.Network, load_on_pending=True, backref=orm.backref("external", lazy='joined', uselist=False, cascade='delete')) revises_on_change = ('network', ) diff --git a/neutron/db/models/l3.py b/neutron/db/models/l3.py index 4d14c112817..9f19ebab8dd 100644 --- a/neutron/db/models/l3.py +++ b/neutron/db/models/l3.py @@ -57,7 +57,7 @@ class Router(standard_attr.HasStandardAttributes, model_base.BASEV2, sa.ForeignKey("flavors.id"), nullable=True) attached_ports = orm.relationship( RouterPort, - backref='router', + backref=orm.backref('router', load_on_pending=True), lazy='dynamic') l3_agents = orm.relationship( 'Agent', lazy='subquery', viewonly=True, @@ -111,7 +111,7 @@ class RouterRoute(model_base.BASEV2, models_v2.Route): ondelete="CASCADE"), primary_key=True) - router = orm.relationship(Router, + router = orm.relationship(Router, load_on_pending=True, backref=orm.backref("route_list", lazy='subquery', cascade='delete')) diff --git a/neutron/db/models/l3_attrs.py b/neutron/db/models/l3_attrs.py index 2ae8d6ec071..6c30ac2c160 100644 --- a/neutron/db/models/l3_attrs.py +++ b/neutron/db/models/l3_attrs.py @@ -42,7 +42,7 @@ class RouterExtraAttributes(model_base.BASEV2): availability_zone_hints = sa.Column(sa.String(255)) router = orm.relationship( - 'Router', + 'Router', load_on_pending=True, backref=orm.backref("extra_attributes", lazy='joined', uselist=False, cascade='delete')) revises_on_change = ('router', ) diff --git a/neutron/db/models/portbinding.py b/neutron/db/models/portbinding.py index 7cbb63640ab..6a57dd2fa4a 100644 --- a/neutron/db/models/portbinding.py +++ b/neutron/db/models/portbinding.py @@ -26,7 +26,7 @@ class PortBindingPort(model_base.BASEV2): primary_key=True) host = sa.Column(sa.String(255), nullable=False) port = orm.relationship( - models_v2.Port, + models_v2.Port, load_on_pending=True, backref=orm.backref("portbinding", lazy='joined', uselist=False, cascade='delete')) diff --git a/neutron/db/models/securitygroup.py b/neutron/db/models/securitygroup.py index ef943eae2f6..894d6f83485 100644 --- a/neutron/db/models/securitygroup.py +++ b/neutron/db/models/securitygroup.py @@ -58,7 +58,7 @@ class SecurityGroupPortBinding(model_base.BASEV2): # Add a relationship to the Port model in order to instruct SQLAlchemy to # eagerly load security group bindings ports = orm.relationship( - models_v2.Port, + models_v2.Port, load_on_pending=True, backref=orm.backref("security_groups", lazy='joined', cascade='delete')) @@ -85,7 +85,7 @@ class SecurityGroupRule(standard_attr.HasStandardAttributes, model_base.BASEV2, port_range_max = sa.Column(sa.Integer) remote_ip_prefix = sa.Column(sa.String(255)) security_group = orm.relationship( - SecurityGroup, + SecurityGroup, load_on_pending=True, backref=orm.backref('rules', cascade='all,delete', lazy='subquery'), primaryjoin="SecurityGroup.id==SecurityGroupRule.security_group_id") source_group = orm.relationship( diff --git a/neutron/db/models/segment.py b/neutron/db/models/segment.py index 91723757ff9..df37fbf450b 100644 --- a/neutron/db/models/segment.py +++ b/neutron/db/models/segment.py @@ -70,7 +70,8 @@ class SegmentHostMapping(model_base.BASEV2): # Add a relationship to the NetworkSegment model in order to instruct # SQLAlchemy to eagerly load this association network_segment = orm.relationship( - NetworkSegment, backref=orm.backref("segment_host_mapping", - lazy='subquery', - cascade='delete')) + NetworkSegment, load_on_pending=True, + backref=orm.backref("segment_host_mapping", + lazy='subquery', + cascade='delete')) revises_on_change = ('network_segment', ) diff --git a/neutron/db/models/subnet_service_type.py b/neutron/db/models/subnet_service_type.py index 980d4626606..d038dde4a50 100644 --- a/neutron/db/models/subnet_service_type.py +++ b/neutron/db/models/subnet_service_type.py @@ -31,7 +31,7 @@ class SubnetServiceType(model_base.BASEV2): # Service types must be valid device owners, therefore share max length service_type = sa.Column(sa.String( length=db_const.DEVICE_OWNER_FIELD_SIZE)) - subnet = orm.relationship(models_v2.Subnet, + subnet = orm.relationship(models_v2.Subnet, load_on_pending=True, backref=orm.backref('service_types', lazy='subquery', cascade='all, delete-orphan', diff --git a/neutron/db/models/tag.py b/neutron/db/models/tag.py index 252b455bb3b..7e37cfe01a0 100644 --- a/neutron/db/models/tag.py +++ b/neutron/db/models/tag.py @@ -26,6 +26,6 @@ class Tag(model_base.BASEV2): nullable=False, primary_key=True) tag = sa.Column(sa.String(60), nullable=False, primary_key=True) standard_attr = orm.relationship( - 'StandardAttribute', + 'StandardAttribute', load_on_pending=True, backref=orm.backref('tags', lazy='subquery', viewonly=True)) revises_on_change = ('standard_attr', ) diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index deb969fd80f..fb680827101 100644 --- a/neutron/db/models_v2.py +++ b/neutron/db/models_v2.py @@ -78,7 +78,10 @@ class Port(standard_attr.HasStandardAttributes, model_base.BASEV2, name = sa.Column(sa.String(db_const.NAME_FIELD_SIZE)) network_id = sa.Column(sa.String(36), sa.ForeignKey("networks.id"), nullable=False) - fixed_ips = orm.relationship(IPAllocation, backref='port', lazy='subquery', + fixed_ips = orm.relationship(IPAllocation, + backref=orm.backref('port', + load_on_pending=True), + lazy='subquery', cascade='all, delete-orphan', order_by=(IPAllocation.ip_address, IPAllocation.subnet_id)) diff --git a/neutron/db/port_security/models.py b/neutron/db/port_security/models.py index 99b1a749bdd..5c84aa47c45 100644 --- a/neutron/db/port_security/models.py +++ b/neutron/db/port_security/models.py @@ -26,7 +26,7 @@ class PortSecurityBinding(model_base.BASEV2): # Add a relationship to the Port model in order to be to able to # instruct SQLAlchemy to eagerly load port security binding port = orm.relationship( - models_v2.Port, + models_v2.Port, load_on_pending=True, backref=orm.backref("port_security", uselist=False, cascade='delete', lazy='joined')) revises_on_change = ('port',) @@ -42,7 +42,7 @@ class NetworkSecurityBinding(model_base.BASEV2): # SQLAlchemy to eagerly load default port security setting for ports # on this network network = orm.relationship( - models_v2.Network, + models_v2.Network, load_on_pending=True, backref=orm.backref("port_security", uselist=False, cascade='delete', lazy='joined')) revises_on_change = ('network',) diff --git a/neutron/db/qos/models.py b/neutron/db/qos/models.py index 4b5209dd635..2afe4260fef 100644 --- a/neutron/db/qos/models.py +++ b/neutron/db/qos/models.py @@ -48,7 +48,7 @@ class QosNetworkPolicyBinding(model_base.BASEV2): primary_key=True) revises_on_change = ('network', ) network = sa.orm.relationship( - models_v2.Network, + models_v2.Network, load_on_pending=True, backref=sa.orm.backref("qos_policy_binding", uselist=False, cascade='delete', lazy='joined')) @@ -68,7 +68,7 @@ class QosPortPolicyBinding(model_base.BASEV2): primary_key=True) revises_on_change = ('port', ) port = sa.orm.relationship( - models_v2.Port, + models_v2.Port, load_on_pending=True, backref=sa.orm.backref("qos_policy_binding", uselist=False, cascade='delete', lazy='joined')) @@ -83,7 +83,7 @@ class QosBandwidthLimitRule(model_base.HasId, model_base.BASEV2): max_kbps = sa.Column(sa.Integer) max_burst_kbps = sa.Column(sa.Integer) revises_on_change = ('qos_policy', ) - qos_policy = sa.orm.relationship(QosPolicy) + qos_policy = sa.orm.relationship(QosPolicy, load_on_pending=True) class QosDscpMarkingRule(model_base.HasId, model_base.BASEV2): @@ -95,7 +95,7 @@ class QosDscpMarkingRule(model_base.HasId, model_base.BASEV2): unique=True) dscp_mark = sa.Column(sa.Integer) revises_on_change = ('qos_policy', ) - qos_policy = sa.orm.relationship(QosPolicy) + qos_policy = sa.orm.relationship(QosPolicy, load_on_pending=True) class QosMinimumBandwidthRule(model_base.HasId, model_base.BASEV2): @@ -112,7 +112,7 @@ class QosMinimumBandwidthRule(model_base.HasId, model_base.BASEV2): nullable=False, server_default=constants.EGRESS_DIRECTION) revises_on_change = ('qos_policy', ) - qos_policy = sa.orm.relationship(QosPolicy) + qos_policy = sa.orm.relationship(QosPolicy, load_on_pending=True) __table_args__ = ( sa.UniqueConstraint( diff --git a/neutron/services/revisions/revision_plugin.py b/neutron/services/revisions/revision_plugin.py index c68045a5c82..ab32b131c0a 100644 --- a/neutron/services/revisions/revision_plugin.py +++ b/neutron/services/revisions/revision_plugin.py @@ -12,6 +12,7 @@ # under the License. from oslo_log import log as logging +import sqlalchemy from sqlalchemy.orm import exc from sqlalchemy.orm import session as se @@ -79,33 +80,20 @@ class RevisionPlugin(service_base.ServicePluginBase): resource_res['revision_number'] = resource_db.revision_number def _find_related_obj(self, session, obj, relationship_col): - """Find a related object for an object based on relationship column. + """Gets a related object off of a relationship. - Given a relationship column, find the object that corresponds to it - either in the current session or by looking it up if it's not present. + Raises a runtime error if the relationship isn't configured correctly + for revision bumping. """ # first check to see if it's directly attached to the object already related_obj = getattr(obj, relationship_col) if related_obj: return related_obj - rel = getattr(obj.__class__, relationship_col) # get relationship - local_rel_col = list(rel.property.local_columns)[0] - if len(rel.property.local_columns) > 1: - raise RuntimeError(_("Bumping revisions with composite foreign " - "keys not supported")) - related_model = rel.property.mapper.class_ - pk = rel.property.mapper.primary_key[0] - rel_id = getattr(obj, local_rel_col.name) - if not rel_id: - return None - for session_obj in session: - if not isinstance(session_obj, related_model): + for rel in sqlalchemy.inspect(obj).mapper.relationships: + if rel.key != relationship_col: continue - if getattr(session_obj, pk.name) == rel_id: - return session_obj - # object isn't in session so we have to query for it - related_obj = ( - session.query(related_model).filter(pk == rel_id). - first() - ) - return related_obj + if not rel.load_on_pending: + raise RuntimeError(_("revises_on_change relationships must " + "have load_on_pending set to True to " + "bump parent revisions on create: %s"), + relationship_col)