From 5fe9408d2e53ac24a578d0ed568a4cede303fe58 Mon Sep 17 00:00:00 2001
From: Christian Berendt <berendt@b1-systems.de>
Date: Wed, 17 Jul 2013 16:48:04 +0200
Subject: [PATCH] make findall in novaclient/base.py more efficient
Use /resources instead of /resources/detail to resolve
the resource ID by the name and load the details of the
resource in a separate step. This reduces the overhead
to resolve the resource ID and results in a better runtime
performance.
This patch does not solve the issue that the name resolving
takes place on the client side. For solving this issue new
Nova API methods are necessary.
fixes bug #1202179
Change-Id: Ib753b1d090cb74b2d137c68f6899dad4ae2ec1ca
---
novaclient/base.py | 19 +++++++++++++++---
novaclient/tests/v1_1/fakes.py | 28 +++++++++++++++++++++++++++
novaclient/tests/v1_1/test_images.py | 3 ++-
novaclient/tests/v1_1/test_servers.py | 3 ++-
novaclient/tests/v1_1/test_shell.py | 24 ++++++++++++++++++-----
5 files changed, 67 insertions(+), 10 deletions(-)
diff --git a/novaclient/base.py b/novaclient/base.py
index c37fa3486..adcf29d5a 100644
--- a/novaclient/base.py
+++ b/novaclient/base.py
@@ -22,6 +22,7 @@ Base utilities to build API operation managers and objects on top of.
import abc
import contextlib
import hashlib
+import inspect
import os
import six
@@ -197,11 +198,23 @@ class ManagerWithFind(Manager):
found = []
searches = kwargs.items()
- for obj in self.list():
+ detailed = True
+ if 'detailed' in inspect.getargspec(self.list).args:
+ detailed = ("human_id" not in kwargs and
+ "name" not in kwargs and
+ "display_name" not in kwargs)
+ listing = self.list(detailed=detailed)
+ else:
+ listing = self.list()
+
+ for obj in listing:
try:
if all(getattr(obj, attr) == value
- for (attr, value) in searches):
- found.append(obj)
+ for (attr, value) in searches):
+ if detailed:
+ found.append(obj)
+ else:
+ found.append(self.get(obj.id))
except AttributeError:
continue
diff --git a/novaclient/tests/v1_1/fakes.py b/novaclient/tests/v1_1/fakes.py
index cc24c7d42..7673317fc 100644
--- a/novaclient/tests/v1_1/fakes.py
+++ b/novaclient/tests/v1_1/fakes.py
@@ -1720,6 +1720,34 @@ class FakeHTTPClient(base_client.HTTPClient):
"links": ''}],
"metadata": {}}]})
+ def get_volumes(self, **kw):
+ return (200, {}, {"volumes": [
+ {"display_name": "Work",
+ "display_description": "volume for work",
+ "status": "ATTACHED",
+ "id": "15e59938-07d5-11e1-90e3-e3dffe0c5983",
+ "created_at": "2011-09-09T00:00:00Z",
+ "attached": "2011-11-11T00:00:00Z",
+ "size": 1024,
+ "attachments": [
+ {"id": "3333",
+ "links": ''}],
+ "metadata": {}}]})
+
+ def get_volumes_15e59938_07d5_11e1_90e3_e3dffe0c5983(self, **kw):
+ return (200, {}, {"volume":
+ {"display_name": "Work",
+ "display_description": "volume for work",
+ "status": "ATTACHED",
+ "id": "15e59938-07d5-11e1-90e3-e3dffe0c5983",
+ "created_at": "2011-09-09T00:00:00Z",
+ "attached": "2011-11-11T00:00:00Z",
+ "size": 1024,
+ "attachments": [
+ {"id": "3333",
+ "links": ''}],
+ "metadata": {}}})
+
def post_volumes(self, **kw):
return (200, {}, {"volume":
{"status": "creating",
diff --git a/novaclient/tests/v1_1/test_images.py b/novaclient/tests/v1_1/test_images.py
index b545cedd5..5f9cfacd0 100644
--- a/novaclient/tests/v1_1/test_images.py
+++ b/novaclient/tests/v1_1/test_images.py
@@ -45,7 +45,8 @@ class ImagesTest(utils.TestCase):
def test_find(self):
i = cs.images.find(name="CentOS 5.2")
self.assertEqual(i.id, 1)
- cs.assert_called('GET', '/images/detail')
+ cs.assert_called('GET', '/images', pos=-2)
+ cs.assert_called('GET', '/images/1', pos=-1)
iml = cs.images.findall(status='SAVING')
self.assertEqual(len(iml), 1)
diff --git a/novaclient/tests/v1_1/test_servers.py b/novaclient/tests/v1_1/test_servers.py
index 6fec98bac..e04e7887b 100644
--- a/novaclient/tests/v1_1/test_servers.py
+++ b/novaclient/tests/v1_1/test_servers.py
@@ -171,7 +171,8 @@ class ServersTest(utils.TestCase):
def test_find(self):
server = cs.servers.find(name='sample-server')
- cs.assert_called('GET', '/servers/detail')
+ cs.assert_called('GET', '/servers', pos=-2)
+ cs.assert_called('GET', '/servers/1234', pos=-1)
self.assertEqual(server.name, 'sample-server')
self.assertRaises(exceptions.NoUniqueMatch, cs.servers.find,
diff --git a/novaclient/tests/v1_1/test_shell.py b/novaclient/tests/v1_1/test_shell.py
index 5cf99d6e2..1ca22c62f 100644
--- a/novaclient/tests/v1_1/test_shell.py
+++ b/novaclient/tests/v1_1/test_shell.py
@@ -536,16 +536,21 @@ class ShellTest(utils.TestCase):
def test_rebuild(self):
self.run_command('rebuild sample-server 1')
+ self.assert_called('GET', '/servers', pos=-8)
+ self.assert_called('GET', '/servers/1234', pos=-7)
+ self.assert_called('GET', '/images/1', pos=-6)
self.assert_called('POST', '/servers/1234/action',
- {'rebuild': {'imageRef': 1}}, pos=-4)
- self.assert_called('GET', '/servers/detail', pos=-3)
+ {'rebuild': {'imageRef': 1}}, pos=-5)
self.assert_called('GET', '/flavors/1', pos=-2)
self.assert_called('GET', '/images/2')
self.run_command('rebuild sample-server 1 --rebuild-password asdf')
+ self.assert_called('GET', '/servers', pos=-8)
+ self.assert_called('GET', '/servers/1234', pos=-7)
+ self.assert_called('GET', '/images/1', pos=-6)
self.assert_called('POST', '/servers/1234/action',
{'rebuild': {'imageRef': 1, 'adminPass': 'asdf'}},
- pos=-4)
+ pos=-5)
self.assert_called('GET', '/flavors/1', pos=-2)
self.assert_called('GET', '/images/2')
@@ -668,7 +673,11 @@ class ShellTest(utils.TestCase):
self.assert_called('DELETE', '/servers/1234', pos=-3)
self.assert_called('DELETE', '/servers/5678', pos=-1)
self.run_command('delete sample-server sample-server2')
- self.assert_called('DELETE', '/servers/1234', pos=-3)
+ self.assert_called('GET', '/servers', pos=-6)
+ self.assert_called('GET', '/servers/1234', pos=-5)
+ self.assert_called('DELETE', '/servers/1234', pos=-4)
+ self.assert_called('GET', '/servers', pos=-3)
+ self.assert_called('GET', '/servers/5678', pos=-2)
self.assert_called('DELETE', '/servers/5678', pos=-1)
def test_delete_two_with_one_nonexistent(self):
@@ -1518,7 +1527,12 @@ class ShellTest(utils.TestCase):
def test_volume_show(self):
self.run_command('volume-show Work')
- self.assert_called('GET', '/volumes/detail')
+ self.assert_called('GET', '/volumes', pos=-2)
+ self.assert_called(
+ 'GET',
+ '/volumes/15e59938-07d5-11e1-90e3-e3dffe0c5983',
+ pos=-1
+ )
def test_volume_create(self):
self.run_command('volume-create 2 --display-name Work')