Repository: juju-solutions/charms.reactive

Issue:
Incorrect handler name logged using not_unless decorator
I'm getting the following in my logs:

2016-01-11 14:58:37 INFO juju-log replication:1: Invoking reactive handler: lib/pypi/charms/reactive/decorators.py:149:_wrapped

It looks like a name isn't being copied from the wrapped function in the not_unless decorator

Discussion:

Patch:
diff --git a/charms/reactive/bus.py b/charms/reactive/bus.py
index 885e498..1bd1364 100644
--- a/charms/reactive/bus.py
+++ b/charms/reactive/bus.py
@@ -170,12 +170,16 @@ def get_state(state, default=None):
 
 
 def _action_id(action):
+    if hasattr(action, '_action_id'):
+        return action._action_id
     return "%s:%s:%s" % (action.__code__.co_filename,
                          action.__code__.co_firstlineno,
                          action.__code__.co_name)
 
 
 def _short_action_id(action):
+    if hasattr(action, '_short_action_id'):
+        return action._short_action_id
     filepath = os.path.relpath(action.__code__.co_filename, hookenv.charm_dir())
     return "%s:%s:%s" % (filepath,
                          action.__code__.co_firstlineno,
diff --git a/charms/reactive/decorators.py b/charms/reactive/decorators.py
index 7918106..7de571c 100644
--- a/charms/reactive/decorators.py
+++ b/charms/reactive/decorators.py
@@ -21,6 +21,7 @@ from charmhelpers.core import hookenv
 from charms.reactive.bus import Handler
 from charms.reactive.bus import get_states
 from charms.reactive.bus import _action_id
+from charms.reactive.bus import _short_action_id
 from charms.reactive.relations import RelationBase
 from charms.reactive.helpers import _hook
 from charms.reactive.helpers import _when_all
@@ -188,19 +189,21 @@ def not_unless(*desired_states):
     This is primarily for informational purposes and as a guard clause.
     """
     def _decorator(func):
+        action_id = _action_id(func)
+        short_action_id = _short_action_id(func)
+
         @wraps(func)
         def _wrapped(*args, **kwargs):
             active_states = get_states()
             missing_states = [state for state in desired_states if state not in active_states]
             if missing_states:
-                func_id = "%s:%s:%s" % (func.__code__.co_filename,
-                                        func.__code__.co_firstlineno,
-                                        func.__code__.co_name)
                 hookenv.log('%s called before state%s: %s' % (
-                    func_id,
+                    short_action_id,
                     's' if len(missing_states) > 1 else '',
                     ', '.join(missing_states)), hookenv.WARNING)
             return func(*args, **kwargs)
+        _wrapped._action_id = action_id
+        _wrapped._short_action_id = short_action_id
         return _wrapped
     return _decorator
 


Test patch:
diff --git a/tests/test_decorators.py b/tests/test_decorators.py
index 4691a30..78733c7 100644
--- a/tests/test_decorators.py
+++ b/tests/test_decorators.py
@@ -236,6 +236,10 @@ class TestReactiveDecorators(unittest.TestCase):
         self.assertEqual(action.call_count, 3)
         assert log_msg(0).endswith('test called before states: foo, bar'), log_msg(0)
         assert log_msg(1).endswith('test called before state: bar'), log_msg(1)
+        self.assertIn('tests/test_decorators.py:', reactive.bus._action_id(test))
+        self.assertIn(':test', reactive.bus._action_id(test))
+        self.assertIn('tests/test_decorators.py:', reactive.bus._short_action_id(test))
+        self.assertIn(':test', reactive.bus._short_action_id(test))
 
     def test_only_once(self):
         calls = []