Skip to content

Commit

Permalink
feat: add the action's ID to the ActionEvent object (#1124)
Browse files Browse the repository at this point in the history
Add an .id property to actions, with the content of the
JUJU_ACTION_UUID environment variable.
  • Loading branch information
tonyandrewmeyer committed Feb 14, 2024
1 parent 2f304d3 commit b13ee4c
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 20 deletions.
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 2.11.0

* Added `ActionEvent.id`, exposing the JUJU_ACTION_UUID environment variable.

# 2.10.0

* Added support for Pebble Notices (`PebbleCustomNoticeEvent`, `get_notices`, and so on)
Expand Down
15 changes: 15 additions & 0 deletions ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,16 @@ class ActionEvent(EventBase):
:meth:`log`.
"""

id: str = ""
"""The Juju ID of the action invocation."""

params: Dict[str, Any]
"""The parameters passed to the action."""

def __init__(self, handle: 'Handle', id: Optional[str] = None):
super().__init__(handle)
self.id = id # type: ignore (for backwards compatibility)

def defer(self) -> NoReturn:
"""Action events are not deferrable like other events.
Expand All @@ -144,10 +151,18 @@ def restore(self, snapshot: Dict[str, Any]):
Not meant to be called directly by charm code.
"""
self.id = cast(str, snapshot['id'])
# Params are loaded at restore rather than __init__ because
# the model is not available in __init__.
self.params = self.framework.model._backend.action_get()

def snapshot(self) -> Dict[str, Any]:
"""Used by the framework to serialize the event to disk.
Not meant to be called by charm code.
"""
return {'id': self.id}

def set_results(self, results: Dict[str, Any]):
"""Report the result of the action.
Expand Down
4 changes: 2 additions & 2 deletions ops/framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -1057,10 +1057,10 @@ class BoundStoredState:
if TYPE_CHECKING:
# to help the type checker and IDEs:
@property
def _data(self) -> StoredStateData: ... # noqa, type: ignore
def _data(self) -> StoredStateData: ... # type: ignore

@property
def _attr_name(self) -> str: ... # noqa, type: ignore
def _attr_name(self) -> str: ... # type: ignore

def __init__(self, parent: Object, attr_name: str):
parent.framework.register_type(StoredStateData, parent)
Expand Down
3 changes: 3 additions & 0 deletions ops/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ def _get_event_args(charm: 'ops.charm.CharmBase',
storage = cast(Union[ops.storage.JujuStorage, ops.storage.SQLiteStorage], storage)
storage.location = storage_location # type: ignore
return [storage], {}
elif issubclass(event_type, ops.charm.ActionEvent):
args: List[Any] = [os.environ['JUJU_ACTION_UUID']]
return args, {}
elif issubclass(event_type, ops.charm.RelationEvent):
relation_name = os.environ['JUJU_RELATION']
relation_id = _get_juju_relation_id()
Expand Down
4 changes: 3 additions & 1 deletion ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ def __init__(
self._unit_name: str = f"{self._meta.name}/0"
self._hooks_enabled: bool = True
self._relation_id_counter: int = 0
self._action_id_counter: int = 0
config_ = self._get_config(config)
self._backend = _TestingModelBackend(self._unit_name, self._meta, config_)
self._model = model.Model(self._meta, self._backend)
Expand Down Expand Up @@ -1883,7 +1884,8 @@ def run_action(self, action_name: str,
action_under_test = _RunningAction(action_name, ActionOutput([], {}), params)
handler = getattr(self.charm.on, f"{action_name.replace('-', '_')}_action")
self._backend._running_action = action_under_test
handler.emit()
self._action_id_counter += 1
handler.emit(str(self._action_id_counter))
self._backend._running_action = None
if action_under_test.failure_message is not None:
raise ActionFailed(
Expand Down
11 changes: 6 additions & 5 deletions test/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ def __init__(self, *args: typing.Any):
def _on_foo_bar_action(self, event: ops.ActionEvent):
self.seen_action_params = event.params
event.log('test-log')
event.set_results({'res': 'val with spaces'})
event.set_results({'res': 'val with spaces', 'id': event.id})
event.fail('test-fail')

def _on_start_action(self, event: ops.ActionEvent):
Expand All @@ -477,12 +477,13 @@ def _on_start_action(self, event: ops.ActionEvent):
self.assertIn('foo_bar_action', events)
self.assertIn('start_action', events)

charm.on.foo_bar_action.emit()
action_id = "1234"
charm.on.foo_bar_action.emit(id=action_id)
self.assertEqual(charm.seen_action_params, {"foo-name": "name", "silent": True})
self.assertEqual(fake_script_calls(self), [
['action-get', '--format=json'],
['action-log', "test-log"],
['action-set', "res=val with spaces"],
['action-set', "res=val with spaces", f"id={action_id}"],
['action-fail', "test-fail"],
])

Expand Down Expand Up @@ -511,7 +512,7 @@ def _on_foo_bar_action(self, event: ops.ActionEvent):
charm.res = bad_res

with self.assertRaises(ValueError):
charm.on.foo_bar_action.emit()
charm.on.foo_bar_action.emit(id='1')

def _test_action_event_defer_fails(self, cmd_type: str):

Expand All @@ -532,7 +533,7 @@ def _on_start_action(self, event: ops.ActionEvent):
charm = MyCharm(framework)

with self.assertRaises(RuntimeError):
charm.on.start_action.emit()
charm.on.start_action.emit(id='2')

def test_action_event_defer_fails(self):
self._test_action_event_defer_fails('action')
Expand Down
4 changes: 2 additions & 2 deletions test/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ class CustomEvents(ops.ObjectEvents):

with patch('sys.stderr', new_callable=io.StringIO):
with patch('pdb.runcall') as mock:
publisher.foobar_action.emit()
publisher.foobar_action.emit(id='1')

self.assertEqual(mock.call_count, 1)
self.assertFalse(observer.called)
Expand All @@ -1833,7 +1833,7 @@ class CustomEvents(ops.ObjectEvents):

with patch('sys.stderr', new_callable=io.StringIO):
with patch('pdb.runcall') as mock:
publisher.foobar_action.emit()
publisher.foobar_action.emit(id='2')

self.assertEqual(mock.call_count, 1)
self.assertFalse(observer.called)
Expand Down
27 changes: 17 additions & 10 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,13 @@ def test_multiple_events_handled(self):
'departing_unit': 'remote/42'},
), (
EventSpec(ops.ActionEvent, 'start_action',
env_var='JUJU_ACTION_NAME'),
env_var='JUJU_ACTION_NAME',
set_in_env={'JUJU_ACTION_UUID': '1'}),
{},
), (
EventSpec(ops.ActionEvent, 'foo_bar_action',
env_var='JUJU_ACTION_NAME'),
env_var='JUJU_ACTION_NAME',
set_in_env={'JUJU_ACTION_UUID': '2'}),
{},
), (
EventSpec(ops.PebbleReadyEvent, 'test_pebble_ready',
Expand Down Expand Up @@ -726,19 +728,20 @@ def test_logger(self):
fake_script(typing.cast(unittest.TestCase, self), 'action-get', "echo '{}'")

test_cases = [(
EventSpec(ops.ActionEvent, 'log_critical_action', env_var='JUJU_ACTION_NAME'),
EventSpec(ops.ActionEvent, 'log_critical_action', env_var='JUJU_ACTION_NAME',
set_in_env={'JUJU_ACTION_UUID': '1'}),
['juju-log', '--log-level', 'CRITICAL', '--', 'super critical'],
), (
EventSpec(ops.ActionEvent, 'log_error_action',
env_var='JUJU_ACTION_NAME'),
env_var='JUJU_ACTION_NAME', set_in_env={'JUJU_ACTION_UUID': '2'}),
['juju-log', '--log-level', 'ERROR', '--', 'grave error'],
), (
EventSpec(ops.ActionEvent, 'log_warning_action',
env_var='JUJU_ACTION_NAME'),
env_var='JUJU_ACTION_NAME', set_in_env={'JUJU_ACTION_UUID': '3'}),
['juju-log', '--log-level', 'WARNING', '--', 'wise warning'],
), (
EventSpec(ops.ActionEvent, 'log_info_action',
env_var='JUJU_ACTION_NAME'),
env_var='JUJU_ACTION_NAME', set_in_env={'JUJU_ACTION_UUID': '4'}),
['juju-log', '--log-level', 'INFO', '--', 'useful info'],
)]

