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

Feature Request: Allow top_spinner to continue even if the tasks below is in error. #45

Open
dipanm opened this issue Jan 14, 2021 · 4 comments · May be fixed by #53
Open

Feature Request: Allow top_spinner to continue even if the tasks below is in error. #45

dipanm opened this issue Jan 14, 2021 · 4 comments · May be fixed by #53

Comments

@dipanm
Copy link

dipanm commented Jan 14, 2021

I am using a TTY::Spinner::Multi

It has multiple child spinners. However, even if a single child fails, the top-level spinner actually stops. The child spinners are async with auto_spin.

As per documentation:
Finally, you need to stop each spinner manually, in our case we mark the second spinner as failure which in turn will stop the top-level multi spinner automatically and mark it as a failure

This is limiting in my use case. I use fields at top level spinner to indicate some aggregate score. Also, in our context, if one of the spinners (and corresponding tasks) fails, a new task is added with the corresponding spinner. So a given spinner failure doesn't imply failure of the top-level spinner and it should not stop. We can explicitly stop the top-level spinner when the full task is done.

In general, there can be many scenarios, where a top-level spinner needs to continue till at least one child is running or maybe even after all children are running. The current behavior could be one of the options or default behavior but should not be a forced choice.

@piotrmurach
Copy link
Owner

That sounds reasonable to me. Do you have a suggestion from the API standpoint how this could be resolved? Any code snippets would be welcome.

@frsantos
Copy link

I too would need stopping the top spinner manually, since I can add spinners to the multi after the previous ones have succeeded or failed.

I think that adding a flag to the stop/success/error would be cumbersome, as we don't know the real state of the multi until we manually stop it. A failed spinner could be retried with a new spinner, as @dipanm said, so the state of the multi could end as success even though one of the spinner has failed.

Probably the best approach could be to allow detaching the state of the multi, so we can set it manually. A flag on the multi constructor could be of use. link_states: true by default, for example.

@piotrmurach
Copy link
Owner

I'd suggest we tackle this by introducing a new option named manual_stop for the multi-spinner.

spinners = TTY::Spinner::Multi.new(":spinner", manual_stop: true)

This would disable any automatic stopping of the top-level spinner when any of the child spinners errors or all of them succeed. I'd keep the current behaviour as is to keep it compatible. My preference is when a library out of the box does what is the most common use case and similarly allows to disable it.

@frsantos I believe the idea above matches your suggestion. However, I'd be reluctant to use link_states as the flag due to its ambiguous meaning. The multi-spinner state is somewhat complex and any potential user could be confused about such flag's purpose. Fundamentally, what we're trying to do here is disable automating stopping of the multi-spinner. Hence I think that manual_stop or auto_stop would convey the meaning more intuitively. Wouldn't you agree?

@frsantos
Copy link

That would work.

However, I am no longer working on the project where I needed the feature, and so I'm no longer using the gem.

@KevSlashNull KevSlashNull linked a pull request Jul 22, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants