Merge branch 'feature-operations-hre' into 'master'

Handle and use HumanReadableExceptions

circle/common/models.py

@@ -23,6 +23,7 @@ from logging import getLogger
 from time import time
 from warnings import warn
 
+from django.contrib import messages
 from django.contrib.auth.models import User
 from django.core.cache import cache
 from django.core.serializers.json import DjangoJSONEncoder
@@ -46,17 +47,24 @@ class WorkerNotFound(Exception):
 
 def activitycontextimpl(act, on_abort=None, on_commit=None):
     try:
-        yield act
-    except BaseException as e:
-        # BaseException is the common parent of Exception and
-        # system-exiting exceptions, e.g. KeyboardInterrupt
+        try:
+            yield act
+        except HumanReadableException as e:
+            result = e
+            raise
+        except BaseException as e:
+            # BaseException is the common parent of Exception and
+            # system-exiting exceptions, e.g. KeyboardInterrupt
+            result = create_readable(
+                ugettext_noop("Failure."),
+                ugettext_noop("Unhandled exception: %(error)s"),
+                error=unicode(e))
+            raise
+    except:
+        logger.exception("Failed activity %s" % unicode(act))
         handler = None if on_abort is None else lambda a: on_abort(a, e)
-        result = create_readable(ugettext_noop("Failure."),
-                                 ugettext_noop("Unhandled exception: "
-                                               "%(error)s"),
-                                 error=unicode(e))
         act.finish(succeeded=False, result=result, event_handler=handler)
-        raise e
+        raise
     else:
         act.finish(succeeded=True, event_handler=on_commit)
 
@@ -196,6 +204,10 @@ class ActivityModel(TimeStampedModel):
                  DeprecationWarning, stacklevel=2)
             value = create_readable(user_text_template="",
                                     admin_text_template=value)
+        elif not hasattr(value, "to_dict"):
+            warn("Use HumanReadableObject.", DeprecationWarning, stacklevel=2)
+            value = create_readable(user_text_template="",
+                                    admin_text_template=unicode(value))
 
         self.result_data = None if value is None else value.to_dict()
 
@@ -361,8 +373,9 @@ class HumanReadableObject(object):
 
     @classmethod
     def create(cls, user_text_template, admin_text_template=None, **params):
-        return cls(user_text_template,
-                   admin_text_template or user_text_template, params)
+        return cls(user_text_template=user_text_template,
+                   admin_text_template=(admin_text_template
+                                        or user_text_template), params=params)
 
     def set(self, user_text_template, admin_text_template=None, **params):
         self._set_values(user_text_template,
@@ -407,10 +420,28 @@ create_readable = HumanReadableObject.create
 class HumanReadableException(HumanReadableObject, Exception):
     """HumanReadableObject that is an Exception so can used in except clause.
     """
-    pass
+    def __init__(self, level=None, *args, **kwargs):
+        super(HumanReadableException, self).__init__(*args, **kwargs)
+        if level is not None:
+            if hasattr(messages, level):
+                self.level = level
+            else:
+                raise ValueError(
+                    "Level should be the name of an attribute of django."
+                    "contrib.messages (and it should be callable with "
+                    "(request, message)). Like 'error', 'warning'.")
+        else:
+            self.level = "error"
+
+    def send_message(self, request, level=None):
+        if request.user and request.user.is_superuser:
+            msg = self.get_admin_text()
+        else:
+            msg = self.get_user_text()
+        getattr(messages, level or self.level)(request, msg)
 
 
-def humanize_exception(message, exception=None, **params):
+def humanize_exception(message, exception=None, level=None, **params):
     """Return new dynamic-class exception which is based on
     HumanReadableException and the original class with the dict of exception.
 
@@ -419,8 +450,10 @@ def humanize_exception(message, exception=None, **params):
     ...
     Welcome!
     """
-
     Ex = type("HumanReadable" + type(exception).__name__,
               (HumanReadableException, type(exception)),
               exception.__dict__)
-    return Ex.create(message, **params)
+    ex = Ex.create(message, **params)
+    if level:
+        ex.level = level
+    return ex

circle/dashboard/tests/test_views.py

@@ -529,36 +529,38 @@ class VmDetailTest(LoginMixin, TestCase):
     def test_permitted_wake_up_wrong_state(self):
         c = Client()
         self.login(c, "user2")
-        with patch.object(WakeUpOperation, 'async') as mock_method:
+        with patch.object(WakeUpOperation, 'async') as mock_method, \
+                patch.object(Instance.WrongStateError, 'send_message') as wro:
             inst = Instance.objects.get(pk=1)
             mock_method.side_effect = inst.wake_up
             inst.status = 'RUNNING'
             inst.set_level(self.u2, 'owner')
-            with patch('dashboard.views.messages') as msg:
-                c.post("/dashboard/vm/1/op/wake_up/")
-                assert msg.error.called
+            c.post("/dashboard/vm/1/op/wake_up/")
             inst = Instance.objects.get(pk=1)
             self.assertEqual(inst.status, 'RUNNING')  # mocked anyway
             assert mock_method.called
+            assert wro.called
 
     def test_permitted_wake_up(self):
         c = Client()
         self.login(c, "user2")
-        with patch.object(Instance, 'select_node', return_value=None):
-            with patch.object(WakeUpOperation, 'async') as new_wake_up:
-                with patch('vm.tasks.vm_tasks.wake_up.apply_async') as wuaa:
-                    inst = Instance.objects.get(pk=1)
-                    new_wake_up.side_effect = inst.wake_up
-                    inst.get_remote_queue_name = Mock(return_value='test')
-                    inst.status = 'SUSPENDED'
-                    inst.set_level(self.u2, 'owner')
-                    with patch('dashboard.views.messages') as msg:
-                        response = c.post("/dashboard/vm/1/op/wake_up/")
-                        assert not msg.error.called
-                    self.assertEqual(response.status_code, 302)
-                    self.assertEqual(inst.status, 'RUNNING')
-                    assert new_wake_up.called
-                    assert wuaa.called
+        with patch.object(Instance, 'select_node', return_value=None), \
+                patch.object(WakeUpOperation, 'async') as new_wake_up, \
+                patch('vm.tasks.vm_tasks.wake_up.apply_async') as wuaa, \
+                patch.object(Instance.WrongStateError, 'send_message') as wro:
+            inst = Instance.objects.get(pk=1)
+            new_wake_up.side_effect = inst.wake_up
+            inst.get_remote_queue_name = Mock(return_value='test')
+            inst.status = 'SUSPENDED'
+            inst.set_level(self.u2, 'owner')
+            with patch('dashboard.views.messages') as msg:
+                response = c.post("/dashboard/vm/1/op/wake_up/")
+                assert not msg.error.called
+            self.assertEqual(response.status_code, 302)
+            self.assertEqual(inst.status, 'RUNNING')
+            assert new_wake_up.called
+            assert wuaa.called
+            assert not wro.called
 
     def test_unpermitted_wake_up(self):
         c = Client()

circle/dashboard/views.py

@@ -71,7 +71,7 @@ from .tables import (
     NodeListTable, NodeVmListTable, TemplateListTable, LeaseListTable,
     GroupListTable, UserKeyListTable
 )
-from common.models import HumanReadableObject
+from common.models import HumanReadableObject, HumanReadableException
 from vm.models import (
     Instance, instance_activity, InstanceActivity, InstanceTemplate, Interface,
     InterfaceTemplate, Lease, Node, NodeActivity, Trait,
@@ -562,9 +562,13 @@ class OperationView(RedirectToLoginMixin, DetailView):
         done = False
         try:
             task = self.get_op().async(user=request.user, **extra)
+        except HumanReadableException as e:
+            e.send_message(request)
+            logger.exception("Could not start operation")
+            result = e
         except Exception as e:
             messages.error(request, _('Could not start operation.'))
-            logger.exception(e)
+            logger.exception("Could not start operation")
             result = e
         else:
             wait = self.wait_for_result
@@ -575,6 +579,10 @@ class OperationView(RedirectToLoginMixin, DetailView):
                 except TimeoutError:
                     logger.debug("Result didn't arrive in %ss",
                                  self.wait_for_result, exc_info=True)
+                except HumanReadableException as e:
+                    e.send_message(request)
+                    logger.exception(e)
+                    result = e
                 except Exception as e:
                     messages.error(request, _('Operation failed.'))
                     logger.debug("Operation failed.", exc_info=True)

circle/vm/models/instance.py

@@ -41,7 +41,7 @@ from model_utils.models import TimeStampedModel, StatusModel
 from taggit.managers import TaggableManager
 
 from acl.models import AclBase
-from common.models import create_readable
+from common.models import create_readable, HumanReadableException
 from common.operations import OperatedMixin
 from ..tasks import vm_tasks, agent_tasks
 from .activity import (ActivityInProgressError, instance_activity,
@@ -276,28 +276,26 @@ class Instance(AclBase, VirtualMachineDescModel, StatusModel, OperatedMixin,
         verbose_name = _('instance')
         verbose_name_plural = _('instances')
 
-    class InstanceDestroyedError(Exception):
+    class InstanceError(HumanReadableException):
 
-        def __init__(self, instance, message=None):
-            if message is None:
-                message = ("The instance (%s) has already been destroyed."
-                           % instance)
+        def __init__(self, instance, params=None, level=None, **kwargs):
+            kwargs.update(params or {})
+            self.instance = kwargs["instance"] = instance
+            super(Instance.InstanceError, self).__init__(
+                level, self.message, self.message, kwargs)
 
-            Exception.__init__(self, message)
+    class InstanceDestroyedError(InstanceError):
+        message = ugettext_noop(
+            "Instance %(instance)s has already been destroyed.")
 
-            self.instance = instance
+    class WrongStateError(InstanceError):
+        message = ugettext_noop(
+            "Current state (%(state)s) of instance %(instance)s is "
+            "inappropriate for the invoked operation.")
 
-    class WrongStateError(Exception):
-
-        def __init__(self, instance, message=None):
-            if message is None:
-                message = ("The instance's current state (%s) is "
-                           "inappropriate for the invoked operation."
-                           % instance.status)
-
-            Exception.__init__(self, message)
-
-            self.instance = instance
+        def __init__(self, instance, params=None, **kwargs):
+            super(Instance.WrongStateError, self).__init__(
+                instance, params, state=instance.status)
 
     def __unicode__(self):
         parts = (self.name, "(" + str(self.id) + ")")