Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stopping multiple processes should not fail fast #199

Closed
zmitchell opened this issue Jul 11, 2024 · 4 comments
Closed

Stopping multiple processes should not fail fast #199

zmitchell opened this issue Jul 11, 2024 · 4 comments
Labels
done Done, awaiting release enhancement New feature or request

Comments

@zmitchell
Copy link

Feature Request

When stopping more than one process, all processes that can be stopped should be stopped, regardless of whether there is an error stopping another process.

Use Case:

For the sake of discussion, assume "bad" means a process either never existed or is no longer running.
Right now if you attempt to stop multiple processes and one of them is bad, the processes that actually get stopped depend on the order in which the processes are passed to process-compose. This is a surprising behavior, and we should aim for the least surprising behavior.

Proposed Change:

When stopping multiple processes, attempt to stop all of them before handling any errors. Some of the errors a user will encounter can be caught before stopping any processes e.g. you can check whether a process exists or whether it's running before trying to stop it, but there may be some errors that can only be caught while trying to stop the process. In that case you can't "unstop" the process and undo the the user's requested operation. In that case it's best to attempt to complete as much of it as possible and inform them of the error.

Who Benefits From The Change(s)?

Everyone

Alternative Approaches

@F1bonacc1
Copy link
Owner

Hi @zmitchell,

Let's say there are 2 processes good and bad. What do you think should be the behavior in terms of error reporting:

  1. What should be the exit code?
  2. What should be printed as an error (if at all)? Will it scale to 20 bad processes?
  3. Today process compose returns the list of the stopped processes. Should it return 2 lists (stopped, failed + reason)? Obviously, that will be an API breaking change.

@zmitchell
Copy link
Author

I think you should not run the command at all if you can tell that bad is not a process that was in the user's config e.g. you should check the process names before shutting anything down.

@zmitchell
Copy link
Author

That eliminates a lot of the ambiguity that prompted this issue.

Regardless, to answer your questions:

  1. I would expect this to be nonzero because we couldn't complete the entirety of the user's request.

  2. It depends on whether you want full or summarized output. My personal preference is to default to full output, which would look like

:green-check: Successfully stopped proc1
:green-check: Successfully stopped proc2
:red-x: Failed to stop proc3
:green-check: Successfully stopped proc4
...

If you wanted more concise output you could do:

Successfully stopped some processes but encountered failures for: proc1, proc2, ...

or

Successfully stopped some processes but encountered failures for:
- proc1
- proc2
...

I think if I have 20 bad processes, that's something I want to know about, and furthermore if 19 of them are bad for one reason and 1 of them is bad for another reason, that's also something I want to know about.

  1. I don't actually see where that list of stopped processes is returned. If I have two processes sleeping1 and sleeping2, then try to stop those and a bad process, this is what I see:
24-08-05 12:00:38.292 FTL failed to stop processes [sleeping1 sleeping2 sleeping3] error="process sleeping3 is not running"

@F1bonacc1 F1bonacc1 added enhancement New feature or request done Done, awaiting release and removed need more info labels Aug 9, 2024
@F1bonacc1
Copy link
Owner

Added in v1.18.0

Cheers ✌🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done Done, awaiting release enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants