Lazy dependent Object initialization

[PietroPasotti]

So here's a swanky Monday idea:

TLDR: proposing a new pattern: put all Objects depending on charm in a property:

• lazy eval makes it less error-prone when it comes to interdependencies
• reevaluation on each access means we are less reliant on initialization order

Now:

class MyCharm(CharmBase):
    def __init__(self, f):
        self.foo = FooLib(self, *params)
        self.bar = BarLib(self, *params)
        f.observe(self.foo.on_event, self._on_foo_event)
        f.observe(self.bar.on_event, self._on_bar_event)

Issues with this pattern:

• sometimes initialization order of FooLib, BarLib matters, because either one may make changes to the charm state (e.g. write data to a relation, set status), and the other one might depend on it. We've had issues with this in the past.

Proposal:

class MyCharm(CharmBase):
    def __init__(self, f):
        f.observe(self.foo.on_event, self._on_foo_event)
        f.observe(self.bar.on_event, self._on_bar_event)
    
   @property
   def foo(self):
        return FooLib(self, *params)
        
   @property
   def bar(self):
        return BarLib(self, *params)

What this gains us:

• Lazy class initialization: code is ran as late as possible, which means lower chances of odd races
• Class reinitialized on each access: so long as init state-mutating logic is idempotent, we can reinit multiple times without having to re-set instance attrs
• it is more obvious that the objects are throwaway

Questions:

• Will the GC play nice with framework-registered observers on, say the FooLib instance? If FooLib sets an observer on some method on itself, will the framework break when it tries to reemit it, or will the reference keep the object alive long enough for the event to be fired on its method?

[simskij]

The best input I can offer at this point is that it definitely is interesting!

[PietroPasotti]

I even thought about using a data descriptor pattern, but that'd make it more complex to pass parameters to them.

Option 3 would be:

class MyCharm(CharmBase):
   foo = FooLib(*static_params)
   bar = BarLib(*static_params)
           
    def __init__(self, f):
        f.observe(self.foo.on_event, self._on_foo_event)
        f.observe(self.bar.on_event, self._on_bar_event)
        self.foo.setup(*dynamic_params)
        self.bar.setup(*dynamic_params)  

[sed-i]

That's a very interesting constraint.
Afaict, this would not eliminate the ordering issue we had with ingress: when we emit custom events (e.g. on.ingress_ready), then remote_write URL could still be set to the fqdn instead of ingress URL.

[Abuelodelanada]

I like it @PietroPasotti, it's a step in the right direction.
(right direction: take the responsibility for instantiating its dependencies away from charm)

[benhoyt]

This piques my interest, but I think I'm missing a chunk of context:

1. I'm unclear what problem this is trying to solve. Can you give a simplified example of a concrete FooLib and BarLib where initialization order matters or the __init__ makes change to the charm state?
2. I'm also not quite sure what you're proposing. Is this just a pattern that charms could follow, or are you suggesting changes to ops itself?

Did you intend for def foo and def bar to be properties?

Yeah, we'd have to check whether the GC would do what we expect here. There's some usage of weak references in the observe code, so I'm not sure at a glance.

[sed-i]

Can you give a simplified example of a concrete FooLib and BarLib

Not so simplified, but:

• AlertmanagerProvider and IngressPerAppRequirer. In alertmanager/76 we ended up using dep. inj. (callable).
• MetricsEndpointProvider and IngressPerUnitRequirer. In prometheus/349 we ended up relying on implicit code ordering (related: traefik/84, 107).
• PrometheusRemoteWriteProvider and IngressPerUnitRequirer. Similar to the above.
• Basically anything that depends on ingress.

[PietroPasotti]

@benhoyt I'm proposing a new pattern. Option 3 also involves adding to ops some way of facilitating creating data-descriptor-based-Objects, but for the rest I'm thinking purely about patterns.
And yeah, depending on whether the GC plays nice we will also need changes to ops.
Also, supposing the various FooLibs queue their observers on __init__, we'd also have to prevent them being initialized multiple times --> duplicate observers being registered. But yeah, that's implementation detail.

Yes, foo and bar were properties. Fixed.

[benhoyt]

This is an interesting discussion, but charmers can implement this now in charms if they want. Also, I'm unsure whether this solves the ordering issue -- it doesn't seem like it would. Feel free to re-open if there's a more concrete proposal or further thoughts.