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

Option to disable ExecutePreprocessor.timeout #256

Closed
dhimmel opened this issue Feb 24, 2016 · 10 comments · Fixed by #264
Closed

Option to disable ExecutePreprocessor.timeout #256

dhimmel opened this issue Feb 24, 2016 · 10 comments · Fixed by #264
Labels
docs good first issue great for new contributors
Milestone

Comments

@dhimmel
Copy link

dhimmel commented Feb 24, 2016

Is there a value for --ExecutePreprocessor.timeout that will disable the per-cell timer? I want my notebooks cells to run until completion, no matter how long that takes.

I'd venture further and propose that the default should be to have no timeout. As the notebook ecosystem progresses, I expect people to want to automate their analysis pipelines, which will include notebooks. When you're using nbconvert to run all inplace, the timeout is unwanted. It's as if python script.py had a timeout enabled by default.

@Carreau
Copy link
Member

Carreau commented Feb 24, 2016

I would say yes according to the list of option of execute preprocessor:

$ jupyter nbconvert --help-all
ExecutePreprocessor options
---------------------------
--ExecutePreprocessor.allow_errors=<Bool>
    Default: False
    If `False` (default), when a cell raises an error the execution is stoppped
    and a `CellExecutionError` is raised. If `True`, execution errors are
    ignored and the execution is continued until the end of the notebook. Output
    from exceptions is included in the cell output in both cases.
...
--ExecutePreprocessor.enabled=<Bool>
    Default: False
--ExecutePreprocessor.interrupt_on_timeout=<Bool>
    Default: False
    If execution of a cell times out, interrupt the kernel and continue
    executing other cells rather than throwing an error and stopping.
...
--ExecutePreprocessor.timeout=<Int>
    Default: 30
    The time to wait (in seconds) for output from executions. If a cell
    execution takes longer, an exception (TimeoutError on python 3+,
    RuntimeError on python 2) is raised.
...

You can also find that on the documentation page

The request to have a value of 0, or negative actually disable is not too far fetched.
I doubt we would disable it by default as the actual behavior was added for people that were actually annoyed it did not stop.

@takluyver
Copy link
Member

I don't think any of those options do what was requested - allow_errors is just for errors raised in the kernel, not for timeout errors.

I think making 0 mean no timeout would make sense, but I wouldn't make it the default. This isn't quite like executing a script, because you can't see any output until it's finished (among other things).

@Carreau
Copy link
Member

Carreau commented Feb 24, 2016

Ok, I missunderstood the options then.

@Carreau Carreau added good first issue great for new contributors URAP labels Feb 24, 2016
@dhimmel
Copy link
Author

dhimmel commented Feb 24, 2016

I think --ExecutePreprocessor.timeout 0 for disabling the timeout would be a great enhancement.

@takluyver, I think it's odd to have a program kill itself by default. Most new users will not be aware of this "feature" and will only become aware of it through debugging, which is frustrating. However, having a computation run for a long time is an expected behavior, which users will investigate solutions for should it be a problem. It's also arbitrary to pick a specific default timeout, when the desired value is highly application dependent.

@minrk
Copy link
Member

minrk commented Feb 25, 2016

I believe timeout = -1 will already do this.

@minrk
Copy link
Member

minrk commented Feb 25, 2016

In general, for timeouts it is common to use either -1 for typed timeout arguments or None to indicate waiting forever. A 0 timeout, while weird at a level like this, does make sense as meaning "Don't wait at all", and that's how the API call we pass this to interprets it.

@takluyver
Copy link
Member

Are you sure? It looks to me like it's getting passed to ZMQSocketChannel.get_msg(), and that seems to expect None as its 'wait indefinitely' sentinel. It looks like passing 0 will just poll for a message and raise Empty if one isn't waiting.

@minrk
Copy link
Member

minrk commented Feb 25, 2016

@takluyver yes, -1 does set an infinite timeout. You are right that 0 will poll and return/raise immediately. That's why we shouldn't use 0. I think None might be clearer than -1 for people not familiar with select/poll, though. We can use this by adding allow_none=True to the traitlet, but for people who need it right now, -1 should already work in every released version.

@takluyver
Copy link
Member

OK, then I'd be in favour of just documenting the behaviour with -1.

@dhimmel
Copy link
Author

dhimmel commented Feb 25, 2016

A 0 timeout, while weird at a level like this, does make sense as meaning "Don't wait at all"

I see how 0 meaning no wait is the most intuitive behavior and also a potentially valuable option in certain situations.

OK, then I'd be in favour of just documenting the behaviour with -1.

Agreed, I can close the issue once the documentation is updated to specify this option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs good first issue great for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants