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

Extend pueue wait to wait for specific Done result states. #434

Closed
mjpieters opened this issue May 12, 2023 · 9 comments
Closed

Extend pueue wait to wait for specific Done result states. #434

mjpieters opened this issue May 12, 2023 · 9 comments
Labels
s: Client This issue touches the pueue client t: Feature A new feature that needs implementation

Comments

@mjpieters
Copy link
Contributor

A detailed description of the feature you would like to see added.

When waiting for tasks with pueue wait, the command exits when the tasks reach the Done state (by default). However, Done tasks have a result state, one of:

  • Success, completed succesfully
  • Failed, the command ended with a non-zero exit code.
  • Failed to Spawn, something prevented the command from running, like a typo
  • Killed, the command was explicitly killed
  • Errored, something went wrong at the pueue level, like an I/O error
  • DependencyFailed, the command never started because a dependency task didn't complete succesfully

pueue wait exits with 0 for any of these states.

There should be an option to either continue waiting on a task and / or exit with a non-zero exit code, when the waited-upon task(s) reach Done but their result state is not Success (default) or a configurable set of result states.

Explain your usecase of the requested feature

I wanted to run a command once all tasks were done, a command that opens a (text-based) console that lets you monitor its state, and so it wasn't really as suited for running under pueue.

However, some of the tasks failed, the command ran anyway, and I had to clean up the fallout as it moved things around that were not ready to be moved.

Had pueue wait exited with an error code or simply waited until I manually had restarted the failed tasks, it would have been a lot better here. A non-zero exit would have exited my setup (I used pueue wait ... && next-command), for example.

Alternatives

I could have used pueue add --after [taskids] [dependent command] but that can't wait for a whole group to complete and requires that [dependent command] is suitable to be controlled by pueue.

I also could have set the daemon pause_group_on_failure configuration option, but that'd have paused the group, which I don't really want; the other commands in the group can complete successfully even if one of the tasks fails, and pausing delays things considerably as I now have to wait on the paused tasks when I am recovering the failed tasks, too.

Right now, the only option I see is to create a dummy task that depends on the specific task ids, and have pueue wait for that command to complete. Pueue will not start a dependent task unless all dependencies have reached a Done(Succes) state. This works as long as you don't add more tasks to the group (there is no way to edit a task to add more dependencies).

Additional context

I'd pefer to have pueue wait wait when tasks are done but were not successful, but I can definitely also see the case for exiting with a non-zero state. Perhaps both should be an option?

@mjpieters mjpieters added the t: Feature A new feature that needs implementation label May 12, 2023
@Nukesor
Copy link
Owner

Nukesor commented May 16, 2023

Sorry for any inconveniences this behavior has caused 😅

I like the option to exit with 1 if a task failed. Seems like a reasonable default.
I'm not sure about making the other one the default, though. The command would wait endlessly, which seems a bit odd to me. Though I guess, since we're printing the status changes, it should be pretty obvious why the command hangs.

The thing is, that this seems to be used in non user-facing scripts quite a bit, where an endlessly waiting process might cause some problems.

@mjpieters
Copy link
Contributor Author

mjpieters commented May 16, 2023

The thing is, that this seems to be used in non user-facing scripts quite a bit.

Which is what the --quiet switch is for, right?

Anyway, I'm open to exiting with a non-zero status being the default on dependency failure, we can always see if others think there should be a 'wait foreever' option.

For my usecase, I could just use a while loop; in untested psuedo-bash:

while !pueue wait ...; do
    echo 'Please check for failures and restart task(s) as necessary'
    sleep 5
done
# do the thing that needed waiting

@Nukesor
Copy link
Owner

Nukesor commented May 16, 2023

I just noticed, that my previous answer was a bit unclear and a lot of what I actually wanted to say got lost in translation. I rephrased it to hopefully better reflect my thought process^^. I'll also go into detail below.

The purpose of --quiet is for not printing any actual output, which isn't a problem anyway.

I had two main concerns:

  1. That users run something like pueue wait and they won't notice that the task failed in the background. But this won't actually a problem, since wait always prints status changes, which will implicitly notify the user about any failed tasks.

  2. Whatever default behavior we decide to use, it will be a breaking change.

    • If we wait on failed tasks, users that're already using that command with --quiet might have endlessly waiting scripts without knowing why.
    • If we're exiting with 1 by default, their scripts might break, but at least they see the error right away.

Intuitively, I would say that the exit 1 should be the default, as is this is the behavior I would expect :D

I'm generally in favor of a --wait-on-fail, especially since this is causing you UI issues.
That while loop of yours would also not work if there're no longer any tasks in the watched group, as pueue wait will then exit with 0 right away.

I got one more question:
It seems like you're doing a mix of scripting stuff in combination and using pueue as a scheduler for longer running tasks. Could you use dependencies to achieve the same behavior? Or would this be unfeasible, since you're mostly waiting for whole groups rather than individual tasks?

@mjpieters
Copy link
Contributor Author

2. Whatever default behavior we decide to use, it will be a breaking change.

I actually was envisioning that an additional switch was added that lets you select what result states to wait for; if not given then the current behaviour (any result state is okay) would be the default.

E.g. pueue wait --result Success ... waits for Done() and exits with 0 if the result is Success, 1 otherwise, but pueue wait ... with no --result switch exits with 0 when Done() is reached, whatever the result.

And yes, you are correct, the loop will fail right now, but in a scenario where pueue wait exits with a non-zero status code for failed tasks those tasks would still be there and the next pueue wait --result Success --group .... call would exit with a non-zero state again, as the indicated tasks are done but have a failed result, until I run some form of pueue restart -i ... to remedy the issues (perhaps with a suitable -e action to repair a broken command).

Dependencies don't always work, not when I am waiting for a whole group, where I am not sure up front how many tasks will be added. I'm not mostly using groups, but when I do dependencies don't work.

@mjpieters
Copy link
Contributor Author

I just realised that I could have a dedicated dependent dummy tasks group, say dependency_dummies, use pueue wait --group dependency_dummies, then add a single task to that group for every task in the actual group I want to wait on with pueue add --group dependency_dummies --after ... -- echo 'Dependency completed successfully'

Still, that's a bit tedious and feels like a hack.

@mjpieters
Copy link
Contributor Author

Ah, except, that doesn't work either, because dependent tasks are moved to Done(DependencyFailed) and so pueue wait will exit once all tasks in the group have reached some Done() state.

@Nukesor Nukesor added the s: Client This issue touches the pueue client label May 22, 2023
@Nukesor
Copy link
Owner

Nukesor commented Jun 6, 2023

Ok.

So the plan would be to

  • Add a new wait status Success
  • If one waits for that status, all tasks one waits for have to have the status Success.
  • If at least one task that's being waited for fails, the task exits immediately with 1
  • When all tasks succeed, the command exits with a 0

I like that. It's straight forward and backward compatible.

I'm not sure yet, if we should add a new parameter (--result) or if we should just add it as a "status".
From a UX side, it feels more practical to have all of those conditions in a single enum.

Waiting for Failure?

I thought about adding the same thing for fail, but I couldn't come up with a user story where one would want to wait for all tasks to fail. Knowing whether a single task failed is covered by the Success status.

@mjpieters
Copy link
Contributor Author

Yes, I can't see a good use case to wait for a specific error result either. And that then leaves just the one result option, at which point it does make sense to just add Success to the WaitTargetStatus enum.

@Nukesor
Copy link
Owner

Nukesor commented Jun 6, 2023

I implemented and tested the behavior in the linked PR :)

I'll be gone for a few days, but I'm planning to release a new version next week Monday/Tuesday.

@Nukesor Nukesor closed this as completed Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: Client This issue touches the pueue client t: Feature A new feature that needs implementation
Projects
None yet
Development

No branches or pull requests

2 participants