Permalink
Browse files

Removed the extra code added to support filtering instances by instan…

…ce uuids.

Instead, added 'uuid' to the list of exact_filter_match names.
Updated the caller to add 'uuid: uuid_list' to the filters dictionary, instead of passing it in as another argument.
Updated the ID to UUID mapping code to return a dictionary, which allows the caller to be more efficient... It removes an extra loop there.
A couple of typo fixes.
  • Loading branch information...
1 parent 3da916d commit 83aca0e78727a870fb6aa788158920a1c8321541 @comstud comstud committed Sep 19, 2011
Showing with 31 additions and 48 deletions.
  1. +3 −2 nova/compute/api.py
  2. +6 −7 nova/db/api.py
  3. +12 −28 nova/db/sqlalchemy/api.py
  4. +3 −6 nova/network/manager.py
  5. +5 −3 nova/tests/fake_network.py
  6. +2 −2 nova/tests/test_compute.py
View
@@ -937,9 +937,10 @@ def _get_instances_by_filters(self, context, filters):
filters)
# NOTE(jkoelker) It is possible that we will get the same
# instance uuid twice (one for ipv4 and ipv6)
- ids = set([r['instance_uuid'] for r in res])
+ uuids = set([r['instance_uuid'] for r in res])
+ filters['uuid'] = uuids
- return self.db.instance_get_all_by_filters(context, filters, ids)
+ return self.db.instance_get_all_by_filters(context, filters)
def _cast_compute_message(self, method, context, instance_id, host=None,
params=None):
View
@@ -463,7 +463,7 @@ def virtual_interface_delete_by_instance(context, instance_id):
def virtual_interface_get_all(context):
- """Gets all virtual interfaces from the table,"""
+ """Gets all virtual interfaces from the table"""
return IMPL.virtual_interface_get_all(context)
@@ -505,10 +505,9 @@ def instance_get_all(context):
return IMPL.instance_get_all(context)
-def instance_get_all_by_filters(context, filters, instance_uuids=None):
+def instance_get_all_by_filters(context, filters):
"""Get all instances that match all filters."""
- return IMPL.instance_get_all_by_filters(context, filters,
- instance_uuids=instance_uuids)
+ return IMPL.instance_get_all_by_filters(context, filters)
def instance_get_active_by_window(context, begin, end=None, project_id=None):
@@ -612,9 +611,9 @@ def instance_get_actions(context, instance_id):
return IMPL.instance_get_actions(context, instance_id)
-def instance_get_uuids_by_ids(context, ids):
- """Return the UUIDs of the instances given the ids"""
- return IMPL.instance_get_uuids_by_ids(context, ids)
+def instance_get_id_to_uuid_mapping(context, ids):
+ """Return a dictionary containing 'ID: UUID' given the ids"""
+ return IMPL.instance_get_id_to_uuid_mapping(context, ids)
###################
View
@@ -1203,7 +1203,7 @@ def instance_get_all(context):
@require_context
-def instance_get_all_by_filters(context, filters, instance_uuids=None):
+def instance_get_all_by_filters(context, filters):
"""Return instances that match all filters. Deleted instances
will be returned by default, unless there's a filter that says
otherwise"""
@@ -1234,7 +1234,7 @@ def _exact_match_filter(query, column, value):
"""Do exact match against a column. value to match can be a list
so you can match any value in the list.
"""
- if isinstance(value, list):
+ if isinstance(value, list) or isinstance(value, set):
column_attr = getattr(models.Instance, column)
return query.filter(column_attr.in_(value))
else:
@@ -1268,7 +1268,7 @@ def _exact_match_filter(query, column, value):
# Filters for exact matches that we can do along with the SQL query...
# For other filters that don't match this, we will do regexp matching
exact_match_filter_names = ['project_id', 'user_id', 'image_ref',
- 'vm_state', 'instance_type_id', 'deleted']
+ 'vm_state', 'instance_type_id', 'deleted', 'uuid']
query_filters = [key for key in filters.iterkeys()
if key in exact_match_filter_names]
@@ -1279,30 +1279,10 @@ def _exact_match_filter(query, column, value):
query_prefix = _exact_match_filter(query_prefix, filter_name,
filters.pop(filter_name))
- db_instances = query_prefix.all()
- db_uuids = [instance['uuid'] for instance in db_instances \
- if instance['uuid']]
-
- if instance_uuids is None:
- instance_uuids = []
-
- uuids = []
-
- # NOTE(jkoelker): String UUIDs only!
- if not instance_uuids:
- uuids = db_uuids
- elif not db_instances:
- uuids = instance_uuids
- else:
- uuids = list(set(instance_uuids) & set(db_uuids))
-
- if not uuids:
+ instances = query_prefix.all()
+ if not instances:
return []
- instances = session.query(models.Instance).\
- filter(models.Instance.uuid.in_(uuids)).\
- all()
-
# Now filter on everything else for regexp matching..
# For filters not in the list, we'll attempt to use the filter_name
# as a column name in Instance..
@@ -1320,6 +1300,8 @@ def _exact_match_filter(query, column, value):
filter_l = lambda instance: _regexp_filter_by_column(instance,
filter_name, filter_re)
instances = filter(filter_l, instances)
+ if not instances:
+ break
return instances
@@ -1564,13 +1546,15 @@ def instance_get_actions(context, instance_id):
@require_context
-def instance_get_uuids_by_ids(context, ids):
+def instance_get_id_to_uuid_mapping(context, ids):
session = get_session()
instances = session.query(models.Instance).\
filter(models.Instance.id.in_(ids)).\
all()
- return [{'uuid': instance['uuid'],
- 'id': instance['id']} for instance in instances]
+ mapping = {}
+ for instance in instances:
+ mapping[instance['id']] = instance['uuid']
+ return mapping
###################
View
@@ -410,7 +410,7 @@ def get_instance_uuids_by_ip_filter(self, context, filters):
ip_filter = re.compile(str(filters.get('ip')))
ipv6_filter = re.compile(str(filters.get('ip6')))
- # NOTE(jkoelker) Should probably figur out a better way to do
+ # NOTE(jkoelker) Should probably figure out a better way to do
# this. But for now it "works", this could suck on
# large installs.
@@ -448,12 +448,9 @@ def get_instance_uuids_by_ip_filter(self, context, filters):
# NOTE(jkoelker) Until we switch over to instance_uuid ;)
ids = [res['instance_id'] for res in results]
- uuids = self.db.instance_get_uuids_by_ids(context, ids)
+ uuid_map = self.db.instance_get_id_to_uuid_mapping(context, ids)
for res in results:
- uuid = [u['uuid'] for u in uuids if u['id'] == res['instance_id']]
- # NOTE(jkoelker) UUID must exist, so no test here
- res['instance_uuid'] = uuid[0]
-
+ res['instance_uuid'] = uuid_map.get(res['instance_id'])
return results
def _get_networks_for_instance(self, context, instance_id, project_id,
@@ -105,10 +105,12 @@ def virtual_interface_get_all(self, context):
'floating_ips': [floats[2]]}]}]
return vifs
- def instance_get_uuids_by_ids(self, context, ids):
+ def instance_get_id_to_uuid_mapping(self, context, ids):
# NOTE(jkoelker): This is just here until we can rely on UUIDs
- return [{'uuid': str(utils.gen_uuid()),
- 'id': id} for id in ids]
+ mapping = {}
+ for id in ids:
+ mapping[id] = str(utils.gen_uuid())
+ return mapping
def __init__(self):
self.db = self.FakeDB()
@@ -1033,8 +1033,8 @@ def test_get_all_by_multiple_options_at_once(self):
'get_instance_uuids_by_ip_filter',
network_manager.get_instance_uuids_by_ip_filter)
self.stubs.Set(network_manager.db,
- 'instance_get_uuids_by_ids',
- db.instance_get_uuids_by_ids)
+ 'instance_get_id_to_uuid_mapping',
+ db.instance_get_id_to_uuid_mapping)
instance_id1 = self._create_instance({'display_name': 'woot',
'id': 0})

0 comments on commit 83aca0e

Please sign in to comment.