openstack
/
manila
Code Issues Proposed changes

db: Don't use strings to indicate relationship names 

Resolve the following RemovedIn20Warning warning:

  Using strings to indicate relationship names in Query.join() is
  deprecated and will be removed in SQLAlchemy 2.0.  Please use the
  class-bound attribute directly.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Change-Id: I155b4ce4b605720c8335d465124fd32cc973a737
This commit is contained in:
Stephen Finucane 2024-04-03 13:32:43 +01:00
parent 96274a077a
commit 9c94a9adfc
2 changed files with 128 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

216
manila/db/sqlalchemy/api.py
View File

@ -44,8 +44,7 @@ from oslo_utils import uuidutils
from sqlalchemy import and_ from sqlalchemy import and_
from sqlalchemy import MetaData from sqlalchemy import MetaData
from sqlalchemy import or_ from sqlalchemy import or_
from sqlalchemy.orm import joinedload from sqlalchemy import orm
from sqlalchemy.orm import subqueryload
from sqlalchemy.sql.expression import false from sqlalchemy.sql.expression import false
from sqlalchemy.sql.expression import literal from sqlalchemy.sql.expression import literal
from sqlalchemy.sql.expression import true from sqlalchemy.sql.expression import true
@ -1755,8 +1754,10 @@ def _share_instance_get(context, share_instance_id, with_share_data=False):
).filter_by( ).filter_by(
id=share_instance_id, id=share_instance_id,
).options( ).options(
joinedload('export_locations').joinedload('_el_metadata_bare'), orm.joinedload(
joinedload('share_type'), models.ShareInstance.export_locations
).joinedload(models.ShareInstanceExportLocations._el_metadata_bare),
orm.joinedload(models.ShareInstance.share_type),
).first() ).first()
if result is None: if result is None:
raise exception.NotFound() raise exception.NotFound()
@ -1779,7 +1780,7 @@ def _share_instance_get_all(context, filters=None):
query = model_query( query = model_query(
context, models.ShareInstance, read_deleted="no", context, models.ShareInstance, read_deleted="no",
).options( ).options(
joinedload('export_locations'), orm.joinedload(models.ShareInstance.export_locations),
) )
filters = filters or {} filters = filters or {}
@ -1802,8 +1803,8 @@ def _share_instance_get_all(context, filters=None):
query = query.join( query = query.join(
models.Share, models.Share,
models.Share.id == models.Share.id == models.ShareInstance.share_id,
models.ShareInstance.share_id) )
is_soft_deleted = filters.get('is_soft_deleted') is_soft_deleted = filters.get('is_soft_deleted')
if is_soft_deleted: if is_soft_deleted:
query = query.filter(models.Share.is_soft_deleted == true()) query = query.filter(models.Share.is_soft_deleted == true())
@ -2047,8 +2048,10 @@ def _share_replica_get_with_filters(context, share_id=None, replica_id=None,
if not context.is_admin: if not context.is_admin:
query = query.join( query = query.join(
models.Share, models.Share,
models.ShareInstance.share_id == models.Share.id).filter( models.ShareInstance.share_id == models.Share.id
models.Share.project_id == context.project_id) ).filter(
models.Share.project_id == context.project_id,
)
if share_id is not None: if share_id is not None:
query = query.filter(models.ShareInstance.share_id == share_id) query = query.filter(models.ShareInstance.share_id == share_id)
@ -2066,7 +2069,9 @@ def _share_replica_get_with_filters(context, share_id=None, replica_id=None,
query = query.filter(models.ShareInstance.status == status) query = query.filter(models.ShareInstance.status == status)
if with_share_server: if with_share_server:
query = query.options(joinedload('share_server')) query = query.options(
orm.joinedload(models.ShareInstance.share_server),
)
return query return query
@ -2239,7 +2244,8 @@ def _process_share_filters(query, filters, project_id=None, is_public=False):
query = query.join( query = query.join(
models.ShareInstanceExportLocations, models.ShareInstanceExportLocations,
models.ShareInstanceExportLocations.share_instance_id == models.ShareInstanceExportLocations.share_instance_id ==
models.ShareInstance.id) models.ShareInstance.id,
)
if export_location_path: if export_location_path:
query = query.filter( query = query.filter(
models.ShareInstanceExportLocations.path == models.ShareInstanceExportLocations.path ==
@ -2259,7 +2265,8 @@ def _process_share_filters(query, filters, project_id=None, is_public=False):
query = query.join( query = query.join(
models.ShareTypeExtraSpecs, models.ShareTypeExtraSpecs,
models.ShareTypeExtraSpecs.share_type_id == models.ShareTypeExtraSpecs.share_type_id ==
models.ShareInstance.share_type_id) models.ShareInstance.share_type_id,
)
for k, v in filters['extra_specs'].items(): for k, v in filters['extra_specs'].items():
query = query.filter(and_(models.ShareTypeExtraSpecs.key == k, query = query.filter(and_(models.ShareTypeExtraSpecs.key == k,
models.ShareTypeExtraSpecs.value == v)) models.ShareTypeExtraSpecs.value == v))
@ -2320,7 +2327,9 @@ def _share_data_get_for_project(
read_deleted="no", read_deleted="no",
).filter_by(project_id=project_id) ).filter_by(project_id=project_id)
if share_type_id: if share_type_id:
query = query.join("instances").filter_by(share_type_id=share_type_id) query = query.join(
models.Share.instances,
).filter_by(share_type_id=share_type_id)
elif user_id: elif user_id:
query = query.filter_by(user_id=user_id) query = query.filter_by(user_id=user_id)
result = query.first() result = query.first()
@ -2362,7 +2371,7 @@ def _share_get(context, share_id, **kwargs):
result = model_query( result = model_query(
context, models.Share, **kwargs, context, models.Share, **kwargs,
).options( ).options(
joinedload('share_metadata') orm.joinedload(models.Share.share_metadata),
).filter_by(id=share_id).first() ).filter_by(id=share_id).first()
if result is None: if result is None:
@ -2399,7 +2408,7 @@ def _share_get_all_with_filters(context, project_id=None, share_server_id=None,
query = model_query( query = model_query(
context, models.Share, context, models.Share,
).options( ).options(
joinedload('share_metadata') orm.joinedload(models.Share.share_metadata),
).join( ).join(
models.ShareInstance, models.ShareInstance,
models.ShareInstance.share_id == models.Share.id models.ShareInstance.share_id == models.Share.id
@ -2453,7 +2462,7 @@ def share_get_all_expired(context):
query = model_query( query = model_query(
context, models.Share, context, models.Share,
).options( ).options(
joinedload('share_metadata') orm.joinedload(models.Share.share_metadata),
).join( ).join(
models.ShareInstance, models.ShareInstance,
models.ShareInstance.share_id == models.Share.id, models.ShareInstance.share_id == models.Share.id,
@ -2853,7 +2862,7 @@ def _share_access_get_query(context, values, read_deleted='no'):
context, models.ShareAccessMapping, context, models.ShareAccessMapping,
read_deleted=read_deleted read_deleted=read_deleted
).options( ).options(
joinedload('share_access_rules_metadata') orm.joinedload(models.ShareAccessMapping.share_access_rules_metadata),
) )
return query.filter_by(**values) return query.filter_by(**values)
@ -2885,7 +2894,9 @@ def _share_access_metadata_get_item(context, access_id, key):
def _share_access_metadata_get_query(context, access_id): def _share_access_metadata_get_query(context, access_id):
return model_query( return model_query(
context, models.ShareAccessRulesMetadata, read_deleted="no", context, models.ShareAccessRulesMetadata, read_deleted="no",
).filter_by(access_id=access_id).options(joinedload('access')) ).filter_by(
access_id=access_id,
).options(orm.joinedload(models.ShareAccessRulesMetadata.access))
@require_context @require_context
@ -3020,7 +3031,7 @@ def share_access_get_with_context(context, access_id):
"""Get access record.""" """Get access record."""
access = _share_access_get_query( access = _share_access_get_query(
context, {'id': access_id} context, {'id': access_id}
).options(joinedload('share')).first() ).options(orm.joinedload(models.ShareAccessMapping.share)).first()
if access: if access:
access['project_id'] = access['share']['project_id'] access['project_id'] = access['share']['project_id']
return access return access
@ -3398,7 +3409,9 @@ def _share_snapshot_instance_get_with_filters(context, instance_ids=None,
if statuses is not None: if statuses is not None:
query = query.filter(models.ShareSnapshotInstance.status.in_(statuses)) query = query.filter(models.ShareSnapshotInstance.status.in_(statuses))
query = query.options(joinedload('share_group_snapshot')) query = query.options(
orm.joinedload(models.ShareSnapshotInstance.share_group_snapshot),
)
return query return query
@ -3486,9 +3499,9 @@ def _share_snapshot_get(context, snapshot_id, project_only=True):
).filter_by( ).filter_by(
id=snapshot_id, id=snapshot_id,
).options( ).options(
joinedload('share'), orm.joinedload(models.ShareSnapshot.share),
joinedload('instances'), orm.joinedload(models.ShareSnapshot.instances),
joinedload('share_snapshot_metadata'), orm.joinedload(models.ShareSnapshot.share_snapshot_metadata),
).first() ).first()
if not result: if not result:
@ -3523,12 +3536,15 @@ def _share_snapshot_get_all_with_filters(context, project_id=None,
if project_id: if project_id:
query = query.filter_by(project_id=project_id) query = query.filter_by(project_id=project_id)
if share_id: if share_id:
query = query.filter_by(share_id=share_id) query = query.filter_by(share_id=share_id)
query = (query.options(joinedload('share'))
.options(joinedload('instances')) query = query.options(
.options(joinedload('share_snapshot_metadata')) orm.joinedload(models.ShareSnapshot.share),
) orm.joinedload(models.ShareSnapshot.instances),
orm.joinedload(models.ShareSnapshot.share_snapshot_metadata),
)
# Snapshots with no instances are filtered out. # Snapshots with no instances are filtered out.
query = query.filter( query = query.filter(
@ -3552,10 +3568,12 @@ def _share_snapshot_get_all_with_filters(context, project_id=None,
'ek': usage_filter_keys} 'ek': usage_filter_keys}
raise exception.InvalidInput(reason=msg) raise exception.InvalidInput(reason=msg)
filters.pop('usage') filters.pop('usage')
if 'status' in filters: if 'status' in filters:
query = query.filter(models.ShareSnapshotInstance.status == ( query = query.filter(models.ShareSnapshotInstance.status == (
filters['status'])) filters['status']))
filters.pop('status') filters.pop('status')
if 'metadata' in filters: if 'metadata' in filters:
for k, v in filters['metadata'].items(): for k, v in filters['metadata'].items():
# pylint: disable=no-member # pylint: disable=no-member
@ -3766,10 +3784,11 @@ def share_snapshot_metadata_get_item(context, share_snapshot_id, key):
def _share_snapshot_metadata_get_query(context, share_snapshot_id): def _share_snapshot_metadata_get_query(context, share_snapshot_id):
return (model_query(context, models.ShareSnapshotMetadata, return model_query(
read_deleted="no"). context, models.ShareSnapshotMetadata, read_deleted="no",
filter_by(share_snapshot_id=share_snapshot_id). ).filter_by(
options(joinedload('share_snapshot'))) share_snapshot_id=share_snapshot_id,
).options(orm.joinedload(models.ShareSnapshotMetadata.share_snapshot))
def _share_snapshot_metadata_get(context, share_snapshot_id): def _share_snapshot_metadata_get(context, share_snapshot_id):
@ -4214,7 +4233,9 @@ def share_snapshot_instance_export_locations_update(
def _share_metadata_get_query(context, share_id): def _share_metadata_get_query(context, share_id):
return model_query( return model_query(
context, models.ShareMetadata, read_deleted="no", context, models.ShareMetadata, read_deleted="no",
).filter_by(share_id=share_id).options(joinedload('share')) ).filter_by(
share_id=share_id,
).options(orm.joinedload(models.ShareMetadata.share))
@require_context @require_context
@ -4339,7 +4360,7 @@ def _export_location_get_all(
).order_by( ).order_by(
"updated_at", "updated_at",
).options( ).options(
joinedload("_el_metadata_bare"), orm.joinedload(models.ShareInstanceExportLocations._el_metadata_bare),
) )
if not include_admin_only: if not include_admin_only:
@ -4347,7 +4368,9 @@ def _export_location_get_all(
if ignore_secondary_replicas: if ignore_secondary_replicas:
replica_state_attr = models.ShareInstance.replica_state replica_state_attr = models.ShareInstance.replica_state
query = query.join("share_instance").filter( query = query.join(
models.ShareInstanceExportLocations.share_instance,
).filter(
or_(replica_state_attr == None, # noqa or_(replica_state_attr == None, # noqa
replica_state_attr == constants.REPLICA_STATE_ACTIVE)) replica_state_attr == constants.REPLICA_STATE_ACTIVE))
@ -4421,12 +4444,14 @@ def _export_location_get_by_uuid(
).filter_by( ).filter_by(
uuid=export_location_uuid, uuid=export_location_uuid,
).options( ).options(
joinedload("_el_metadata_bare"), orm.joinedload(models.ShareInstanceExportLocations._el_metadata_bare),
) )
if ignore_secondary_replicas: if ignore_secondary_replicas:
replica_state_attr = models.ShareInstance.replica_state replica_state_attr = models.ShareInstance.replica_state
query = query.join("share_instance").filter( query = query.join(
models.ShareInstanceExportLocations.share_instance,
).filter(
or_( or_(
replica_state_attr == None, # noqa replica_state_attr == None, # noqa
replica_state_attr == constants.REPLICA_STATE_ACTIVE, replica_state_attr == constants.REPLICA_STATE_ACTIVE,
@ -4744,9 +4769,9 @@ def _share_network_get_query(context):
return model_query( return model_query(
context, models.ShareNetwork, project_only=True, context, models.ShareNetwork, project_only=True,
).options( ).options(
joinedload('share_instances'), orm.joinedload(models.ShareNetwork.share_instances),
joinedload('security_services'), orm.joinedload(models.ShareNetwork.security_services),
subqueryload('share_network_subnets'), orm.subqueryload(models.ShareNetwork.share_network_subnets),
) )
@ -4960,8 +4985,9 @@ def _count_share_networks(
read_deleted="no", read_deleted="no",
).filter_by(project_id=project_id) ).filter_by(project_id=project_id)
if share_type_id: if share_type_id:
query = query.join("share_instances").filter_by( query = query.join(
share_type_id=share_type_id) models.ShareNetwork.share_instances,
).filter_by(share_type_id=share_type_id)
elif user_id is not None: elif user_id is not None:
query = query.filter_by(user_id=user_id) query = query.filter_by(user_id=user_id)
return query.first()[0] return query.first()[0]
@ -4975,9 +5001,11 @@ def _share_network_subnet_get_query(context):
return model_query( return model_query(
context, models.ShareNetworkSubnet, context, models.ShareNetworkSubnet,
).options( ).options(
joinedload('share_servers'), orm.joinedload(models.ShareNetworkSubnet.share_servers),
joinedload('share_network'), orm.joinedload(models.ShareNetworkSubnet.share_network),
joinedload('share_network_subnet_metadata'), orm.joinedload(
models.ShareNetworkSubnet.share_network_subnet_metadata
),
) )
@ -5154,7 +5182,9 @@ def _share_network_subnet_metadata_get_query(context, share_network_subnet_id):
read_deleted="no", read_deleted="no",
).filter_by( ).filter_by(
share_network_subnet_id=share_network_subnet_id, share_network_subnet_id=share_network_subnet_id,
).options(joinedload('share_network_subnet')) ).options(
orm.joinedload(models.ShareNetworkSubnetMetadata.share_network_subnet),
)
@require_context @require_context
@ -5277,9 +5307,9 @@ def _share_server_get_query(context):
return model_query( return model_query(
context, models.ShareServer, context, models.ShareServer,
).options( ).options(
joinedload('share_instances'), orm.joinedload(models.ShareServer.share_instances),
joinedload('network_allocations'), orm.joinedload(models.ShareServer.network_allocations),
joinedload('share_network_subnets'), orm.joinedload(models.ShareServer.share_network_subnets),
) )
@ -5797,10 +5827,10 @@ def _share_type_get_query(context, read_deleted=None, expected_fields=None):
context, context,
models.ShareTypes, models.ShareTypes,
read_deleted=read_deleted, read_deleted=read_deleted,
).options(joinedload('extra_specs')) ).options(orm.joinedload(models.ShareTypes.extra_specs))
if 'projects' in expected_fields: if 'projects' in expected_fields:
query = query.options(joinedload('projects')) query = query.options(orm.joinedload(models.ShareTypes.projects))
if not context.is_admin: if not context.is_admin:
the_filter = [models.ShareTypes.is_public == true()] the_filter = [models.ShareTypes.is_public == true()]
@ -6064,7 +6094,7 @@ def _share_type_extra_specs_query(context, share_type_id):
context, models.ShareTypeExtraSpecs, read_deleted="no", context, models.ShareTypeExtraSpecs, read_deleted="no",
).filter_by( ).filter_by(
share_type_id=share_type_id, share_type_id=share_type_id,
).options(joinedload('share_type')) ).options(orm.joinedload(models.ShareTypeExtraSpecs.share_type))
@require_context @require_context
@ -6090,7 +6120,11 @@ def share_type_extra_specs_delete(context, share_type_id, key):
def _share_type_extra_specs_get_item(context, share_type_id, key): def _share_type_extra_specs_get_item(context, share_type_id, key):
result = _share_type_extra_specs_query( result = _share_type_extra_specs_query(
context, share_type_id, context, share_type_id,
).filter_by(key=key).options(joinedload('share_type')).first() ).filter_by(
key=key,
).options(
orm.joinedload(models.ShareTypeExtraSpecs.share_type),
).first()
if not result: if not result:
raise exception.ShareTypeExtraSpecsNotFound( raise exception.ShareTypeExtraSpecsNotFound(
@ -6265,12 +6299,11 @@ def purge_deleted_records(context, age_in_days):
def _share_group_get(context, share_group_id): def _share_group_get(context, share_group_id):
result = (model_query(context, models.ShareGroup, result = model_query(
project_only=True, context, models.ShareGroup, project_only=True, read_deleted='no',
read_deleted='no'). ).filter_by(
filter_by(id=share_group_id). id=share_group_id,
options(joinedload('share_types')). ).options(orm.joinedload(models.ShareGroup.share_types)).first()
first())
if not result: if not result:
raise exception.ShareGroupNotFound(share_group_id=share_group_id) raise exception.ShareGroupNotFound(share_group_id=share_group_id)
@ -6327,14 +6360,16 @@ def _share_group_get_all(context, project_id=None, share_server_id=None,
raise exception.InvalidInput(reason=msg) raise exception.InvalidInput(reason=msg)
if detailed: if detailed:
return query.options(joinedload('share_types')).all() return query.options(
else: orm.joinedload(models.ShareGroup.share_types),
query = query.with_entities( ).all()
models.ShareGroup.id, models.ShareGroup.name)
values = [] query = query.with_entities(
for sg_id, sg_name in query.all(): models.ShareGroup.id, models.ShareGroup.name)
values.append({"id": sg_id, "name": sg_name}) values = []
return values for sg_id, sg_name in query.all():
values.append({"id": sg_id, "name": sg_name})
return values
@require_admin_context @require_admin_context
@ -6440,8 +6475,10 @@ def _count_share_groups(context, project_id, user_id=None, share_type_id=None):
read_deleted="no", read_deleted="no",
).filter_by(project_id=project_id) ).filter_by(project_id=project_id)
if share_type_id: if share_type_id:
query = query.join("share_group_share_type_mappings").filter_by( query = query.join(
share_type_id=share_type_id) # models.ShareGroupShareTypeMapping,
models.ShareGroup.share_types,
).filter_by(share_type_id=share_type_id)
elif user_id is not None: elif user_id is not None:
query = query.filter_by(user_id=user_id) query = query.filter_by(user_id=user_id)
return query.first()[0] return query.first()[0]
@ -6458,9 +6495,10 @@ def _count_share_group_snapshots(
).filter_by(project_id=project_id) ).filter_by(project_id=project_id)
if share_type_id: if share_type_id:
query = query.join( query = query.join(
"share_group" models.ShareGroupSnapshot.share_group,
).join( ).join(
"share_group_share_type_mappings" # models.ShareGroupShareTypeMapping,
models.ShareGroup.share_types,
).filter_by(share_type_id=share_type_id) ).filter_by(share_type_id=share_type_id)
elif user_id is not None: elif user_id is not None:
query = query.filter_by(user_id=user_id) query = query.filter_by(user_id=user_id)
@ -6542,8 +6580,8 @@ def _share_group_snapshot_get(context, share_group_snapshot_id):
project_only=True, project_only=True,
read_deleted='no', read_deleted='no',
).options( ).options(
joinedload('share_group'), orm.joinedload(models.ShareGroupSnapshot.share_group),
joinedload('share_group_snapshot_members'), orm.joinedload(models.ShareGroupSnapshot.share_group_snapshot_members),
).filter_by( ).filter_by(
id=share_group_snapshot_id, id=share_group_snapshot_id,
).first() ).first()
@ -6594,16 +6632,18 @@ def _share_group_snapshot_get_all(
if detailed: if detailed:
return query.options( return query.options(
joinedload('share_group'), orm.joinedload(models.ShareGroupSnapshot.share_group),
joinedload('share_group_snapshot_members') orm.joinedload(
models.ShareGroupSnapshot.share_group_snapshot_members
),
).all() ).all()
else:
query = query.with_entities(models.ShareGroupSnapshot.id, query = query.with_entities(models.ShareGroupSnapshot.id,
models.ShareGroupSnapshot.name) models.ShareGroupSnapshot.name)
values = [] values = []
for sgs_id, sgs_name in query.all(): for sgs_id, sgs_name in query.all():
values.append({"id": sgs_id, "name": sgs_name}) values.append({"id": sgs_id, "name": sgs_name})
return values return values
@require_context @require_context
@ -6792,12 +6832,12 @@ def _share_group_type_get_query(
models.ShareGroupTypes, models.ShareGroupTypes,
read_deleted=read_deleted read_deleted=read_deleted
).options( ).options(
joinedload('group_specs'), orm.joinedload(models.ShareGroupTypes.group_specs),
joinedload('share_types'), orm.joinedload(models.ShareGroupTypes.share_types),
) )
if 'projects' in expected_fields: if 'projects' in expected_fields:
query = query.options(joinedload('projects')) query = query.options(orm.joinedload(models.ShareGroupTypes.projects))
if not context.is_admin: if not context.is_admin:
the_filter = [models.ShareGroupTypes.is_public == true()] the_filter = [models.ShareGroupTypes.is_public == true()]
@ -6904,8 +6944,8 @@ def _share_group_type_get_by_name(context, name):
context, context,
models.ShareGroupTypes, models.ShareGroupTypes,
).options( ).options(
joinedload('group_specs'), orm.joinedload(models.ShareGroupTypes.group_specs),
joinedload('share_types'), orm.joinedload(models.ShareGroupTypes.share_types),
).filter_by( ).filter_by(
name=name, name=name,
).first() ).first()
@ -7033,7 +7073,7 @@ def _share_group_type_specs_query(context, type_id):
).filter_by( ).filter_by(
share_group_type_id=type_id, share_group_type_id=type_id,
).options( ).options(
joinedload('share_group_type'), orm.joinedload(models.ShareGroupTypeSpecs.share_group_type),
) )
@ -7067,7 +7107,7 @@ def _share_group_type_specs_get_item(context, type_id, key):
).filter_by( ).filter_by(
key=key, key=key,
).options( ).options(
joinedload('share_group_type'), orm.joinedload(models.ShareGroupTypeSpecs.share_group_type),
).first() ).first()
if not result: if not result:

17
manila/test.py
View File

@ -176,23 +176,6 @@ class WarningsFixture(fixtures.Fixture):
category=sqla_exc.SADeprecationWarning, category=sqla_exc.SADeprecationWarning,
) )
# ..but filter everything out until we get around to fixing them
# TODO(stephenfin): Fix all of these
warnings.filterwarnings(
'ignore',
module='manila',
message='Using strings to indicate column or relationship paths ',
category=sqla_exc.SADeprecationWarning,
)
warnings.filterwarnings(
'ignore',
module='manila',
message='Using strings to indicate relationship names in Query',
category=sqla_exc.SADeprecationWarning,
)
# Enable general SQLAlchemy warnings also to ensure we're not doing # Enable general SQLAlchemy warnings also to ensure we're not doing
# silly stuff. It's possible that we'll need to filter things out here # silly stuff. It's possible that we'll need to filter things out here
# with future SQLAlchemy versions, but that's a good thing # with future SQLAlchemy versions, but that's a good thing