Merge "Remove true/false return from action.execute()"

watcher/applier/action_plan/default.py

@@ -53,16 +53,15 @@ class DefaultActionPlanHandler(base.BaseActionPlanHandler):
                         event_types.EventTypes.LAUNCH_ACTION_PLAN,
                         ap_objects.State.ONGOING)
             applier = default.DefaultApplier(self.ctx, self.applier_manager)
-            result = applier.execute(self.action_plan_uuid)
+            applier.execute(self.action_plan_uuid)
+            state = ap_objects.State.SUCCEEDED
+
         except Exception as e:
             LOG.exception(e)
-            result = False
+            state = ap_objects.State.FAILED
+
         finally:
-            if result is True:
-                status = ap_objects.State.SUCCEEDED
-            else:
-                status = ap_objects.State.FAILED
             # update state
             self.notify(self.action_plan_uuid,
                         event_types.EventTypes.LAUNCH_ACTION_PLAN,
-                        status)
+                        state)

watcher/applier/workflow_engine/default.py

@@ -22,6 +22,7 @@ from taskflow import task
 
 from watcher._i18n import _LE, _LW, _LC
 from watcher.applier.workflow_engine import base
+from watcher.common import exception
 from watcher.objects import action as obj_action
 
 LOG = log.getLogger(__name__)
@@ -77,10 +78,9 @@ class DefaultWorkFlowEngine(base.BaseWorkFlowEngine):
 
             e = engines.load(flow)
             e.run()
-            return True
+
         except Exception as e:
-            LOG.exception(e)
-            return False
+            raise exception.WorkflowExecutionException(error=e)
 
 
 class TaskFlowActionContainer(task.Task):
@@ -121,14 +121,9 @@ class TaskFlowActionContainer(task.Task):
         try:
             LOG.debug("Running action %s", self.name)
 
