openstack/python-heatclient
Code Issues Proposed changes

Do not paginate stack list unless page_size is set

Browse Source 
Pagination on the server has not yet been implemented, however
the client assumes it is, causing infinite recursion when paginate
is called.

With this change, no pagination is attempted unless page_size is
specified. However the 'limit' parameter is still honored, so it
is still possible to cap the total number of results returned.

heatclient users should not set page_size until pagination has been
implemented on the server. The stack list in Horizon currently
does not attempt to paginate, so is not affected.

Fixes: bug #1207839
Change-Id: I4087d3a8af48206d6ebe3edc441469464e4a401a
This commit is contained in:
Steve Baker
2013-08-05 15:29:16 +12:00
parent ea0e957978
commit 79dea076db
4 changed files with 234 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
heatclient/tests/fakes.py
View File

@@ -45,8 +45,7 @@ def script_heat_list():
{'content-type': 'application/json'},
json.dumps(resp_dict))
v1client.Client.json_request('GET',
'/stacks?limit=20').AndReturn((resp,
resp_dict))
'/stacks?').AndReturn((resp, resp_dict))
def script_heat_normal_error():

2
heatclient/tests/test_shell.py
View File

@@ -166,7 +166,7 @@ class ShellValidationTest(TestCase):
self.m.StubOutWithMock(v1client.Client, 'json_request')
fakes.script_keystone_client()
v1client.Client.json_request(
'GET', '/stacks?limit=20').AndRaise(exc.Unauthorized)
'GET', '/stacks?').AndRaise(exc.Unauthorized)
self.m.ReplayAll()
fake_env = {

231
heatclient/tests/test_stacks.py
View File

@@ -14,16 +14,32 @@
# under the License.
from heatclient.v1.stacks import Stack
from heatclient.v1.stacks import StackManager
from mock import MagicMock
import testscenarios
from testscenarios.scenarios import multiply_scenarios
import testtools
load_tests = testscenarios.load_tests_apply_scenarios
def mock_stack(manager, stack_name, stack_id):
return Stack(manager, {
"id": stack_id,
"stack_name": stack_name,
"links": [{
"href": "http://192.0.2.1:8004/v1/1234/stacks/%s/%s" % (
stack_name, stack_id),
"rel": "self"}],
"description": "No description",
"stack_status_reason": "Stack create completed successfully",
"creation_time": "2013-08-04T20:57:55Z",
"updated_time": "2013-08-04T20:57:55Z",
"stack_status": "CREATE_COMPLETE"
})
class StackStatusActionTest(testtools.TestCase):
scenarios = multiply_scenarios([
@@ -41,7 +57,8 @@ class StackStatusActionTest(testtools.TestCase):
def test_status_action(self):
stack_status = '%s_%s' % (self.action, self.status)
stack = Stack(None, {'stack_status': stack_status})
stack = mock_stack(None, 'stack_1', 'abcd1234')
stack.stack_status = stack_status
self.assertEqual(self.action, stack.action)
self.assertEqual(self.status, stack.status)
@@ -50,12 +67,218 @@ class StackOperationsTest(testtools.TestCase):
def test_delete_stack(self):
manager = MagicMock()
stack = Stack(manager, {'id': 'abcd1234'})
stack = mock_stack(manager, None, 'abcd1234')
stack.delete()
manager.delete.assert_called_once_with('abcd1234')
def test_get_stack(self):
manager = MagicMock()
stack = Stack(manager, {'id': 'abcd1234', 'stack_name': 'the_stack'})
stack = mock_stack(manager, 'the_stack', 'abcd1234')
stack.get()
manager.get.assert_called_once_with('the_stack/abcd1234')
class StackManagerNoPaginationTest(testtools.TestCase):
scenarios = [
('total_0', dict(total=0)),
('total_1', dict(total=1)),
('total_9', dict(total=9)),
('total_1', dict(total=10)),
('total_11', dict(total=11)),
('total_19', dict(total=19)),
('total_20', dict(total=20)),
('total_21', dict(total=21)),
('total_49', dict(total=49)),
('total_50', dict(total=50)),
('total_51', dict(total=51)),
('total_95', dict(total=95)),
]
# absolute limit for results returned
limit = 50
def mock_manager(self):
manager = StackManager(None)
manager._list = MagicMock()
def mock_list(*args, **kwargs):
def results():
for i in range(0, self.total):
stack_name = 'stack_%s' % (i + 1)
stack_id = 'abcd1234-%s' % (i + 1)
yield mock_stack(manager, stack_name, stack_id)
return list(results())
manager._list.side_effect = mock_list
return manager
def test_stack_list_no_pagination(self):
manager = self.mock_manager()
results = list(manager.list(limit=self.limit))
manager._list.assert_called_once_with(
'/stacks?', 'stacks')
last_result = min(self.limit, self.total)
# paginate is not specified, so the total
# results (up to the limit) is always returned
self.assertEqual(last_result, len(results))
if last_result > 0:
self.assertEqual('stack_1', results[0].stack_name)
self.assertEqual('stack_%s' % last_result, results[-1].stack_name)
def test_stack_list_no_pagination_no_limit(self):
manager = self.mock_manager()
results = list(manager.list())
manager._list.assert_called_once_with(
'/stacks?', 'stacks')
# paginate is not specified, so the total
# results is always returned
self.assertEqual(self.total, len(results))
if self.total > 0:
self.assertEqual('stack_1', results[0].stack_name)
self.assertEqual('stack_%s' % self.total, results[-1].stack_name)
class StackManagerPaginationTest(testtools.TestCase):
scenarios = [
('0_offset_0', dict(
page_size=10,
offset=0,
total=0,
results=((0, 0),)
)),
('1_offset_0', dict(
page_size=10,
offset=0,
total=1,
results=((0, 1),)
)),
('9_offset_0', dict(
page_size=10,
offset=0,
total=9,
results=((0, 9),)
)),
('10_offset_0', dict(
page_size=10,
offset=0,
total=10,
results=((0, 10), (10, 10))
)),
('11_offset_0', dict(
page_size=10,
offset=0,
total=11,
results=((0, 10), (10, 11))
)),
('11_offset_10', dict(
page_size=10,
offset=10,
total=11,
results=((10, 11),)
)),
('19_offset_10', dict(
page_size=10,
offset=10,
total=19,
results=((10, 19),)
)),
('20_offset_10', dict(
page_size=10,
offset=10,
total=20,
results=((10, 20), (20, 20))
)),
('21_offset_10', dict(
page_size=10,
offset=10,
total=21,
results=((10, 20), (20, 21))
)),
('21_offset_0', dict(
page_size=10,
offset=0,
total=21,
results=((0, 10), (10, 20), (20, 21))
)),
('21_offset_20', dict(
page_size=10,
offset=20,
total=21,
results=((20, 21),)
)),
('95_offset_90', dict(
page_size=10,
offset=90,
total=95,
results=((90, 95),)
)),
]
# absolute limit for results returned
limit = 50
def mock_manager(self):
manager = StackManager(None)
manager._list = MagicMock()
def mock_list(arg_url, arg_response_key):
try:
result = self.results[self.result_index]
except IndexError:
return
self.result_index = self.result_index + 1
offset = result[0]
url = '/stacks?'
if offset > 0:
url += 'marker=abcd1234-%s&' % offset
url += 'limit=%s' % self.page_size
self.assertEqual(url, arg_url)
def results():
for i in range(*result):
stack_name = 'stack_%s' % (i + 1)
stack_id = 'abcd1234-%s' % (i + 1)
yield mock_stack(manager, stack_name, stack_id)
return list(results())
manager._list.side_effect = mock_list
return manager
def test_stack_list_pagination(self):
manager = self.mock_manager()
list_params = {
'page_size': self.page_size,
'limit': self.limit
}
if self.offset > 0:
marker = 'abcd1234-%s' % self.offset
list_params['marker'] = marker
self.result_index = 0
results = list(manager.list(**list_params))
# assert that the list method has been called enough times
self.assertEqual(len(self.results), self.result_index)
last_result = min(self.limit, self.total - self.offset)
# one or more list calls have been recomposed into a single list
self.assertEqual(last_result, len(results))
if last_result > 0:
self.assertEqual('stack_%s' % (self.offset + 1),
results[0].stack_name)
self.assertEqual('stack_%s' % (self.offset + last_result),
results[-1].stack_name)

10
heatclient/v1/stacks.py
View File

@@ -17,8 +17,6 @@ import urllib
from heatclient.common import base
DEFAULT_PAGE_SIZE = 20
class Stack(base.Resource):
def __repr__(self):
@@ -86,10 +84,12 @@ class StackManager(base.Manager):
if (page_size and len(stacks) == page_size and
(absolute_limit is None or 0 < seen < absolute_limit)):
qp['marker'] = stack.id
for image in paginate(qp, seen):
yield image
for stack in paginate(qp, seen):
yield stack
params = {'limit': kwargs.get('page_size', DEFAULT_PAGE_SIZE)}
params = {}
if 'page_size' in kwargs:
params['limit'] = kwargs['page_size']
if 'marker' in kwargs:
params['marker'] = kwargs['marker']