flyingcircusio / batou_ext

A library of batou extensions.
Other
4 stars 1 forks source link

OCI: optional monitoring and rebuild #171

Closed zagy closed 3 months ago

zagy commented 5 months ago

Requires #180

PhilTaken commented 4 months ago

Why is the _need_restart required here? would it not be easier to just not raise an UpdateNeeded?

zagy commented 4 months ago

Why is the _need_restart required here? would it not be easier to just not raise an UpdateNeeded?

Because we don't always want a restart but only when a) the unit is actually there and b) if there was no rebuild. If there was a rebuild because the config changed the container will already be restarted by the rebuild, won't it?

PhilTaken commented 4 months ago

a) the unit is actually there

good point although this should only be an issue if rebuild == False

If there was a rebuild because the config changed the container will already be restarted by the rebuild

not necessarily true, see my comment above. likewise for the registry password.

I would argue it's simpler to change the update() logic to restart the service unless the nix file was changed since that would imply a restart via rebuild

zagy commented 4 months ago

I've reworked this change. @PhilTaken please review again.

zagy commented 4 months ago

Yeah, well. You cannot have both IMHO. @frlan are you okay with that?