Expand Down Expand Up @@ -779,7 +782,8 @@ def test_sets_model_name(self):
state = self._simulate_event(EventSpec(
ops.ActionEvent, 'get_model_name_action',
env_var='JUJU_ACTION_NAME',
model_name='test-model-name'))
model_name='test-model-name',
set_in_env={'JUJU_ACTION_UUID': '1'}))
assert isinstance(state, ops.BoundStoredState)
self.assertEqual(state._on_get_model_name_action, ['test-model-name'])

Expand All @@ -791,7 +795,8 @@ def test_has_valid_status(self):
"""echo '{"status": "unknown", "message": ""}'""")
state = self._simulate_event(EventSpec(
ops.ActionEvent, 'get_status_action',
env_var='JUJU_ACTION_NAME'))
env_var='JUJU_ACTION_NAME',
set_in_env={'JUJU_ACTION_UUID': '1'}))
assert isinstance(state, ops.BoundStoredState)
self.assertEqual(state.status_name, 'unknown')
self.assertEqual(state.status_message, '')
Expand All @@ -801,7 +806,8 @@ def test_has_valid_status(self):
"""echo '{"status": "blocked", "message": "help meeee"}'""")
state = self._simulate_event(EventSpec(
ops.ActionEvent, 'get_status_action',
env_var='JUJU_ACTION_NAME'))
env_var='JUJU_ACTION_NAME',
set_in_env={'JUJU_ACTION_UUID': '1'}))
assert isinstance(state, ops.BoundStoredState)
self.assertEqual(state.status_name, 'blocked')
self.assertEqual(state.status_message, 'help meeee')
Expand Down Expand Up @@ -1169,7 +1175,8 @@ def test_crash_action(self):
with self.assertRaises(subprocess.CalledProcessError):
self._simulate_event(EventSpec(
ops.ActionEvent, 'keyerror_action',
env_var='JUJU_ACTION_NAME'))
env_var='JUJU_ACTION_NAME',
set_in_env={'JUJU_ACTION_UUID': '1'}))
self.stderr.seek(0)
stderr = self.stderr.read()
self.assertIn('KeyError', stderr)
Expand Down
1 change: 1 addition & 0 deletions test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -5445,6 +5445,7 @@ def __init__(self, framework: ops.Framework):
def _on_simple_action(self, event: ops.ActionEvent):
"""An action that doesn't generate logs, have any results, or fail."""
self.simple_was_called = True
assert isinstance(event.id, str)

def _on_fail_action(self, event: ops.ActionEvent):
event.fail("this will be ignored")
Expand Down

0 comments on commit b13ee4c

Please sign in to comment.