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

Consider changing Task.Poll interface function to add a context.Context argument #6

Open
inkel opened this issue Mar 9, 2022 · 0 comments

Comments

@inkel
Copy link
Contributor

inkel commented Mar 9, 2022

While looking at the current interface definition for Task I've seen that Task.Poll doesn't accept any argument nor return an error (more on this later).

Then looking at implementations of the interface, I've found that a context could be useful here:

Contexts are great and very powerful when used this way. As an example, if the interface was called this way, we could have in our main.go the following piece of code:

func main() {
	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt)
	defer stop()

	// …create tasks and everything…

	root.Poll(ctx)

	// …continue…
}

And if the user clicks ^C while the program is calling root.Poll it will cancel the context, thus passing this cancellation information down to all processes and we will have better resource management and cancellation, rather than an abrupt termination.

What do you think?

PS: I mention this function also returning an error. I think it could be very useful information to have, for instance, implementations would perhaps return ctx.Err() which might indicate to the caller that the context was cancelled or timed out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant