From 031eda582642a635286262959d8ca729d34ee82c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 6 Oct 2022 12:56:11 +0100 Subject: [PATCH] db: Replace use of backref Per the SQLAlchemy docs [1]: The relationship.backref keyword should be considered legacy, and use of relationship.back_populates with explicit relationship() constructs should be preferred. A number of the relationships defined here don't have foreign keys (long live mordred?) so their conversion is slightly more difficult than would otherwise be the case. A blog post is available to explain what's going on [2] and might be worth a read. The learnings from that blog post do have the benefit of allowing us to simplify some existing relationships that had unnecessary arguments defined. [1] https://docs.sqlalchemy.org/en/14/orm/backref.html [2] https://that.guru/blog/sqlalchemy-relationships-without-foreign-keys/ Change-Id: I5a135b012dabdff7cf06204fc3c5438aaa0985c9 Signed-off-by: Stephen Finucane --- nova/db/api/models.py | 51 ++++++------- nova/db/main/models.py | 159 ++++++++++++++++++++++++++++++++++------- 2 files changed, 160 insertions(+), 50 deletions(-) diff --git a/nova/db/api/models.py b/nova/db/api/models.py index f3175a0b688b..5a236e98f16c 100644 --- a/nova/db/api/models.py +++ b/nova/db/api/models.py @@ -147,11 +147,15 @@ class CellMapping(BASE): transport_url = sa.Column(sa.Text()) database_connection = sa.Column(sa.Text()) disabled = sa.Column(sa.Boolean, default=False) + host_mapping = orm.relationship( 'HostMapping', - backref=orm.backref('cell_mapping', uselist=False), - foreign_keys=id, - primaryjoin='CellMapping.id == HostMapping.cell_id') + back_populates='cell_mapping', + ) + instance_mapping = orm.relationship( + 'InstanceMapping', + back_populates='cell_mapping', + ) class InstanceMapping(BASE): @@ -178,11 +182,11 @@ class InstanceMapping(BASE): # transition period first. user_id = sa.Column(sa.String(255), nullable=True) queued_for_delete = sa.Column(sa.Boolean) + cell_mapping = orm.relationship( 'CellMapping', - backref=orm.backref('instance_mapping', uselist=False), - foreign_keys=cell_id, - primaryjoin='InstanceMapping.cell_id == CellMapping.id') + back_populates='instance_mapping', + ) class HostMapping(BASE): @@ -198,6 +202,11 @@ class HostMapping(BASE): sa.Integer, sa.ForeignKey('cell_mappings.id'), nullable=False) host = sa.Column(sa.String(255), nullable=False) + cell_mapping = orm.relationship( + 'CellMapping', + back_populates='host_mapping', + ) + class RequestSpec(BASE): """Represents the information passed to the scheduler.""" @@ -235,6 +244,9 @@ class Flavors(BASE): is_public = sa.Column(sa.Boolean, default=True) description = sa.Column(sa.Text) + extra_specs = orm.relationship('FlavorExtraSpecs', back_populates='flavor') + projects = orm.relationship('FlavorProjects', back_populates='flavor') + class FlavorExtraSpecs(BASE): """Represents additional specs as key/value pairs for a flavor""" @@ -251,10 +263,8 @@ class FlavorExtraSpecs(BASE): value = sa.Column(sa.String(255)) flavor_id = sa.Column( sa.Integer, sa.ForeignKey('flavors.id'), nullable=False) - flavor = orm.relationship( - Flavors, backref='extra_specs', - foreign_keys=flavor_id, - primaryjoin='FlavorExtraSpecs.flavor_id == Flavors.id') + + flavor = orm.relationship(Flavors, back_populates='extra_specs') class FlavorProjects(BASE): @@ -267,10 +277,8 @@ class FlavorProjects(BASE): flavor_id = sa.Column( sa.Integer, sa.ForeignKey('flavors.id'), nullable=False) project_id = sa.Column(sa.String(255), nullable=False) - flavor = orm.relationship( - Flavors, backref='projects', - foreign_keys=flavor_id, - primaryjoin='FlavorProjects.flavor_id == Flavors.id') + + flavor = orm.relationship(Flavors, back_populates='projects') class BuildRequest(BASE): @@ -354,12 +362,9 @@ class InstanceGroup(BASE): project_id = sa.Column(sa.String(255)) uuid = sa.Column(sa.String(36), nullable=False) name = sa.Column(sa.String(255)) - _policies = orm.relationship( - InstanceGroupPolicy, - primaryjoin='InstanceGroup.id == InstanceGroupPolicy.group_id') - _members = orm.relationship( - InstanceGroupMember, - primaryjoin='InstanceGroup.id == InstanceGroupMember.group_id') + + _policies = orm.relationship(InstanceGroupPolicy) + _members = orm.relationship(InstanceGroupMember) @property def policy(self): @@ -487,7 +492,5 @@ class Reservation(BASE): resource = sa.Column(sa.String(255)) delta = sa.Column(sa.Integer, nullable=False) expire = sa.Column(sa.DateTime) - usage = orm.relationship( - "QuotaUsage", - foreign_keys=usage_id, - primaryjoin='Reservation.usage_id == QuotaUsage.id') + + usage = orm.relationship('QuotaUsage') diff --git a/nova/db/main/models.py b/nova/db/main/models.py index 0da6aac05c0b..bce0d3f2e59c 100644 --- a/nova/db/main/models.py +++ b/nova/db/main/models.py @@ -161,8 +161,8 @@ class Service(BASE, NovaBase, models.SoftDeleteMixin): version = sa.Column(sa.Integer, default=0) instance = orm.relationship( - "Instance", - backref='services', + 'Instance', + back_populates='services', primaryjoin='and_(Service.host == Instance.host,' 'Service.binary == "nova-compute",' 'Instance.deleted == 0)', @@ -425,6 +425,96 @@ class Instance(BASE, NovaBase, models.SoftDeleteMixin): hidden = sa.Column(sa.Boolean, default=False) + block_device_mapping = orm.relationship( + 'BlockDeviceMapping', + back_populates='instance', + primaryjoin=( + 'and_(BlockDeviceMapping.instance_uuid == Instance.uuid, ' + 'BlockDeviceMapping.deleted == 0)' + ), + ) + console_auth_tokens = orm.relationship( + 'ConsoleAuthToken', + back_populates='instance', + foreign_keys='ConsoleAuthToken.instance_uuid', + primaryjoin=( + 'and_(Instance.uuid == ConsoleAuthToken.instance_uuid,' + 'Instance.deleted == 0)' + ), + ) + extra = orm.relationship( + 'InstanceExtra', + back_populates='instance', + uselist=False, + ) + info_cache = orm.relationship( + 'InstanceInfoCache', + back_populates='instance', + uselist=False, + ) + pci_devices = orm.relationship( + 'PciDevice', + back_populates='instance', + foreign_keys='PciDevice.instance_uuid', + primaryjoin=( + 'and_(Instance.uuid == PciDevice.instance_uuid,' + 'PciDevice.deleted == 0)' + ), + ) + services = orm.relationship( + 'Service', + back_populates='instance', + primaryjoin=( + 'and_(Instance.host == Service.host,' + 'Service.binary == "nova-compute",' + 'Instance.deleted == 0)' + ), + foreign_keys='Service.host', + ) + security_groups = orm.relationship( + 'SecurityGroup', + secondary='security_group_instance_association', + back_populates='instances', + primaryjoin=( + 'and_(' + 'SecurityGroupInstanceAssociation.instance_uuid == Instance.uuid,' + # (anthony) the condition below shouldn't be necessary now that the + # association is being marked as deleted. However, removing this + # may cause existing deployments to choke, so I'm leaving it + 'Instance.deleted == 0)' + ), + secondaryjoin=( + 'and_(' + 'SecurityGroup.id == SecurityGroupInstanceAssociation.security_group_id,' # noqa: E501 + 'SecurityGroupInstanceAssociation.deleted == 0,' + 'SecurityGroup.deleted == 0)' + ), + ) + system_metadata = orm.relationship( + 'InstanceSystemMetadata', + back_populates='instance', + ) + tags = orm.relationship( + 'Tag', + back_populates='instance', + primaryjoin=( + 'and_(Instance.uuid == Tag.resource_id,Instance.deleted == 0)' + ), + foreign_keys='Tag.resource_id', + ) + + +# NOTE(stephenfin): https://github.com/sqlalchemy/sqlalchemy/discussions/8619 +Instance.metadata = orm.relationship( + 'InstanceMetadata', + back_populates='instance', + foreign_keys='InstanceMetadata.instance_uuid', + primaryjoin=( + 'and_(Instance.uuid == InstanceMetadata.instance_uuid,' + 'InstanceMetadata.deleted == 0)' + ), +) + class InstanceInfoCache(BASE, NovaBase, models.SoftDeleteMixin): """Represents a cache of information about an instance @@ -442,7 +532,7 @@ class InstanceInfoCache(BASE, NovaBase, models.SoftDeleteMixin): instance_uuid = sa.Column(sa.String(36), sa.ForeignKey('instances.uuid'), nullable=False) instance = orm.relationship(Instance, - backref=orm.backref('info_cache', uselist=False), + back_populates='info_cache', foreign_keys=instance_uuid, primaryjoin=instance_uuid == Instance.uuid) @@ -466,8 +556,7 @@ class InstanceExtra(BASE, NovaBase, models.SoftDeleteMixin): # and can be removed in the future release. resources = orm.deferred(sa.Column(sa.Text)) instance = orm.relationship(Instance, - backref=orm.backref('extra', - uselist=False), + back_populates='extra', foreign_keys=instance_uuid, primaryjoin=instance_uuid == Instance.uuid) @@ -616,7 +705,7 @@ class BlockDeviceMapping(BASE, NovaBase, models.SoftDeleteMixin): # transition period first. uuid = sa.Column(sa.String(36)) instance = orm.relationship(Instance, - backref=orm.backref('block_device_mapping'), + back_populates='block_device_mapping', foreign_keys=instance_uuid, primaryjoin='and_(BlockDeviceMapping.' 'instance_uuid==' @@ -664,10 +753,9 @@ class BlockDeviceMapping(BASE, NovaBase, models.SoftDeleteMixin): encryption_format = sa.Column(sa.String(128)) encryption_options = sa.Column(sa.String(4096)) + # TODO(stephenfin): Remove once we drop the security_groups field from the # Instance table. Until then, this is tied to the SecurityGroup table - - class SecurityGroupInstanceAssociation(BASE, NovaBase, models.SoftDeleteMixin): __tablename__ = 'security_group_instance_association' __table_args__ = ( @@ -698,6 +786,7 @@ class SecurityGroup(BASE, NovaBase, models.SoftDeleteMixin): project_id = sa.Column(sa.String(255)) instances = orm.relationship(Instance, + back_populates='security_groups', secondary = "security_group_instance_association", primaryjoin = 'and_(' 'SecurityGroup.id == ' @@ -709,8 +798,17 @@ class SecurityGroup(BASE, NovaBase, models.SoftDeleteMixin): # (anthony) the condition below shouldn't be necessary now that the # association is being marked as deleted. However, removing this # may cause existing deployments to choke, so I'm leaving it - 'Instance.deleted == 0)', - backref='security_groups') + 'Instance.deleted == 0)') + + rules = orm.relationship( + 'SecurityGroupIngressRule', + primaryjoin=( + 'and_(' + 'SecurityGroupIngressRule.parent_group_id == SecurityGroup.id,' + 'SecurityGroupIngressRule.deleted == 0)' + ), + back_populates='parent_group', + ) # TODO(stephenfin): Remove once we drop the security_groups field from the @@ -723,8 +821,9 @@ class SecurityGroupIngressRule(BASE, NovaBase, models.SoftDeleteMixin): parent_group_id = sa.Column( sa.Integer, sa.ForeignKey('security_groups.id')) - parent_group = orm.relationship("SecurityGroup", backref="rules", - foreign_keys=parent_group_id, + parent_group = orm.relationship("SecurityGroup", + back_populates="rules", + foreign_keys=[parent_group_id], primaryjoin='and_(' 'SecurityGroupIngressRule.parent_group_id == SecurityGroup.id,' 'SecurityGroupIngressRule.deleted == 0)') @@ -738,7 +837,7 @@ class SecurityGroupIngressRule(BASE, NovaBase, models.SoftDeleteMixin): # granting access for. group_id = sa.Column(sa.Integer, sa.ForeignKey('security_groups.id')) grantee_group = orm.relationship("SecurityGroup", - foreign_keys=group_id, + foreign_keys=[group_id], primaryjoin='and_(' 'SecurityGroupIngressRule.group_id == SecurityGroup.id,' 'SecurityGroupIngressRule.deleted == 0)') @@ -824,12 +923,16 @@ class InstanceMetadata(BASE, NovaBase, models.SoftDeleteMixin): key = sa.Column(sa.String(255)) value = sa.Column(sa.String(255)) instance_uuid = sa.Column(sa.String(36), sa.ForeignKey('instances.uuid')) - instance = orm.relationship(Instance, backref="metadata", - foreign_keys=instance_uuid, - primaryjoin='and_(' - 'InstanceMetadata.instance_uuid == ' - 'Instance.uuid,' - 'InstanceMetadata.deleted == 0)') + + instance = orm.relationship( + Instance, + back_populates="metadata", + foreign_keys=instance_uuid, + primaryjoin=( + 'and_(InstanceMetadata.instance_uuid == Instance.uuid,' + 'InstanceMetadata.deleted == 0)' + ), + ) class InstanceSystemMetadata(BASE, NovaBase, models.SoftDeleteMixin): @@ -845,8 +948,11 @@ class InstanceSystemMetadata(BASE, NovaBase, models.SoftDeleteMixin): sa.ForeignKey('instances.uuid'), nullable=False) - instance = orm.relationship(Instance, backref="system_metadata", - foreign_keys=instance_uuid) + instance = orm.relationship( + Instance, + back_populates='system_metadata', + foreign_keys=instance_uuid, + ) class VolumeUsage(BASE, NovaBase, models.SoftDeleteMixin): @@ -1022,7 +1128,8 @@ class PciDevice(BASE, NovaBase, models.SoftDeleteMixin): numa_node = sa.Column(sa.Integer, nullable=True) parent_addr = sa.Column(sa.String(12), nullable=True) - instance = orm.relationship(Instance, backref="pci_devices", + instance = orm.relationship(Instance, + back_populates="pci_devices", foreign_keys=instance_uuid, primaryjoin='and_(' 'PciDevice.instance_uuid == Instance.uuid,' @@ -1040,11 +1147,11 @@ class Tag(BASE, models.ModelBase): tag = sa.Column(sa.Unicode(80), primary_key=True, nullable=False) instance = orm.relationship( - "Instance", - backref='tags', + 'Instance', + back_populates='tags', + foreign_keys=resource_id, primaryjoin='and_(Tag.resource_id == Instance.uuid,' 'Instance.deleted == 0)', - foreign_keys=resource_id ) @@ -1074,7 +1181,7 @@ class ConsoleAuthToken(BASE, NovaBase): instance = orm.relationship( "Instance", - backref='console_auth_tokens', + back_populates='console_auth_tokens', primaryjoin='and_(ConsoleAuthToken.instance_uuid == Instance.uuid,' 'Instance.deleted == 0)', foreign_keys=instance_uuid