-            # todo(jed) remove return (true or false) raise an Exception
-            result = self.action.execute()
-            if result is not True:
-                self.engine.notify(self._db_action,
-                                   obj_action.State.FAILED)
-            else:
-                self.engine.notify(self._db_action,
-                                   obj_action.State.SUCCEEDED)
+            self.action.execute()
+            self.engine.notify(self._db_action,
+                               obj_action.State.SUCCEEDED)
         except Exception as e:
             LOG.exception(e)
             LOG.error(_LE('The WorkFlow Engine has failed '

watcher/common/exception.py

@@ -234,6 +234,9 @@ class PatchError(Invalid):
 
 # decision engine
 
+class WorkflowExecutionException(WatcherException):
+    msg_fmt = _('Workflow execution error: %(error)s')
+
 
 class IllegalArgumentException(WatcherException):
     msg_fmt = _('Illegal argument')

watcher/locale/watcher.pot

@@ -7,9 +7,9 @@
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: python-watcher 0.24.1.dev4\n"
+"Project-Id-Version: python-watcher 0.24.1.dev12\n"
 "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
-"POT-Creation-Date: 2016-03-14 15:29+0100\n"
+"POT-Creation-Date: 2016-03-16 18:18-0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -109,17 +109,17 @@ msgstr ""
 msgid "Migration of type %(migration_type)s is not supported."
 msgstr ""
 
-#: watcher/applier/workflow_engine/default.py:134
+#: watcher/applier/workflow_engine/default.py:129
 #, python-format
 msgid "The WorkFlow Engine has failed to execute the action %s"
 msgstr ""
 
-#: watcher/applier/workflow_engine/default.py:152
+#: watcher/applier/workflow_engine/default.py:147
 #, python-format
 msgid "Revert action %s"
 msgstr ""
 
-#: watcher/applier/workflow_engine/default.py:158
+#: watcher/applier/workflow_engine/default.py:153
 msgid "Oops! We need disaster recover plan"
 msgstr ""
 
@@ -292,64 +292,69 @@ msgstr ""
 msgid "Couldn't apply patch '%(patch)s'. Reason: %(reason)s"
 msgstr ""
 
-#: watcher/common/exception.py:239
+#: watcher/common/exception.py:238
+#, python-format
+msgid "Workflow execution error: %(error)s"
+msgstr ""
+
+#: watcher/common/exception.py:242
 msgid "Illegal argument"
 msgstr ""
 
-#: watcher/common/exception.py:243
+#: watcher/common/exception.py:246
 msgid "No such metric"
 msgstr ""
 
-#: watcher/common/exception.py:247
+#: watcher/common/exception.py:250
 msgid "No rows were returned"
 msgstr ""
 
-#: watcher/common/exception.py:251
+#: watcher/common/exception.py:254
 #, python-format
 msgid "%(client)s connection failed. Reason: %(reason)s"
 msgstr ""
 
-#: watcher/common/exception.py:255
+#: watcher/common/exception.py:258
 msgid "'Keystone API endpoint is missing''"
 msgstr ""
 
-#: watcher/common/exception.py:259
+#: watcher/common/exception.py:262
 msgid "The list of hypervisor(s) in the cluster is empty"
 msgstr ""
 
-#: watcher/common/exception.py:263
+#: watcher/common/exception.py:266
 msgid "The metrics resource collector is not defined"
 msgstr ""
 
-#: watcher/common/exception.py:267
+#: watcher/common/exception.py:270
 msgid "the cluster state is not defined"
 msgstr ""
 
-#: watcher/common/exception.py:273
+#: watcher/common/exception.py:276
 #, python-format
 msgid "The instance '%(name)s' is not found"
 msgstr ""
 
-#: watcher/common/exception.py:277
+#: watcher/common/exception.py:280
 msgid "The hypervisor is not found"
 msgstr ""
 
-#: watcher/common/exception.py:281
+#: watcher/common/exception.py:284
 #, python-format
 msgid "Error loading plugin '%(name)s'"
 msgstr ""
 
-#: watcher/common/exception.py:285
+#: watcher/common/exception.py:288
 #, python-format
 msgid "The identifier '%(name)s' is a reserved word"
 msgstr ""
 
-#: watcher/common/exception.py:289
+#: watcher/common/exception.py:292
 #, python-format
 msgid "The %(name)s resource %(id)s is not soft deleted"
 msgstr ""
 
-#: watcher/common/exception.py:293
+#: watcher/common/exception.py:296
 msgid "Limit should be positive"
 msgstr ""
 

watcher/tests/applier/workflow_engine/test_default_workflow_engine.py

@@ -25,6 +25,7 @@ from stevedore import extension
 
 from watcher.applier.actions import base as abase
 from watcher.applier.workflow_engine import default as tflow
+from watcher.common import exception
 from watcher.common import utils
 from watcher import objects
 from watcher.tests.db import base
@@ -63,10 +64,15 @@ class TestDefaultWorkFlowEngine(base.DbTestCase):
             context=self.context,
             applier_manager=mock.MagicMock())
 
-    def test_execute(self):
+    @mock.patch('taskflow.engines.load')
+    @mock.patch('taskflow.patterns.graph_flow.Flow.link')
+    def test_execute(self, graph_flow, engines):
         actions = mock.MagicMock()
-        result = self.engine.execute(actions)
-        self.assertEqual(True, result)
+        try:
+            self.engine.execute(actions)
+            self.assertTrue(engines.called)
+        except Exception as exc:
+            self.fail(exc)
 
     def create_action(self, action_type, parameters, next):
         action = {
@@ -91,64 +97,84 @@ class TestDefaultWorkFlowEngine(base.DbTestCase):
         for a in actions:
             self.check_action_state(a, expected_state)
 
-    def test_execute_with_no_actions(self):
+    @mock.patch('taskflow.engines.load')
+    @mock.patch('taskflow.patterns.graph_flow.Flow.link')
+    def test_execute_with_no_actions(self, graph_flow, engines):
         actions = []
-        result = self.engine.execute(actions)
-        self.assertEqual(True, result)
+        try:
+            self.engine.execute(actions)
+            self.assertFalse(graph_flow.called)
+            self.assertTrue(engines.called)
+        except Exception as exc:
+            self.fail(exc)
 
     def test_execute_with_one_action(self):
         actions = [self.create_action("nop", {'message': 'test'}, None)]
-        result = self.engine.execute(actions)
-        self.assertEqual(True, result)
-        self.check_actions_state(actions, objects.action.State.SUCCEEDED)
+        try:
+            self.engine.execute(actions)
+            self.check_actions_state(actions, objects.action.State.SUCCEEDED)
+
+        except Exception as exc:
+            self.fail(exc)
 
     def test_execute_with_two_actions(self):
         actions = []
-        next = self.create_action("sleep", {'duration': 0.0}, None)
-        first = self.create_action("nop", {'message': 'test'}, next.id)
+        second = self.create_action("sleep", {'duration': 0.0}, None)
+        first = self.create_action("nop", {'message': 'test'}, second.id)
 
         actions.append(first)
-        actions.append(next)
+        actions.append(second)
 
-        result = self.engine.execute(actions)
-        self.assertEqual(True, result)
-        self.check_actions_state(actions, objects.action.State.SUCCEEDED)
+        try:
+            self.engine.execute(actions)
+            self.check_actions_state(actions, objects.action.State.SUCCEEDED)
+
+        except Exception as exc:
+            self.fail(exc)
 
     def test_execute_with_three_actions(self):
         actions = []
-        next2 = self.create_action("nop", {'message': 'next'}, None)
-        next = self.create_action("sleep", {'duration': 0.0}, next2.id)
-        first = self.create_action("nop", {'message': 'hello'}, next.id)
+
+        third = self.create_action("nop", {'message': 'next'}, None)
+        second = self.create_action("sleep", {'duration': 0.0}, third.id)
+        first = self.create_action("nop", {'message': 'hello'}, second.id)
+
         self.check_action_state(first, objects.action.State.PENDING)
-        self.check_action_state(next, objects.action.State.PENDING)
-        self.check_action_state(next2, objects.action.State.PENDING)
+        self.check_action_state(second, objects.action.State.PENDING)
+        self.check_action_state(third, objects.action.State.PENDING)
 
         actions.append(first)
-        actions.append(next)
-        actions.append(next2)
+        actions.append(second)
+        actions.append(third)
 
-        result = self.engine.execute(actions)
-        self.assertEqual(True, result)
-        self.check_actions_state(actions, objects.action.State.SUCCEEDED)
+        try:
+            self.engine.execute(actions)
+            self.check_actions_state(actions, objects.action.State.SUCCEEDED)
+
+        except Exception as exc:
+            self.fail(exc)
 
     def test_execute_with_exception(self):
         actions = []
-        next2 = self.create_action("no_exist", {'message': 'next'}, None)
-        next = self.create_action("sleep", {'duration': 0.0}, next2.id)
-        first = self.create_action("nop", {'message': 'hello'}, next.id)
+
+        third = self.create_action("no_exist", {'message': 'next'}, None)
+        second = self.create_action("sleep", {'duration': 0.0}, third.id)
+        first = self.create_action("nop", {'message': 'hello'}, second.id)
 
         self.check_action_state(first, objects.action.State.PENDING)
-        self.check_action_state(next, objects.action.State.PENDING)
-        self.check_action_state(next2, objects.action.State.PENDING)
-        actions.append(first)
-        actions.append(next)
-        actions.append(next2)
+        self.check_action_state(second, objects.action.State.PENDING)
+        self.check_action_state(third, objects.action.State.PENDING)
+
+        actions.append(first)
+        actions.append(second)
+        actions.append(third)
+
+        self.assertRaises(exception.WorkflowExecutionException,
+                          self.engine.execute, actions)
 
-        result = self.engine.execute(actions)
-        self.assertEqual(False, result)
         self.check_action_state(first, objects.action.State.SUCCEEDED)
-        self.check_action_state(next, objects.action.State.SUCCEEDED)
-        self.check_action_state(next2, objects.action.State.FAILED)
+        self.check_action_state(second, objects.action.State.SUCCEEDED)
+        self.check_action_state(third, objects.action.State.FAILED)
 
     @mock.patch("watcher.common.loader.default.DriverManager")
     def test_execute_with_action_exception(self, m_driver):
@@ -161,6 +187,7 @@ class TestDefaultWorkFlowEngine(base.DbTestCase):
                                           obj=None),
             namespace=FakeAction.namespace())
         actions = [self.create_action("dontcare", {}, None)]
-        result = self.engine.execute(actions)
-        self.assertEqual(False, result)
+
+        self.assertRaises(exception.WorkflowExecutionException,
+                          self.engine.execute, actions)
         self.check_action_state(actions[0], objects.action.State.FAILED)