Get rid of oslo_db warning about "id" not being in "sort_keys"
* Now when we query a collection of objects from DB we don't use paginated query if query set is not limited * In case of paginated query we now always insert "id" into sort keys to avoid possible looping for multiple paginated querying * Minor cosmetic changes Change-Id: Ie4707b04a97c7fa17dcf278c74e2e852346ac33c
This commit is contained in:
@@ -144,16 +144,23 @@ def _paginate_query(model, limit=None, marker=None, sort_keys=None,
|
|||||||
if not query:
|
if not query:
|
||||||
query = _secure_query(model)
|
query = _secure_query(model)
|
||||||
|
|
||||||
|
sort_keys = sort_keys if sort_keys else []
|
||||||
|
sort_dirs = sort_dirs if sort_dirs else []
|
||||||
|
|
||||||
|
if 'id' not in sort_keys:
|
||||||
|
sort_keys.append('id')
|
||||||
|
sort_dirs.append('asc')
|
||||||
|
|
||||||
query = db_utils.paginate_query(
|
query = db_utils.paginate_query(
|
||||||
query,
|
query,
|
||||||
model,
|
model,
|
||||||
limit,
|
limit,
|
||||||
sort_keys if sort_keys else {},
|
sort_keys,
|
||||||
marker=marker,
|
marker=marker,
|
||||||
sort_dirs=sort_dirs
|
sort_dirs=sort_dirs
|
||||||
)
|
)
|
||||||
|
|
||||||
return query.all()
|
return query
|
||||||
|
|
||||||
|
|
||||||
def _delete_all(model, session=None, **kwargs):
|
def _delete_all(model, session=None, **kwargs):
|
||||||
@@ -170,24 +177,27 @@ def _get_collection(model, insecure=False, limit=None, marker=None,
|
|||||||
if fields else ()
|
if fields else ()
|
||||||
)
|
)
|
||||||
|
|
||||||
tags = kwargs.pop('tags', None)
|
|
||||||
|
|
||||||
query = (b.model_query(model, *columns) if insecure
|
query = (b.model_query(model, *columns) if insecure
|
||||||
else _secure_query(model, *columns))
|
else _secure_query(model, *columns))
|
||||||
query = query.filter_by(**kwargs)
|
query = query.filter_by(**kwargs)
|
||||||
|
|
||||||
|
tags = kwargs.pop('tags', None)
|
||||||
|
|
||||||
# To match the tag list, a resource must contain at least all of the
|
# To match the tag list, a resource must contain at least all of the
|
||||||
# tags present in the filter parameter.
|
# tags present in the filter parameter.
|
||||||
if tags:
|
if tags:
|
||||||
tag_attr = getattr(model, 'tags')
|
tag_attr = getattr(model, 'tags')
|
||||||
|
|
||||||
if len(tags) == 1:
|
if len(tags) == 1:
|
||||||
expr = tag_attr.contains(tags)
|
expr = tag_attr.contains(tags)
|
||||||
else:
|
else:
|
||||||
expr = sa.and_(*[tag_attr.contains(tag) for tag in tags])
|
expr = sa.and_(*[tag_attr.contains(tag) for tag in tags])
|
||||||
|
|
||||||
query = query.filter(expr)
|
query = query.filter(expr)
|
||||||
|
|
||||||
try:
|
if marker or limit:
|
||||||
return _paginate_query(
|
# Paginate query only if query set is limited.
|
||||||
|
query = _paginate_query(
|
||||||
model,
|
model,
|
||||||
limit,
|
limit,
|
||||||
marker,
|
marker,
|
||||||
@@ -195,10 +205,13 @@ def _get_collection(model, insecure=False, limit=None, marker=None,
|
|||||||
sort_dirs,
|
sort_dirs,
|
||||||
query
|
query
|
||||||
)
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
return query.all()
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise exc.DBQueryEntryError(
|
raise exc.DBQueryEntryError(
|
||||||
"Failed when querying database, error type: %s, "
|
"Failed when querying database, error type: %s, "
|
||||||
"error message: %s" % (e.__class__.__name__, e.message)
|
"error message: %s" % (e.__class__.__name__, str(e))
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -65,10 +65,12 @@ class ActionTestsV2(base.TestCase):
|
|||||||
delimiter='&'
|
delimiter='&'
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# NOTE: 'id' gets included into sort keys automatically with 'desc'
|
||||||
|
# sorting to avoid pagination looping.
|
||||||
expected_sub_dict = {
|
expected_sub_dict = {
|
||||||
'limit': 1,
|
'limit': 1,
|
||||||
'sort_keys': 'name',
|
'sort_keys': 'name,id',
|
||||||
'sort_dirs': 'desc'
|
'sort_dirs': 'desc,asc'
|
||||||
}
|
}
|
||||||
|
|
||||||
self.assertDictContainsSubset(expected_sub_dict, param_dict)
|
self.assertDictContainsSubset(expected_sub_dict, param_dict)
|
||||||
|
|||||||
@@ -88,10 +88,12 @@ class ExecutionTestsV2(base.TestCase):
|
|||||||
delimiter='&'
|
delimiter='&'
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# NOTE: 'id' gets included into sort keys automatically with 'desc'
|
||||||
|
# sorting to avoid pagination looping.
|
||||||
expected_dict = {
|
expected_dict = {
|
||||||
'limit': 1,
|
'limit': 1,
|
||||||
'sort_keys': 'workflow_name',
|
'sort_keys': 'workflow_name,id',
|
||||||
'sort_dirs': 'asc',
|
'sort_dirs': 'asc,asc',
|
||||||
}
|
}
|
||||||
|
|
||||||
self.assertTrue(
|
self.assertTrue(
|
||||||
|
|||||||
@@ -73,10 +73,12 @@ class WorkflowTestsV2(base.TestCase):
|
|||||||
delimiter='&'
|
delimiter='&'
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# NOTE: 'id' gets included into sort keys automatically with 'desc'
|
||||||
|
# sorting to avoid pagination looping.
|
||||||
expected_sub_dict = {
|
expected_sub_dict = {
|
||||||
'limit': 1,
|
'limit': 1,
|
||||||
'sort_keys': 'name',
|
'sort_keys': 'name,id',
|
||||||
'sort_dirs': 'desc'
|
'sort_dirs': 'desc,asc'
|
||||||
}
|
}
|
||||||
|
|
||||||
self.assertDictContainsSubset(expected_sub_dict, param_dict)
|
self.assertDictContainsSubset(expected_sub_dict, param_dict)
|
||||||
|
|||||||
Reference in New Issue
Block a user