Allowed timeout to be none #260
Conversation
| @@ -93,7 +93,14 @@ def validate(self, obj, value): | |||
| }, | |||
| """Run nbconvert in place, overwriting the existing notebook (only | |||
| relevant when converting to notebook format)""" | |||
| ), | |||
| 'tiemout' : ( | |||
There was a problem hiding this comment.
Looks like this might be a typo ;)
There was a problem hiding this comment.
I wouldn't expect this to be a flag (which means that you give it as --timeout with no argument). If we're adding it to the command line interface, I'd expect it to be an option, so you give e.g. --timeout 20.
|
@sandyshao Thanks for introducing yourself and for submitting the PR. I left a small suggestion on the code. Welcome to Jupyter! |
| @@ -38,12 +38,14 @@ class ExecutePreprocessor(Preprocessor): | |||
| Executes all the cells in a notebook | |||
| """ | |||
|
|
|||
| timeout = Integer(30, config=True, | |||
| timeout = Integer(10, config=True, allow_none=True, | |||
There was a problem hiding this comment.
Is there any reason we're changing the default?
There was a problem hiding this comment.
No, it should be changed back to 30. I told @sandyshao to open even an unfinished PR to get used to Git/GitHub and get review / interact with you guys a bit.
|
Hey @sandyshao Congratulation for your first PR (Pull request) and first time using git! You did great for a first time ! They will give some feedback on what you did, you will receive the feedback by email, but I suggest you look at it using GitHub interface as sometime comments are attached to a specific line. The User interface can be confusing at the beginning so feel free to ask if you have any questions if you have some. One of the other automatic feedback you will get when you do PRs is from TravisCI, our automating testing Bot which says something like:
You should be able to click on it and see where the error came from, @takluyver already detected it, we mistakenly introduce a When you have the time what you can do is try to fix the various comment that have been given to you, commit and sync your branch with your fork on GitHub, this will update this PR. Once your changes are good enough we'll merge them into the main codebase and you'll be officially a contributor ! |
|
Related: Issue #256 @sandyshao Thanks for updating your commits! Feel free to leave a comment once you update, we dont' get automatically notified. This is good enough for a first pull request ! I'm merging and we can refine later if ever needed. |
|
@sandyshao congratulation to your first contribution on GitHub. Here is some (virtual) cake: 🍰 |
Cf jupyter#260 for implementation and closes jupyter#256
|
@sandyshao Congrats! Here's to your first contribution 🍪 and 🍵 |
Cf jupyter#260 for implementation and closes jupyter#256
Hi all, I am an undergraduate student at Berkeley working with Matthias. This is to fix the timeout problem. I set the allow_none to truw to make sure the notebook cell will continue to work. Thanks in advance for all the suggestions and advices!