Reconsider the "FrameworkAdapter" infrastructure

[facundobatista]

It's a layer above many things. A quite lightweight one, though, so there's not a lot of work hidden in that layer, but as it's a layer above several other things, to where each property belongs is not easily discoverable.

Most of the FrameworkAdapter attributes are from the Model, so they can always be accessed doing self.framework.model in the charm. But note that the charm already has shortcuts like app or unit (check the docs here).

So, for example you're currently doing...

juju_app = fw_adapter.get_app_name()

...and it's shorter and more accurate to do...

juju_app = self.framework.app.name

It's even absurd to have the separation layer when doing exactly the same thing, forcing you to write...

            self.fw_adapter.observe(event, delegator)

...when it's just doing exactly the same that you would be doing by...

            self.framework.observe(event, delegator)

This in general lowers a lot the learnability of this code. People getting here needs to learn this new abstraction layer instead of just using the Framework, as they are used to do.

An example of that is the developer needing to learn/understand what you're doing in...

    if not fw_adapter.am_i_leader():

...when it's simple (as long you know the Framework, which is a knowledge it's expected to have) to understand what the code does in...

    if not self.unit.is_leader():

It's fine IMO to have some layers/adapters as you have (for building the build_juju_pod_spec, for example).

But in the case of the Framework, which is by design tightly coupled with the Charm (and the Model), I would totally recommend to not have this adapter.

[relaxdiego]

Thanks for the feedback @facundobatista! I have tried to remove FrameworkAdapter from some parts in a topic branch as you suggested. What's missing is my ability to assert if the SUT did exactly as designed. Please see: 88006ba

More specifically:

charm-k8s-grafana/test/charm_test.py

Lines 43 to 53 in 88006ba

	def test__on_start__it_sets_the_pod_spec_and_unit_status(self):
	# Setup
	harness = Harness(charm.Charm)
	harness.begin()
	# Exercise
	harness.charm.on.start.emit()
	# Assert
	# How to check if framework.model.pod.set_spec() and
	# framework.model.unit.status were called/set correctly?

[relaxdiego]

I should also reference the comment that I added on an issue in the Operator Framework which is closely related to the question that I have above: canonical/operator#282 (comment)

[relaxdiego]

To be absolutely clear, I'm not using the above as a means to justify the existence of the FrameworkAdapter. Honestly, I would love to not have to maintain that if I can avoid it. But to let go of it, I must have a way to interrogate some other object regarding the side effects that I expect my SUT to cause at certain events.

[facundobatista]

Maybe @jameinel could help us here?

[jameinel]

So if you have to have the individual calls, you could always patch harness.charm.framework to be a mock which would intercept and record the individual calls.
I think it makes more sense to test it as "if the pod status is this, show that it sets the unit status to that" and test those as individual steps instead of as a sequence of events.