Merge "Error handling in the API"

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
 

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

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(
-                deployed_profiles = api.NodeProfile.list_deployed_ids(request)
+                request, _error_default=None)
-            except Exception:
+            if deployed_profiles is None or datum.id in deployed_profiles:
-                msg = _('Unable to retrieve existing servers list.')
-                exceptions.handle(request, msg)
-                return False
-            if datum.id in deployed_profiles:
                 return False
         return super(DeleteNodeProfile, self).allowed(request, datum)
 

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)
-            node_profiles = api.NodeProfile.list(self.request)
-        except Exception:
-            exceptions.handle(self.request,
-                              _('Unable to retrieve node profile list.'))
-            return []
         node_profiles.sort(key=lambda np: (np.vcpus, np.ram, np.disk))
         return node_profiles
 

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)
-            free_nodes = api.Node.list(request, associated=False)
+        deployed_nodes = api.Node.list(request, associated=True)
-            deployed_nodes = api.Node.list(request, associated=True)
-        except Exception:
-            free_nodes = []
-            deployed_nodes = []
-            exceptions.handle(request,
-                              _('Unable to retrieve nodes.'))
 
         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,
-            deployed_nodes_count = len(api.Node.list(self.request,
+                                                 associated=True))
-                                                     associated=True))
-        except Exception:
-            deployed_nodes_count = 0
-            exceptions.handle(self.request,
-                              _('Unable to retrieve deployed nodes count.'))
         return deployed_nodes_count
 
     def get_deployed_nodes_data(self):
-        try:
+        redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index')
-            deployed_nodes = api.Node.list(self.request, associated=True)
+        deployed_nodes = api.Node.list(self.request, associated=True,
-        except Exception:
+                                       _error_redirect=redirect)
-            deployed_nodes = []
-            redirect = urlresolvers.reverse(
-                'horizon:infrastructure:nodes:index')
-            exceptions.handle(self.request,
-                              _('Unable to retrieve deployed nodes.'),
-                              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,
-            free_nodes_count = len(api.Node.list(self.request,
+                                             associated=False))
-                                                 associated=False))
-        except Exception:
-            free_nodes_count = "0"
-            exceptions.handle(self.request,
-                              _('Unable to retrieve free nodes count.'))
         return free_nodes_count
 
     def get_free_nodes_data(self):
-        try:
+        redirect = urlresolvers.reverse('horizon:infrastructure:nodes:index')
-            free_nodes = api.Node.list(self.request, associated=False)
+        free_nodes = api.Node.list(self.request, associated=False,
-        except Exception:
+                                   _error_redirect=redirect)
-            free_nodes = []
-            redirect = urlresolvers.reverse(
-                'horizon:infrastructure:nodes:index')
-            exceptions.handle(self.request,
-                              _('Unable to retrieve free nodes.'),
-                              redirect=redirect)
         return free_nodes
 
 

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',))

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))
-            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.'))
         return free_nodes_count
 
     def get_deployed_nodes_count(self):
-        try:
+        deployed_nodes_count = len(api.Node.list(self.request,
-            deployed_nodes_count = len(api.Node.list(self.request,
+                                                 associated=True))
-                                                     associated=True))
-        except Exception:
-            deployed_nodes_count = 0
-            exceptions.handle(self.request,
-                              _('Unable to retrieve deployed nodes.'))
         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')
-
+        redirect = reverse_lazy('horizon:infrastructure:nodes:index')
-        try:
+        node = api.Node.get(request, node_uuid, _error_redirect=redirect)
-            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)
-
         context['node'] = node
         return context

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)
-            roles = api.OvercloudRole.list(request)
-        except Exception:
-            roles = []
-            exceptions.handle(request,
-                              _("Unable to retrieve overcloud roles."))
         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)

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.side_effect': lambda *args, **kwargs: oc,
-        'get_the_overcloud.side_effect': lambda *args: 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': '',

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,
-            overcloud = api.Overcloud.get(self.request, overcloud_id)
+                                      _error_redirect=redirect)
-        except Exception:
-            msg = _("Unable to retrieve deployment.")
-            exceptions.handle(self.request, msg, 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,
-            role = api.OvercloudRole.get(self.request, role_id)
+                                     _error_redirect=redirect)
-        except Exception:
-            msg = _("Unable to retrieve overcloud role.")
-            exceptions.handle(self.request, msg, redirect=redirect)
         return role