diff --git a/tuskar_ui/api.py b/tuskar_ui/api.py index ef91447f5..d712af6b4 100644 --- a/tuskar_ui/api.py +++ b/tuskar_ui/api.py @@ -15,18 +15,18 @@ import django.conf import heatclient import logging +from django.utils.translation import ugettext_lazy as _ from horizon.utils import memoized - from novaclient.v1_1.contrib import baremetal -from tuskarclient.v1 import client as tuskar_client - from openstack_dashboard.api import base from openstack_dashboard.api import glance from openstack_dashboard.api import heat from openstack_dashboard.api import nova from openstack_dashboard.test.test_data import utils +from tuskarclient.v1 import client as tuskar_client from tuskar_ui.cached_property import cached_property # noqa +from tuskar_ui.handle_errors import handle_errors # noqa from tuskar_ui.test.test_data import tuskar_data LOG = logging.getLogger(__name__) @@ -133,12 +133,14 @@ class NodeProfile(object): return cls(nova.flavor_get(request, node_profile_id)) @classmethod + @handle_errors(_("Unable to retrieve node profile list."), []) def list(cls, request): return [cls(item) for item in nova.flavor_list(request)] - @staticmethod + @classmethod @memoized.memoized - def list_deployed_ids(request): + @handle_errors(_("Unable to retrieve existing servers list."), []) + def list_deployed_ids(cls, request): """Get and memoize ID's of deployed node profiles.""" servers = nova.server_list(request)[0] return set(server.flavor['id'] for server in servers) @@ -199,6 +201,7 @@ class Overcloud(base.APIResourceWrapper): return [cls(oc, request=request) for oc in ocs] @classmethod + @handle_errors(_("Unable to retrieve deployment")) def get(cls, request, overcloud_id): """Return the Tuskar Overcloud that matches the ID @@ -489,6 +492,7 @@ class Node(base.APIResourceWrapper): return cls(node) @classmethod + @handle_errors(_("Unable to retrieve node")) def get(cls, request, uuid): """Return the Node in Ironic that matches the ID @@ -542,6 +546,7 @@ class Node(base.APIResourceWrapper): return cls(node, instance=server, request=request) @classmethod + @handle_errors(_("Unable to retrieve nodes"), []) def list(cls, request, associated=None): """Return a list of Nodes in Ironic @@ -799,6 +804,7 @@ class OvercloudRole(base.APIResourceWrapper): _attrs = ('id', 'name', 'description', 'image_name', 'flavor_id') @classmethod + @handle_errors(_("Unable to retrieve overcloud roles"), []) def list(cls, request): """Return a list of Overcloud Roles in Tuskar @@ -813,6 +819,7 @@ class OvercloudRole(base.APIResourceWrapper): return [cls(role) for role in roles] @classmethod + @handle_errors(_("Unable to retrieve overcloud role")) def get(cls, request, role_id): """Return the Tuskar OvercloudRole that matches the ID diff --git a/tuskar_ui/handle_errors.py b/tuskar_ui/handle_errors.py new file mode 100644 index 000000000..b1f14a3b5 --- /dev/null +++ b/tuskar_ui/handle_errors.py @@ -0,0 +1,56 @@ +# -*- coding: utf8 -*- +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import functools + +import horizon.exceptions + + +def handle_errors(error_message, error_default=None): + """A decorator for adding default error handling to API calls. + + It wraps the original method in a try-except block, with horizon's + error handling added. + + Note: it should only be used on methods that take request as the first + (second after self or cls) argument. + + The decorated method accepts a number of additional parameters: + + :param _error_handle: whether to handle the errors in this call + :param _error_message: override the error message + :param _error_default: override the default value returned on error + :param _error_redirect: specify a redirect url for errors + :param _error_ignore: ignore known errors + """ + def decorator(func): + @functools.wraps(func) + def wrapper(self, request, *args, **kwargs): + _error_handle = kwargs.pop('_error_handle', True) + _error_message = kwargs.pop('_error_message', error_message) + _error_default = kwargs.pop('_error_default', error_default) + _error_redirect = kwargs.pop('_error_redirect', None) + _error_ignore = kwargs.pop('_error_ignore', False) + if not _error_handle: + return func(self, request, *args, **kwargs) + try: + return func(self, request, *args, **kwargs) + except Exception: + horizon.exceptions.handle(request, _error_message, + ignore=_error_ignore, + redirect=_error_redirect) + return _error_default + wrapper.wrapped = func + return wrapper + return decorator diff --git a/tuskar_ui/infrastructure/node_profiles/tables.py b/tuskar_ui/infrastructure/node_profiles/tables.py index 1b7b7d1a6..9c2b9ad2a 100644 --- a/tuskar_ui/infrastructure/node_profiles/tables.py +++ b/tuskar_ui/infrastructure/node_profiles/tables.py @@ -14,7 +14,6 @@ from django.utils.translation import ugettext_lazy as _ -from horizon import exceptions from horizon import tables from openstack_dashboard.dashboards.admin.flavors \ @@ -46,13 +45,9 @@ class DeleteNodeProfile(flavor_tables.DeleteFlavor): :type datum: tuskar_ui.api.NodeProfile """ if datum is not None: - try: - deployed_profiles = api.NodeProfile.list_deployed_ids(request) - except Exception: - msg = _('Unable to retrieve existing servers list.') - exceptions.handle(request, msg) - return False - if datum.id in deployed_profiles: + deployed_profiles = api.NodeProfile.list_deployed_ids( + request, _error_default=None) + if deployed_profiles is None or datum.id in deployed_profiles: return False return super(DeleteNodeProfile, self).allowed(request, datum) diff --git a/tuskar_ui/infrastructure/node_profiles/views.py b/tuskar_ui/infrastructure/node_profiles/views.py index 0f24331d6..d40cb1a2c 100644 --- a/tuskar_ui/infrastructure/node_profiles/views.py +++ b/tuskar_ui/infrastructure/node_profiles/views.py @@ -12,9 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -from django.utils.translation import ugettext_lazy as _ - -from horizon import exceptions from horizon import tables from horizon import workflows @@ -30,12 +27,7 @@ class IndexView(tables.DataTableView): template_name = 'infrastructure/node_profiles/index.html' def get_data(self): - try: - node_profiles = api.NodeProfile.list(self.request) - except Exception: - exceptions.handle(self.request, - _('Unable to retrieve node profile list.')) - return [] + node_profiles = api.NodeProfile.list(self.request) node_profiles.sort(key=lambda np: (np.vcpus, np.ram, np.disk)) return node_profiles diff --git a/tuskar_ui/infrastructure/nodes/tabs.py b/tuskar_ui/infrastructure/nodes/tabs.py index 0b01133d4..0a4bc23f0 100644 --- a/tuskar_ui/infrastructure/nodes/tabs.py +++ b/tuskar_ui/infrastructure/nodes/tabs.py @@ -17,7 +17,6 @@ from django.core import urlresolvers from django.utils.translation import ugettext_lazy as _ -from horizon import exceptions from horizon import tabs from tuskar_ui import api @@ -30,14 +29,8 @@ class OverviewTab(tabs.Tab): template_name = ("infrastructure/nodes/_overview.html") def get_context_data(self, request): - try: - free_nodes = api.Node.list(request, associated=False) - deployed_nodes = api.Node.list(request, associated=True) - except Exception: - free_nodes = [] - deployed_nodes = [] - exceptions.handle(request, - _('Unable to retrieve nodes.')) + free_nodes = api.Node.list(request, associated=False) + deployed_nodes = api.Node.list(request, associated=True) free_nodes_down = [node for node in free_nodes if node.power_state != 'on'] @@ -67,25 +60,14 @@ class DeployedTab(tabs.TableTab): template_name = ("horizon/common/_detail_table.html") def get_items_count(self): - try: - deployed_nodes_count = len(api.Node.list(self.request, - associated=True)) - except Exception: - deployed_nodes_count = 0 - exceptions.handle(self.request, - _('Unable to retrieve deployed nodes count.')) + deployed_nodes_count = len(api.Node.list(self.request, + associated=True)) return deployed_nodes_count def get_deployed_nodes_data(self): - try: - deployed_nodes = api.Node.list(self.request, associated=True) - except Exception: - deployed_nodes = [] - redirect = urlresolvers.reverse( - 'horizon:infrastructure:nodes:index') - exceptions.handle(self.request, - _('Unable to retrieve deployed nodes.'), - redirect=redirect) + redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index') + deployed_nodes = api.Node.list(self.request, associated=True, + _error_redirect=redirect) return deployed_nodes @@ -97,25 +79,14 @@ class FreeTab(tabs.TableTab): template_name = ("horizon/common/_detail_table.html") def get_items_count(self): - try: - free_nodes_count = len(api.Node.list(self.request, - associated=False)) - except Exception: - free_nodes_count = "0" - exceptions.handle(self.request, - _('Unable to retrieve free nodes count.')) + free_nodes_count = len(api.Node.list(self.request, + associated=False)) return free_nodes_count def get_free_nodes_data(self): - try: - free_nodes = api.Node.list(self.request, associated=False) - except Exception: - free_nodes = [] - redirect = urlresolvers.reverse( - 'horizon:infrastructure:nodes:index') - exceptions.handle(self.request, - _('Unable to retrieve free nodes.'), - redirect=redirect) + redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index') + free_nodes = api.Node.list(self.request, associated=False, + _error_redirect=redirect) return free_nodes diff --git a/tuskar_ui/infrastructure/nodes/tests.py b/tuskar_ui/infrastructure/nodes/tests.py index e8e50b152..2d24068ea 100644 --- a/tuskar_ui/infrastructure/nodes/tests.py +++ b/tuskar_ui/infrastructure/nodes/tests.py @@ -18,6 +18,7 @@ from mock import patch, call # noqa from openstack_dashboard.test.test_data import utils from tuskar_ui import api as api +from tuskar_ui.handle_errors import handle_errors # noqa from tuskar_ui.test import helpers as test from tuskar_ui.test.test_data import tuskar_data @@ -30,6 +31,10 @@ tuskar_data.data(TEST_DATA) class NodesTests(test.BaseAdminViewTests): + @handle_errors("Error!", []) + def _raise_tuskar_exception(self, request, *args, **kwargs): + raise self.exceptions.tuskar + def test_index_get(self): with patch('tuskar_ui.api.Node', **{ @@ -65,10 +70,10 @@ class NodesTests(test.BaseAdminViewTests): def test_free_nodes_list_exception(self): with patch('tuskar_ui.api.Node', **{ 'spec_set': ['list'], - 'list.side_effect': self.exceptions.tuskar, + 'list.side_effect': self._raise_tuskar_exception, }) as mock: res = self.client.get(INDEX_URL + '?tab=nodes__free') - self.assertEqual(mock.list.call_count, 2) + self.assertEqual(mock.list.call_count, 3) self.assertRedirectsNoFollow(res, INDEX_URL) @@ -101,10 +106,10 @@ class NodesTests(test.BaseAdminViewTests): with patch('tuskar_ui.api.Node', **{ 'spec_set': ['list', 'instance'], 'instance': instance, - 'list.side_effect': self.exceptions.tuskar, + 'list.side_effect': self._raise_tuskar_exception, }) as mock: res = self.client.get(INDEX_URL + '?tab=nodes__deployed') - self.assertEqual(mock.list.call_count, 2) + self.assertEqual(mock.list.call_count, 3) self.assertRedirectsNoFollow(res, INDEX_URL) @@ -197,7 +202,7 @@ class NodesTests(test.BaseAdminViewTests): def test_node_detail_exception(self): with patch('tuskar_ui.api.Node', **{ 'spec_set': ['get'], - 'get.side_effect': self.exceptions.tuskar, + 'get.side_effect': self._raise_tuskar_exception, }) as mock: res = self.client.get( urlresolvers.reverse(DETAIL_VIEW, args=('no-such-node',)) diff --git a/tuskar_ui/infrastructure/nodes/views.py b/tuskar_ui/infrastructure/nodes/views.py index 792b2c0bc..cb99f647f 100644 --- a/tuskar_ui/infrastructure/nodes/views.py +++ b/tuskar_ui/infrastructure/nodes/views.py @@ -13,9 +13,7 @@ # under the License. from django.core.urlresolvers import reverse_lazy -from django.utils.translation import ugettext_lazy as _ -from horizon import exceptions from horizon import forms as horizon_forms from horizon import tabs as horizon_tabs from horizon import views as horizon_views @@ -30,23 +28,12 @@ class IndexView(horizon_tabs.TabbedTableView): template_name = 'infrastructure/nodes/index.html' def get_free_nodes_count(self): - try: - free_nodes_count = len(api.Node.list(self.request, - associated=False)) - except Exception: - free_nodes_count = 0 - exceptions.handle(self.request, - _('Unable to retrieve free nodes.')) + free_nodes_count = len(api.Node.list(self.request, associated=False)) return free_nodes_count def get_deployed_nodes_count(self): - try: - deployed_nodes_count = len(api.Node.list(self.request, - associated=True)) - except Exception: - deployed_nodes_count = 0 - exceptions.handle(self.request, - _('Unable to retrieve deployed nodes.')) + deployed_nodes_count = len(api.Node.list(self.request, + associated=True)) return deployed_nodes_count def get_context_data(self, **kwargs): @@ -81,14 +68,7 @@ class DetailView(horizon_views.APIView): def get_data(self, request, context, *args, **kwargs): node_uuid = kwargs.get('node_uuid') - - try: - node = api.Node.get(request, node_uuid) - except Exception: - node = None - redirect = reverse_lazy('horizon:infrastructure:nodes:index') - msg = _('Unable to retrieve node with UUID "%s"') % node_uuid - exceptions.handle(request, msg, redirect=redirect) - + redirect = reverse_lazy('horizon:infrastructure:nodes:index') + node = api.Node.get(request, node_uuid, _error_redirect=redirect) context['node'] = node return context diff --git a/tuskar_ui/infrastructure/overcloud/tabs.py b/tuskar_ui/infrastructure/overcloud/tabs.py index 207cb2245..2f4308a8b 100644 --- a/tuskar_ui/infrastructure/overcloud/tabs.py +++ b/tuskar_ui/infrastructure/overcloud/tabs.py @@ -16,7 +16,6 @@ from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext_lazy -from horizon import exceptions from horizon import tabs from tuskar_ui import api @@ -64,12 +63,7 @@ class OverviewTab(tabs.Tab): def get_context_data(self, request, **kwargs): overcloud = self.tab_group.kwargs['overcloud'] - try: - roles = api.OvercloudRole.list(request) - except Exception: - roles = [] - exceptions.handle(request, - _("Unable to retrieve overcloud roles.")) + roles = api.OvercloudRole.list(request) role_data = [_get_role_data(overcloud, role) for role in roles] total = sum(d['node_count'] for d in role_data) progress = 100 * sum(d.get('running_node_count', 0) diff --git a/tuskar_ui/infrastructure/overcloud/tests.py b/tuskar_ui/infrastructure/overcloud/tests.py index 97a872595..56d64ec32 100644 --- a/tuskar_ui/infrastructure/overcloud/tests.py +++ b/tuskar_ui/infrastructure/overcloud/tests.py @@ -60,11 +60,11 @@ def _mock_overcloud(**kwargs): 'update', ], 'counts': [], - 'create.side_effect': lambda *args: oc, + 'create.side_effect': lambda *args, **kwargs: oc, 'dashboard_url': '', 'delete.return_value': None, - 'get.side_effect': lambda *args: oc, - 'get_the_overcloud.side_effect': lambda *args: oc, + 'get.side_effect': lambda *args, **kwargs: oc, + 'get_the_overcloud.side_effect': lambda *args, **kwargs: oc, 'id': 1, 'is_deployed': True, 'is_deploying': False, @@ -72,7 +72,7 @@ def _mock_overcloud(**kwargs): 'resources.return_value': [], 'stack_events': [], 'stack': stack, - 'update.side_effect': lambda *args: oc, + 'update.side_effect': lambda *args, **kwargs: oc, } params.update(kwargs) with patch('tuskar_ui.api.Overcloud', **params) as Overcloud: @@ -379,7 +379,7 @@ class OvercloudTests(test.BaseAdminViewTests): 'image_name', 'flavor_id', ], - 'get.side_effect': lambda *args: role, + 'get.side_effect': lambda *args, **kwargs: role, 'name': 'Compute', 'description': '...', 'image_name': '', diff --git a/tuskar_ui/infrastructure/overcloud/views.py b/tuskar_ui/infrastructure/overcloud/views.py index fe23b3b0e..b86655e97 100644 --- a/tuskar_ui/infrastructure/overcloud/views.py +++ b/tuskar_ui/infrastructure/overcloud/views.py @@ -16,7 +16,6 @@ from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy as _ from django.views.generic import base as base_views -from horizon import exceptions import horizon.forms from horizon import tables as horizon_tables from horizon import tabs as horizon_tabs @@ -41,12 +40,8 @@ class OvercloudMixin(object): if redirect is None: redirect = reverse(INDEX_URL) overcloud_id = self.kwargs['overcloud_id'] - try: - overcloud = api.Overcloud.get(self.request, overcloud_id) - except Exception: - msg = _("Unable to retrieve deployment.") - exceptions.handle(self.request, msg, redirect=redirect) - + overcloud = api.Overcloud.get(self.request, overcloud_id, + _error_redirect=redirect) return overcloud @@ -54,11 +49,8 @@ class OvercloudRoleMixin(object): @memoized.memoized def get_role(self, redirect=None): role_id = self.kwargs['role_id'] - try: - role = api.OvercloudRole.get(self.request, role_id) - except Exception: - msg = _("Unable to retrieve overcloud role.") - exceptions.handle(self.request, msg, redirect=redirect) + role = api.OvercloudRole.get(self.request, role_id, + _error_redirect=redirect) return role