common: refactor Operation.activity_code_suffix to method

circle/common/operations.py

@@ -46,13 +46,9 @@ class Operation(object):
     abortable = False
     has_percentage = False
 
-    @property
-    def activity_code_suffix(self):
-        return getattr(self, "_real_activity_code_suffix", self.id)
-
-    @activity_code_suffix.setter
-    def activity_code_suffix(self, value):
-        self._real_activity_code_suffix = value
+    @classmethod
+    def get_activity_code_suffix(cls):
+        return cls.id
 
     def __call__(self, **kwargs):
         return self.call(**kwargs)
@@ -250,7 +246,7 @@ class OperatedMixin(object):
                   operation could be found.
         """
         for op in getattr(self, operation_registry_name, {}).itervalues():
-            if has_suffix(activity_code, op.activity_code_suffix):
+            if has_suffix(activity_code, op.get_activity_code_suffix()):
                 return op(self)
         else:
             return None

circle/common/tests/test_operations.py

@@ -27,9 +27,7 @@ class OperationTestCase(TestCase):
         class AbortEx(Exception):
             pass
 
-        op = Operation(MagicMock())
-        op.activity_code_suffix = 'test'
-        op.id = 'test'
+        op = TestOp(MagicMock())
         op.async_operation = MagicMock(
             apply_async=MagicMock(side_effect=AbortEx))
 
@@ -44,9 +42,7 @@ class OperationTestCase(TestCase):
         class AbortEx(Exception):
             pass
 
-        op = Operation(MagicMock())
-        op.activity_code_suffix = 'test'
-        op.id = 'test'
+        op = TestOp(MagicMock())
         with patch.object(Operation, 'create_activity', side_effect=AbortEx):
             with patch.object(Operation, 'check_precond') as chk_pre:
                 try:
@@ -55,9 +51,7 @@ class OperationTestCase(TestCase):
                     self.assertTrue(chk_pre.called)
 
     def test_auth_check_on_non_system_call(self):
-        op = Operation(MagicMock())
-        op.activity_code_suffix = 'test'
-        op.id = 'test'
+        op = TestOp(MagicMock())
         user = MagicMock()
         with patch.object(Operation, 'check_auth') as check_auth:
             with patch.object(Operation, 'check_precond'), \
@@ -67,9 +61,7 @@ class OperationTestCase(TestCase):
             check_auth.assert_called_with(user)
 
     def test_no_auth_check_on_system_call(self):
-        op = Operation(MagicMock())
-        op.activity_code_suffix = 'test'
-        op.id = 'test'
+        op = TestOp(MagicMock())
         with patch.object(Operation, 'check_auth', side_effect=AssertionError):
             with patch.object(Operation, 'check_precond'), \
                     patch.object(Operation, 'create_activity'), \
@@ -77,39 +69,25 @@ class OperationTestCase(TestCase):
                 op.call(system=True)
 
     def test_no_exception_for_more_arguments_when_operation_takes_kwargs(self):
-        class KwargOp(Operation):
-            activity_code_suffix = 'test'
-            id = 'test'
-
-            def _operation(self, **kwargs):
-                pass
-
-        op = KwargOp(MagicMock())
-        with patch.object(KwargOp, 'create_activity'), \
-                patch.object(KwargOp, '_exec_op'):
+        op = TestOp(MagicMock())
+        with patch.object(TestOp, 'create_activity'), \
+                patch.object(TestOp, '_exec_op'):
             op.call(system=True, foo=42)
 
     def test_exception_for_unexpected_arguments(self):
-        class TestOp(Operation):
-            activity_code_suffix = 'test'
-            id = 'test'
-
-            def _operation(self):
-                pass
-
         op = TestOp(MagicMock())
         with patch.object(TestOp, 'create_activity'), \
                 patch.object(TestOp, '_exec_op'):
-            self.assertRaises(TypeError, op.call, system=True, foo=42)
+            self.assertRaises(TypeError, op.call, system=True, bar=42)
 
     def test_exception_for_missing_arguments(self):
-        class TestOp(Operation):
-            activity_code_suffix = 'test'
+        op = TestOp(MagicMock())
+        with patch.object(TestOp, 'create_activity'):
+            self.assertRaises(TypeError, op.call, system=True)
+
+
+class TestOp(Operation):
     id = 'test'
 
     def _operation(self, foo):
         pass
-
-        op = TestOp(MagicMock())
-        with patch.object(TestOp, 'create_activity'):
-            self.assertRaises(TypeError, op.call, system=True)

circle/vm/operations.py

@@ -135,12 +135,13 @@ class InstanceOperation(Operation):
                                  "parent activity does not match the user "
                                  "provided as parameter.")
 
-            return parent.create_sub(code_suffix=self.activity_code_suffix,
-                                     readable_name=name,
-                                     resultant_state=self.resultant_state)
+            return parent.create_sub(
+                code_suffix=self.get_activity_code_suffix(),
+                readable_name=name, resultant_state=self.resultant_state)
         else:
             return InstanceActivity.create(
-                code_suffix=self.activity_code_suffix, instance=self.instance,
+                code_suffix=self.get_activity_code_suffix(),
+                instance=self.instance,
                 readable_name=name, user=user,
                 concurrency_check=self.concurrency_check,
                 resultant_state=self.resultant_state)
@@ -154,7 +155,6 @@ class InstanceOperation(Operation):
 class RemoteInstanceOperation(RemoteOperationMixin, InstanceOperation):
 
     remote_queue = ('vm', 'fast')
-    # activity_code_suffix = property(lambda self: self.id or self.task.name)
 
     def _get_remote_queue(self):
         return self.instance.get_remote_queue_name(*self.remote_queue)
@@ -920,7 +920,6 @@ class ChangeStateOperation(InstanceOperation):
 
 @register_operation
 class RedeployOperation(InstanceOperation):
-    activity_code_suffix = 'redeploy'
     id = 'redeploy'
     name = _("redeploy")
     description = _("Change the virtual machine state to NOSTATE "
@@ -974,17 +973,17 @@ class NodeOperation(Operation):
                                  "parent activity does not match the user "
                                  "provided as parameter.")
 
-            return parent.create_sub(code_suffix=self.activity_code_suffix,
+            return parent.create_sub(
+                code_suffix=self.get_activity_code_suffix(),
                 readable_name=name)
         else:
-            return NodeActivity.create(code_suffix=self.activity_code_suffix,
-                                       node=self.node, user=user,
-                                       readable_name=name)
+            return NodeActivity.create(
+                code_suffix=self.get_activity_code_suffix(), node=self.node,
+                user=user, readable_name=name)
 
 
 @register_operation
 class ResetNodeOperation(NodeOperation):
-    activity_code_suffix = 'reset'
     id = 'reset'
     name = _("reset")
     description = _("Disable missing node and redeploy all instances "
@@ -1184,7 +1183,6 @@ class EnsureAgentMixin(object):
 
 @register_operation
 class PasswordResetOperation(EnsureAgentMixin, InstanceOperation):
-    activity_code_suffix = 'password_reset'
     id = 'password_reset'
     name = _("password reset")
     description = _("Generate and set a new login password on the virtual "
@@ -1205,7 +1203,6 @@ class PasswordResetOperation(EnsureAgentMixin, InstanceOperation):
 
 @register_operation
 class MountStoreOperation(EnsureAgentMixin, InstanceOperation):
-    activity_code_suffix = 'mount_store'
     id = 'mount_store'
     name = _("mount store")
     description = _(