Skip to content

Commit

Permalink
Minor refactoring to make it easier to subclass ops._main._Manager.
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyandrewmeyer committed Nov 27, 2024
1 parent f9a621d commit eb441f3
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 31 deletions.
65 changes: 39 additions & 26 deletions ops/_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,28 +112,6 @@ def _setup_event_links(charm_dir: Path, charm: 'ops.charm.CharmBase', juju_conte
_create_event_link(charm, bound_event, link_to)


def _emit_charm_event(charm: 'ops.charm.CharmBase', event_name: str, juju_context: _JujuContext):
"""Emits a charm event based on a Juju event name.
Args:
charm: A charm instance to emit an event from.
event_name: A Juju event name to emit on a charm.
juju_context: An instance of the _JujuContext class.
"""
event_to_emit = None
try:
event_to_emit = getattr(charm.on, event_name)
except AttributeError:
logger.debug('Event %s not defined for %s.', event_name, charm)

# If the event is not supported by the charm implementation, do
# not error out or try to emit it. This is to support rollbacks.
if event_to_emit is not None:
args, kwargs = _get_event_args(charm, event_to_emit, juju_context)
logger.debug('Emitting Juju event %s.', event_name)
event_to_emit.emit(*args, **kwargs)


def _get_event_args(
charm: 'ops.charm.CharmBase',
bound_event: 'ops.framework.BoundEvent',
Expand Down Expand Up @@ -401,8 +379,11 @@ def __init__(
model_backend: Optional[ops.model._ModelBackend] = None,
use_juju_for_storage: Optional[bool] = None,
charm_state_path: str = CHARM_STATE_FILE,
juju_context: Optional[_JujuContext] = None,
):
self._juju_context = _JujuContext.from_dict(os.environ)
if juju_context is None:
juju_context = _JujuContext.from_dict(os.environ)
self._juju_context = juju_context
self._charm_state_path = charm_state_path
self._charm_class = charm_class
if model_backend is None:
Expand All @@ -413,7 +394,7 @@ def __init__(
self._setup_root_logging()

self._charm_root = self._juju_context.charm_dir
self._charm_meta = CharmMeta.from_charm_root(self._charm_root)
self._charm_meta = self._load_charm_meta()
self._use_juju_for_storage = use_juju_for_storage

# Set up dispatcher, framework and charm objects.
Expand All @@ -423,6 +404,9 @@ def __init__(
self.framework = self._make_framework(self.dispatcher)
self.charm = self._make_charm(self.framework, self.dispatcher)

def _load_charm_meta(self):
return CharmMeta.from_charm_root(self._charm_root)

def _make_charm(self, framework: 'ops.framework.Framework', dispatcher: _Dispatcher):
charm = self._charm_class(framework)
dispatcher.ensure_event_links(charm)
Expand Down Expand Up @@ -482,7 +466,7 @@ def _make_framework(self, dispatcher: _Dispatcher):
# If we are in a RelationBroken event, we want to know which relation is
# broken within the model, not only in the event's `.relation` attribute.

if self._juju_context.dispatch_path.endswith('-relation-broken'):
if self._juju_context.dispatch_path.endswith(('-relation-broken', '_relation_broken')):
broken_relation_id = self._juju_context.relation_id
else:
broken_relation_id = None
Expand Down Expand Up @@ -515,19 +499,48 @@ def _emit(self):
self.framework.reemit()

# Emit the Juju event.
_emit_charm_event(self.charm, self.dispatcher.event_name, self._juju_context)
self._emit_charm_event(self.dispatcher.event_name)
# Emit collect-status events.
ops.charm._evaluate_status(self.charm)

def _get_event_to_emit(self, event_name: str) -> Optional[ops.framework.BoundEvent]:
try:
return getattr(self.charm.on, event_name)
except AttributeError:
logger.debug('Event %s not defined for %s.', event_name, self.charm)
# If the event is not defined, return None.

def _emit_charm_event(self, event_name: str):
"""Emits a charm event based on a Juju event name.
Args:
charm: A charm instance to emit an event from.
event_name: A Juju event name to emit on a charm.
juju_context: An instance of the _JujuContext class.
"""
event_to_emit = self._get_event_to_emit(event_name)

# If the event is not supported by the charm implementation, do
# not error out or try to emit it. This is to support rollbacks.
if event_to_emit is not None:
args, kwargs = _get_event_args(self.charm, event_to_emit, self._juju_context)
logger.debug('Emitting Juju event %s.', event_name)
event_to_emit.emit(*args, **kwargs)

def _commit(self):
"""Commit the framework and gracefully teardown."""
self.framework.commit()

def _close(self):
"""Perform any necessary cleanup before the framework is closed."""
# Provided for child classes - nothing needs to be done in the base.

def run(self):
"""Emit and then commit the framework."""
try:
self._emit()
self._commit()
self._close()
finally:
self.framework.close()

Expand Down
2 changes: 1 addition & 1 deletion ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@
# monkeypatch it in, so that the ops.testing.ActionFailed object is the
# one that we expect, even if people are mixing Harness and Scenario.
# https://github.com/canonical/ops-scenario/issues/201
import scenario._runtime as _runtime
import scenario.context as _context
import scenario.runtime as _runtime

_context.ActionFailed = ActionFailed # type: ignore[reportPrivateImportUsage]
_runtime.ActionFailed = ActionFailed # type: ignore[reportPrivateImportUsage]
Expand Down
6 changes: 3 additions & 3 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def __init__(


@patch('ops._main.setup_root_logging', new=lambda *a, **kw: None) # type: ignore
@patch('ops._main._emit_charm_event', new=lambda *a, **kw: None) # type: ignore
@patch('ops._main._Manager._emit_charm_event', new=lambda *a, **kw: None) # type: ignore
@patch('ops.charm._evaluate_status', new=lambda *a, **kw: None) # type: ignore
class TestCharmInit:
@patch('sys.stderr', new_callable=io.StringIO)
Expand Down Expand Up @@ -235,11 +235,11 @@ def __init__(self, framework: ops.Framework):
dispatch.chmod(0o755)

with patch.dict(os.environ, fake_environ):
with patch('ops._main._emit_charm_event') as mock_charm_event:
with patch('ops._main._Manager._emit_charm_event') as mock_charm_event:
ops.main(MyCharm)

assert mock_charm_event.call_count == 1
return mock_charm_event.call_args[0][1]
return mock_charm_event.call_args[0][0]

def test_most_legacy(self):
"""Without dispatch, sys.argv[0] is used."""
Expand Down
2 changes: 1 addition & 1 deletion test/test_main_invocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
@pytest.fixture
def charm_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
monkeypatch.setattr('sys.argv', ('hooks/install',))
monkeypatch.setattr('ops._main._emit_charm_event', Mock())
monkeypatch.setattr('ops._main._Manager._emit_charm_event', Mock())
monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock())
monkeypatch.setattr('ops.charm._evaluate_status', Mock())
monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path))
Expand Down

0 comments on commit eb441f3

Please sign in to comment.