x/tuskar-ui
Code Issues Proposed changes

Error handling in the API

Browse Source 
Currently error handling in Tuskar-UI involves a lot of repeated
code, as the same pattern is used everywhere and the same API calls
tend to be handled in the same way no matter where they appear.

This patch introduces a decorator that can be used to add default
error handling to all API calls that take a request as a parameter.

That default error handling can be disabled or modified using the
special parameters passed to the call, all of which start with
"_error" to avoid conflicts.

Change-Id: Ie441789c471deeb6d64267bf4424f5661ef073af
This commit is contained in:
Radomir Dopieralski 2014-02-27 11:33:59 +01:00
parent 9af7f5b6a2
commit d7e9c9a0cd
10 changed files with 109 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
tuskar_ui/api.py
View File

@ -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

56
tuskar_ui/handle_errors.py Normal file
View File

@ -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

11
tuskar_ui/infrastructure/node_profiles/tables.py
View File

@ -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)

10
tuskar_ui/infrastructure/node_profiles/views.py
View File

@ -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

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

@ -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

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

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

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

@ -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

8
tuskar_ui/infrastructure/overcloud/tabs.py
View File

@ -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)

10
tuskar_ui/infrastructure/overcloud/tests.py
View File

@ -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': '',

16
tuskar_ui/infrastructure/overcloud/views.py
View File

@ -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