x/tuskar-ui
Code Issues Proposed changes

API call optimization and test cleanup

Browse Source 
* Reduced number of API calls to Node.list
* Cleaned up node_data.py (it contained duplicate dict entries)
* Fixed test for maintenace tab to match test data

Change-Id: I7157b42802c2663266c70e4bb2c4cc97f740ad8b
This commit is contained in:
Ana Krivokapic 2014-11-14 18:59:38 +01:00
parent 75a5429dfd
commit 2f67ecd263
4 changed files with 25 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
tuskar_ui/infrastructure/nodes/tabs.py
View File

@ -39,7 +39,7 @@ class OverviewTab(tabs.Tab):
template_name = "infrastructure/nodes/_overview.html" template_name = "infrastructure/nodes/_overview.html"
def get_context_data(self, request): def get_context_data(self, request):
nodes = api.node.Node.list(request) nodes = self.tab_group.kwargs['nodes']
cpus = sum(int(node.cpus) for node in nodes if node.cpus) cpus = sum(int(node.cpus) for node in nodes if node.cpus)
memory_mb = sum(int(node.memory_mb) for node in nodes if memory_mb = sum(int(node.memory_mb) for node in nodes if
node.memory_mb) node.memory_mb)
@ -198,10 +198,6 @@ class BaseTab(tabs.TableTab):
def get_base_nodes_table_data(self): def get_base_nodes_table_data(self):
nodes, prev, more = self._nodes_info nodes, prev, more = self._nodes_info
if 'errors' in self.request.GET:
return api.node.filter_nodes(nodes, healthy=False)
return nodes return nodes
def has_prev_data(self, table): def has_prev_data(self, table):
@ -221,16 +217,10 @@ class AllTab(BaseTab):
@cached_property @cached_property
def _nodes(self): def _nodes(self):
redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index') return self.tab_group.kwargs['nodes']
return api.node.Node.list(self.request, _error_redirect=redirect)
def get_all_nodes_table_data(self): def get_all_nodes_table_data(self):
nodes, prev, more = self._nodes_info nodes, prev, more = self._nodes_info
if 'errors' in self.request.GET:
return api.node.filter_nodes(nodes, healthy=False)
return nodes return nodes
@ -251,9 +241,6 @@ class ProvisionedTab(BaseTab):
def get_provisioned_nodes_table_data(self): def get_provisioned_nodes_table_data(self):
nodes, prev, more = self._nodes_info nodes, prev, more = self._nodes_info
if 'errors' in self.request.GET:
return api.node.filter_nodes(nodes, healthy=False)
if nodes: if nodes:
all_resources = api.heat.Resource.list_all_resources(self.request) all_resources = api.heat.Resource.list_all_resources(self.request)
for node in nodes: for node in nodes:
@ -285,10 +272,6 @@ class FreeTab(BaseTab):
def get_free_nodes_table_data(self): def get_free_nodes_table_data(self):
nodes, prev, more = self._nodes_info nodes, prev, more = self._nodes_info
if 'errors' in self.request.GET:
return api.node.filter_nodes(nodes, healthy=False)
return nodes return nodes
@ -302,9 +285,8 @@ class MaintenanceTab(BaseTab):
@cached_property @cached_property
def _nodes(self): def _nodes(self):
redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index') nodes = self.tab_group.kwargs['nodes']
return api.node.Node.list(self.request, maintenance=True, return list(utils.filter_items(nodes, maintenance=True))
_error_redirect=redirect)
def get_maintenance_nodes_table_data(self): def get_maintenance_nodes_table_data(self):
return self._nodes return self._nodes

35
tuskar_ui/infrastructure/nodes/tests.py
View File

