Skip to content

Commit

Permalink
Adds a method for overriding specific API messages.
Browse files Browse the repository at this point in the history
Also provides a specific error message for dependent snapshots
during volume deletion.

Fixes bug 1037241. Fixes bug 1020326.

Change-Id: I40e5f537bc1806ec97c21f3eeea1d74beca04250
  • Loading branch information
ttrifonov authored and gabrielhurley committed Sep 7, 2012
1 parent eb21d51 commit ceb22f1
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 4 deletions.
10 changes: 9 additions & 1 deletion horizon/dashboards/nova/volumes/tables.py
Expand Up @@ -37,7 +37,15 @@ class DeleteVolume(tables.DeleteAction):
data_type_plural = _("Volumes")

def delete(self, request, obj_id):
api.volume_delete(request, obj_id)
obj = self.table.get_object_by_id(obj_id)
name = self.table.get_object_display(obj)
try:
api.volume_delete(request, obj_id)
except:
msg = _('Unable to delete volume "%s". One or more snapshots '
'depend on it.')
exceptions.check_message(["snapshots", "dependent"], msg % name)
raise

def allowed(self, request, volume=None):
if volume:
Expand Down
51 changes: 51 additions & 0 deletions horizon/dashboards/nova/volumes/tests.py
Expand Up @@ -180,6 +180,57 @@ def test_create_volume_number_over_alloted_quota(self):
' volumes.']
self.assertEqual(res.context['form'].errors['__all__'], expected_error)

@test.create_stubs({api: ('volume_list',
'volume_delete',
'server_list')})
def test_delete_volume(self):
volume = self.volumes.first()
formData = {'action':
'volumes__delete__%s' % volume.id}

api.volume_list(IsA(http.HttpRequest), search_opts=None).\
AndReturn(self.volumes.list())
api.volume_delete(IsA(http.HttpRequest), volume.id)
api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())
api.volume_list(IsA(http.HttpRequest), search_opts=None).\
AndReturn(self.volumes.list())
api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())

self.mox.ReplayAll()

url = reverse('horizon:nova:volumes:index')
res = self.client.post(url, formData, follow=True)
self.assertMessageCount(res, count=0)

@test.create_stubs({api: ('volume_list',
'volume_delete',
'server_list')})
def test_delete_volume_error_existing_snapshot(self):
volume = self.volumes.first()
formData = {'action':
'volumes__delete__%s' % volume.id}
exc = self.exceptions.cinder.__class__(400,
"error: dependent snapshots")

api.volume_list(IsA(http.HttpRequest), search_opts=None).\
AndReturn(self.volumes.list())
api.volume_delete(IsA(http.HttpRequest), volume.id). \
AndRaise(exc)
api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())
api.volume_list(IsA(http.HttpRequest), search_opts=None).\
AndReturn(self.volumes.list())
api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())

self.mox.ReplayAll()

url = reverse('horizon:nova:volumes:index')
res = self.client.post(url, formData, follow=True)
self.assertMessageCount(res, error=1)
self.assertEqual(list(res.context['messages'])[0].message,
u'Unable to delete volume "%s". '
u'One or more snapshots depend on it.' %
volume.display_name)

@test.create_stubs({api: ('volume_get',), api.nova: ('server_list',)})
def test_edit_attachments(self):
volume = self.volumes.first()
Expand Down
15 changes: 15 additions & 0 deletions horizon/exceptions.py
Expand Up @@ -206,6 +206,18 @@ def error_color(msg):
return termcolors.colorize(msg, **PALETTE['ERROR'])


def check_message(keywords, message):
"""
Checks an exception for given keywords and raises a new ``ActionError``
with the desired message if the keywords are found. This allows selective
control over API error messages.
"""
exc_type, exc_value, exc_traceback = sys.exc_info()
if set(str(exc_value).split(" ")).issuperset(set(keywords)):
exc_value._safe_message = message
raise


def handle(request, message=None, redirect=None, ignore=False,
escalate=False, log_level=None, force_log=None):
""" Centralized error handling for Horizon.
Expand Down Expand Up @@ -254,6 +266,9 @@ class indicating the type of exception that was encountered will be
# We trust messages from our own exceptions
if issubclass(exc_type, HorizonException):
message = exc_value
# Check for an override message
elif getattr(exc_value, "_safe_message", None):
message = exc_value._safe_message
# If the message has a placeholder for the exception, fill it in
elif message and "%(exc)s" in message:
message = message % {"exc": exc_value}
Expand Down
10 changes: 7 additions & 3 deletions horizon/tables/actions.py
Expand Up @@ -513,12 +513,16 @@ def handle(self, table, request, obj_ids):
self.success_ids.append(datum_id)
LOG.info('%s: "%s"' %
(self._conjugate(past=True), datum_display))
except:
except Exception, ex:
# Handle the exception but silence it since we'll display
# an aggregate error message later. Otherwise we'd get
# multiple error messages displayed to the user.
exceptions.handle(request, ignore=True)
action_failure.append(datum_display)
if getattr(ex, "_safe_message", None):
ignore = False
else:
ignore = True
action_failure.append(datum_display)
exceptions.handle(request, ignore=ignore)

#Begin with success message class, downgrade to info if problems
success_message_level = messages.success
Expand Down
4 changes: 4 additions & 0 deletions horizon/tests/test_data/exceptions.py
Expand Up @@ -17,6 +17,7 @@
from novaclient import exceptions as nova_exceptions
from quantumclient.common import exceptions as quantum_exceptions
from swiftclient import client as swift_exceptions
from cinderclient import exceptions as cinder_exceptions

from .utils import TestDataContainer

Expand Down Expand Up @@ -61,3 +62,6 @@ def data(TEST):

swift_exception = swift_exceptions.ClientException
TEST.exceptions.swift = create_stubbed_exception(swift_exception)

cinder_exception = cinder_exceptions.BadRequest
TEST.exceptions.cinder = create_stubbed_exception(cinder_exception)

0 comments on commit ceb22f1

Please sign in to comment.