-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add synctl warning when no process is stopped and fix restart #6598
Conversation
…hat the process might not be started by synctl (matrix-org#5166) Signed-off-by: Romain Bouyé <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! Sorry for the delay in reviewing this! Looks pretty good overall, but I left a few comments.
…ify comment and add missing type hint
@romainbou Can you fix the lint issues? You can run them locally with: |
@romainbou Do you have plans to take a look back at this PR? It'd be great to finish it up! |
@romainbou Can you merge |
@clokep cool thanks yes indeed. After merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for making those changes! 👍
this seems to have had the side-effect of changing the behaviour of |
it seems that it was a deliberate change, and indeed that was somewhat suggested by #6994. On reflection, I feel like the downsides of that change outweigh the upsides (specifically: it's a surprise to anyone who has ever used |
I should also mention: I don't think that changing the behaviour of An even better fix here is to sort out #4641, but for now the warning text is an improvement. |
…6598) * If an error occurs when stopping a process synctl now logs a warning. * During a restart, synctl will avoid attempting to start Synapse if an error occurs during stopping Synapse.
As suggested in #5166, I added a warning when no process is killed for
synctl stop
andsynctl restart
For
synctl restart
if no process is killed don't attempt to start anyway.Example stopping synapse twice: