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

Add methods for handle timeouts #277

Closed
wants to merge 2 commits into from

Conversation

seregayoga
Copy link

@seregayoga seregayoga commented Oct 13, 2017

I added a few new methods for avoiding workaround described in this section https://github.com/robertkrimen/otto#halting-problem
Fixes #278

@seregayoga seregayoga changed the title add feature for handle timeouts Add methods for handle timeouts Oct 13, 2017
@seregayoga
Copy link
Author

@deoxxa what do you think about this?

@deoxxa
Copy link
Collaborator

deoxxa commented Oct 20, 2017

This actually looks perfectly reasonable. I can't really see anything wrong with it, and I'm sure it fits perfectly for this specific use case, but I'm also not totally convinced that it needs to be a top-level, core API function.

I'm thinking it might be better to integrate context.Context support instead, since that's really emerging as the way to do deadlines and timeouts. That is a larger job though and might not happen anytime soon - it depends on someone putting in the work, and I know that I personally don't have the time spare right now. If/when this happens, I wouldn't want to have two (maybe incompatible or conflicting) ways to do timeouts.

What say ye, @stevenh? Any opinions?

@stevenh
Copy link
Collaborator

stevenh commented Oct 21, 2017 via email

@deoxxa
Copy link
Collaborator

deoxxa commented Oct 21, 2017

I'm actually not sure - I'm just assuming there could be tricky stuff. It might be as easy as wiring up a handler between context.Done and the existing interrupt channel though. I won't have time to investigate it for at least the next couple of weeks though.

@seregayoga
Copy link
Author

@deoxxa what signature would you like to see? In method Run, for example.

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be better done with context.Context as then we get timeout and cancellation.

@stevenh
Copy link
Collaborator

stevenh commented Nov 25, 2022

Closing due to age with no feedback

@stevenh stevenh closed this Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add possibility to set timeout
3 participants