@ -50,32 +50,27 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
def test_index_get(self): def test_index_get(self):
with patch('tuskar_ui.api.node.Node', **{ with patch('tuskar_ui.api.node.Node', **{
'spec_set': ['list'], # Only allow these attributes 'spec_set': ['list'],
'list.return_value': [], 'list.return_value': [],
}) as mock: }) as mock:
res = self.client.get(INDEX_URL) res = self.client.get(INDEX_URL)
self.assertEqual(mock.list.call_count, 6) self.assertEqual(mock.list.call_count, 3)
self.assertTemplateUsed( self.assertTemplateUsed(
res, 'infrastructure/nodes/index.html') res, 'infrastructure/nodes/index.html')
self.assertTemplateUsed(res, 'infrastructure/nodes/_overview.html') self.assertTemplateUsed(res, 'infrastructure/nodes/_overview.html')
def _test_index_tab(self, tab_name): def _all_mocked_nodes(self):
nodes = [api.node.Node(api.node.IronicNode(node)) return [api.node.Node(api.node.IronicNode(node))
for node in self.ironicclient_nodes.list()] for node in self.ironicclient_nodes.list()]
# TODO(akrivoka): this should be placed in the test data, but currently
# that's not possible due to the drawbacks in the Node architecture.
# We should rework the entire api/node.py and fix this problem.
for node in nodes:
node.ip_address = '1.1.1.1'
def _test_index_tab(self, tab_name, nodes):
with patch('tuskar_ui.api.node.Node', **{ with patch('tuskar_ui.api.node.Node', **{
'spec_set': ['list'], 'spec_set': ['list'],
'list.return_value': nodes, 'list.return_value': nodes,
}) as Node: }) as Node:
res = self.client.get(INDEX_URL + '?tab=nodes__' + tab_name) res = self.client.get(INDEX_URL + '?tab=nodes__' + tab_name)
self.assertEqual(Node.list.call_count, 6) self.assertEqual(Node.list.call_count, 3)
self.assertTemplateUsed( self.assertTemplateUsed(
res, 'infrastructure/nodes/index.html') res, 'infrastructure/nodes/index.html')
@ -85,16 +80,20 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
nodes) nodes)
def test_all_nodes(self): def test_all_nodes(self):
self._test_index_tab('all') nodes = self._all_mocked_nodes()
self._test_index_tab('all', nodes)
def test_provisioned_nodes(self): def test_provisioned_nodes(self):
self._test_index_tab('provisioned') nodes = self._all_mocked_nodes()
self._test_index_tab('provisioned', nodes)
def test_free_nodes(self): def test_free_nodes(self):
self._test_index_tab('free') nodes = self._all_mocked_nodes()
self._test_index_tab('free', nodes)
def test_maintenance_nodes(self): def test_maintenance_nodes(self):
self._test_index_tab('maintenance') nodes = self._all_mocked_nodes()[6:]
self._test_index_tab('maintenance', nodes)
def _test_index_tab_list_exception(self, tab_name): def _test_index_tab_list_exception(self, tab_name):
with patch('tuskar_ui.api.node.Node', **{ with patch('tuskar_ui.api.node.Node', **{
@ -246,11 +245,11 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
with contextlib.nested( with contextlib.nested(
patch('tuskar_ui.api.node.Node', **{ patch('tuskar_ui.api.node.Node', **{
'spec_set': ['get'], # Only allow these attributes 'spec_set': ['get'],
'get.return_value': node, 'get.return_value': node,
}), }),
patch('tuskar_ui.api.heat.Resource', **{ patch('tuskar_ui.api.heat.Resource', **{
'spec_set': ['get_by_node'], # Only allow these attributes 'spec_set': ['get_by_node'],
'get_by_node.side_effect': ( 'get_by_node.side_effect': (
self._raise_horizon_exception_not_found), self._raise_horizon_exception_not_found),
}), }),

4
tuskar_ui/infrastructure/nodes/views.py
View File

@ -60,6 +60,10 @@ class IndexView(infrastructure_views.ItemCountMixin,
def get_data(self): def get_data(self):
return api.node.Node.list(self.request) return api.node.Node.list(self.request)
def get_tabs(self, request, **kwargs):
nodes = self.get_data()
return self.tab_group_class(request, nodes=nodes, **kwargs)
class RegisterView(horizon_forms.ModalFormView): class RegisterView(horizon_forms.ModalFormView):
form_class = forms.RegisterNodeFormset form_class = forms.RegisterNodeFormset

6
tuskar_ui/test/test_data/node_data.py
View File

@ -134,7 +134,6 @@ def data(TEST):
}, },
'power_state': 'on', 'power_state': 'on',
'target_power_state': 'on', 'target_power_state': 'on',
'provision_state': 'active',
'maintenance': None, 'maintenance': None,
'newly_discovered': None, 'newly_discovered': None,
'provision_state': 'active', 'provision_state': 'active',
@ -160,7 +159,6 @@ def data(TEST):
}, },
'power_state': 'on', 'power_state': 'on',
'target_power_state': 'on', 'target_power_state': 'on',
'provision_state': 'active',
'maintenance': None, 'maintenance': None,
'newly_discovered': None, 'newly_discovered': None,
'provision_state': 'active', 'provision_state': 'active',
@ -186,7 +184,6 @@ def data(TEST):
}, },
'power_state': 'rebooting', 'power_state': 'rebooting',
'target_power_state': 'on', 'target_power_state': 'on',
'provision_state': 'active',
'maintenance': None, 'maintenance': None,
'newly_discovered': None, 'newly_discovered': None,
'provision_state': 'deploying', 'provision_state': 'deploying',
@ -212,7 +209,6 @@ def data(TEST):
}, },
'power_state': 'on', 'power_state': 'on',
'target_power_state': 'on', 'target_power_state': 'on',
'provision_state': 'active',
'maintenance': None, 'maintenance': None,
'newly_discovered': None, 'newly_discovered': None,
'provision_state': 'deploying', 'provision_state': 'deploying',
@ -241,7 +237,6 @@ def data(TEST):
'provision_state': 'error', 'provision_state': 'error',
'maintenance': None, 'maintenance': None,
'newly_discovered': None, 'newly_discovered': None,
'provision_state': 'deploying',
'extra': {} 'extra': {}
}) })
node_6 = node.Node( node_6 = node.Node(
@ -264,7 +259,6 @@ def data(TEST):
}, },
'power_state': 'on', 'power_state': 'on',
'target_power_state': 'on', 'target_power_state': 'on',
'provision_state': 'active',
'maintenance': None, 'maintenance': None,
'newly_discovered': None, 'newly_discovered': None,
'provision_state': 'active', 'provision_state': 'active',