Document whether we can restart a stopped service
The docstring is quite minimal: https://github.com/canonical/operator/blob/808fb8f79e2e8d14aa3cb4e36081bd1886d68620/ops/model.py#L2221
In the past we used to:
if self._container.get_service(self._name).is_running():
self._container.restart(self._name)
else:
self._container.start(self._name)
It would be nice to have it explicitly documented if this is still needed or not.
The code has a data race smell on the surface at least.
I wonder if it’s better to try: start() except: restart() or maybe the opposite 🤔
Nice suggestion @dimaqq!
In any case, would be nice to have an updated doc indicating whether we need to guard a restart at all.
If guarding is needed, then perhaps add a kwarg, stopped_ok: bool?
This is not needed, and I believe it hasn't been since the restart command was added, see https://github.com/canonical/pebble/pull/58. Restart doesn't try to stop services that aren't running, but it will stop and start any services that are running. See also the docs about replan vs restart at https://github.com/canonical/pebble?tab=readme-ov-file#updating-and-restarting-services
Interestingly, and somewhat related -- there's a little backwards-compatibility shim in Container.restart for old versions of Pebble (quite old by now) that don't yet have restart, to stop the running services first.
@dimaqq Would you be willing to push up a PR to improve on this in the Container.restart docstring, and the underlying pebble.Client.restart_services docstring? Let's try to keep it brief (ideally one sentence), but indicate 1) that it will force a stop and start if they're already, and 2) won't try to stop services that aren't running, just start them.
This was done in #1320