From 2f67ecd263ee105e7425b818d093dd3471a83975 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic Date: Fri, 14 Nov 2014 18:59:38 +0100 Subject: [PATCH] API call optimization and test cleanup * 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 --- tuskar_ui/infrastructure/nodes/tabs.py | 26 +++--------------- tuskar_ui/infrastructure/nodes/tests.py | 35 ++++++++++++------------- tuskar_ui/infrastructure/nodes/views.py | 4 +++ tuskar_ui/test/test_data/node_data.py | 6 ----- 4 files changed, 25 insertions(+), 46 deletions(-) diff --git a/tuskar_ui/infrastructure/nodes/tabs.py b/tuskar_ui/infrastructure/nodes/tabs.py index 641e573af..e549b4c78 100644 --- a/tuskar_ui/infrastructure/nodes/tabs.py +++ b/tuskar_ui/infrastructure/nodes/tabs.py @@ -39,7 +39,7 @@ class OverviewTab(tabs.Tab): template_name = "infrastructure/nodes/_overview.html" 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) memory_mb = sum(int(node.memory_mb) for node in nodes if node.memory_mb) @@ -198,10 +198,6 @@ class BaseTab(tabs.TableTab): def get_base_nodes_table_data(self): nodes, prev, more = self._nodes_info - - if 'errors' in self.request.GET: - return api.node.filter_nodes(nodes, healthy=False) - return nodes def has_prev_data(self, table): @@ -221,16 +217,10 @@ class AllTab(BaseTab): @cached_property def _nodes(self): - redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index') - - return api.node.Node.list(self.request, _error_redirect=redirect) + return self.tab_group.kwargs['nodes'] def get_all_nodes_table_data(self): nodes, prev, more = self._nodes_info - - if 'errors' in self.request.GET: - return api.node.filter_nodes(nodes, healthy=False) - return nodes @@ -251,9 +241,6 @@ class ProvisionedTab(BaseTab): def get_provisioned_nodes_table_data(self): nodes, prev, more = self._nodes_info - if 'errors' in self.request.GET: - return api.node.filter_nodes(nodes, healthy=False) - if nodes: all_resources = api.heat.Resource.list_all_resources(self.request) for node in nodes: @@ -285,10 +272,6 @@ class FreeTab(BaseTab): def get_free_nodes_table_data(self): nodes, prev, more = self._nodes_info - - if 'errors' in self.request.GET: - return api.node.filter_nodes(nodes, healthy=False) - return nodes @@ -302,9 +285,8 @@ class MaintenanceTab(BaseTab): @cached_property def _nodes(self): - redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index') - return api.node.Node.list(self.request, maintenance=True, - _error_redirect=redirect) + nodes = self.tab_group.kwargs['nodes'] + return list(utils.filter_items(nodes, maintenance=True)) def get_maintenance_nodes_table_data(self): return self._nodes diff --git a/tuskar_ui/infrastructure/nodes/tests.py b/tuskar_ui/infrastructure/nodes/tests.py index 0296cf1a4..8d2f757bb 100644 --- a/tuskar_ui/infrastructure/nodes/tests.py +++ b/tuskar_ui/infrastructure/nodes/tests.py @@ -50,32 +50,27 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase): def test_index_get(self): with patch('tuskar_ui.api.node.Node', **{ - 'spec_set': ['list'], # Only allow these attributes + 'spec_set': ['list'], 'list.return_value': [], }) as mock: res = self.client.get(INDEX_URL) - self.assertEqual(mock.list.call_count, 6) + self.assertEqual(mock.list.call_count, 3) self.assertTemplateUsed( res, 'infrastructure/nodes/index.html') self.assertTemplateUsed(res, 'infrastructure/nodes/_overview.html') - def _test_index_tab(self, tab_name): - nodes = [api.node.Node(api.node.IronicNode(node)) - 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 _all_mocked_nodes(self): + return [api.node.Node(api.node.IronicNode(node)) + for node in self.ironicclient_nodes.list()] + def _test_index_tab(self, tab_name, nodes): with patch('tuskar_ui.api.node.Node', **{ 'spec_set': ['list'], 'list.return_value': nodes, }) as Node: 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( res, 'infrastructure/nodes/index.html') @@ -85,16 +80,20 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase): nodes) 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): - self._test_index_tab('provisioned') + nodes = self._all_mocked_nodes() + self._test_index_tab('provisioned', nodes) 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): - 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): with patch('tuskar_ui.api.node.Node', **{ @@ -246,11 +245,11 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase): with contextlib.nested( patch('tuskar_ui.api.node.Node', **{ - 'spec_set': ['get'], # Only allow these attributes + 'spec_set': ['get'], 'get.return_value': node, }), 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': ( self._raise_horizon_exception_not_found), }), diff --git a/tuskar_ui/infrastructure/nodes/views.py b/tuskar_ui/infrastructure/nodes/views.py index 53e1195aa..99582d42e 100644 --- a/tuskar_ui/infrastructure/nodes/views.py +++ b/tuskar_ui/infrastructure/nodes/views.py @@ -60,6 +60,10 @@ class IndexView(infrastructure_views.ItemCountMixin, def get_data(self): 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): form_class = forms.RegisterNodeFormset diff --git a/tuskar_ui/test/test_data/node_data.py b/tuskar_ui/test/test_data/node_data.py index e23d1ae92..e3f6fb511 100644 --- a/tuskar_ui/test/test_data/node_data.py +++ b/tuskar_ui/test/test_data/node_data.py @@ -134,7 +134,6 @@ def data(TEST): }, 'power_state': 'on', 'target_power_state': 'on', - 'provision_state': 'active', 'maintenance': None, 'newly_discovered': None, 'provision_state': 'active', @@ -160,7 +159,6 @@ def data(TEST): }, 'power_state': 'on', 'target_power_state': 'on', - 'provision_state': 'active', 'maintenance': None, 'newly_discovered': None, 'provision_state': 'active', @@ -186,7 +184,6 @@ def data(TEST): }, 'power_state': 'rebooting', 'target_power_state': 'on', - 'provision_state': 'active', 'maintenance': None, 'newly_discovered': None, 'provision_state': 'deploying', @@ -212,7 +209,6 @@ def data(TEST): }, 'power_state': 'on', 'target_power_state': 'on', - 'provision_state': 'active', 'maintenance': None, 'newly_discovered': None, 'provision_state': 'deploying', @@ -241,7 +237,6 @@ def data(TEST): 'provision_state': 'error', 'maintenance': None, 'newly_discovered': None, - 'provision_state': 'deploying', 'extra': {} }) node_6 = node.Node( @@ -264,7 +259,6 @@ def data(TEST): }, 'power_state': 'on', 'target_power_state': 'on', - 'provision_state': 'active', 'maintenance': None, 'newly_discovered': None, 'provision_state': 'active',