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.

Sign up for GitHub

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

Jump to bottom

Add Custom Timeout - Closes #8 #14

Merged
merged 2 commits into from Apr 22, 2018
Merged

Add Custom Timeout - Closes #8 #14

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f1e46f
:sparkles: Add Custom Timeout - Closes #8
ELC Apr 22, 2018
af3af57
:ok_hand: Avoid using Globals and Refactor
ELC Apr 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion nbflow/example/SConstruct
View file
Open in desktop
Expand Up @@ -2,4 +2,8 @@ import os
from nbflow.scons import setup

env = Environment(ENV=os.environ)
setup(env, ["analyses"])
timeout = ARGUMENTS.get('timeout', 0)
Copy link
Owner

Choose a reason for hiding this comment

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

Ideally this is something that would be handled by the nbflow code, rather than something users have to put in the SConstruct file. Maybe pass the ARGUMENTS variable to the setup function directly and let setup parse the timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The next commit fixes this parsing the ARGUMENTS inside setup

if int(timeout):
setup(env, ["analyses"], int(timeout))
else:
setup(env, ["analyses"])
10 changes: 10 additions & 0 deletions nbflow/example/analyses/analyze_data.ipynb
View file
Open in desktop
Expand Up @@ -23,6 +23,16 @@
"import json"
]
},
{
"cell_type": "code",
"execution_count": 3,
"metadata": {},
"outputs": [],
"source": [
"import time\n",
"time.sleep(5) # Simulate long running task"
]
},
{
"cell_type": "code",
"execution_count": 3,
Expand Down
9 changes: 7 additions & 2 deletions nbflow/scons.py
View file
Open in desktop
Expand Up @@ -4,11 +4,13 @@

import nbconvert

TIMEOUT = "120"

def build_cmd(notebook):
cmd = [
"jupyter", "nbconvert",
"--log-level=ERROR",
"--ExecutePreprocessor.timeout=120",
"--ExecutePreprocessor.timeout=" + TIMEOUT,
"--execute",
"--inplace",
"--to", "notebook"
Expand Down Expand Up @@ -49,9 +51,12 @@ def print_cmd_line(s, targets, sources, env):
sys.stdout.write("%s --> %s\n"% (str(sources[0]), str(target)))


def setup(env, directories):
def setup(env, directories, timeout=None):
env['PRINT_CMD_LINE_FUNC'] = print_cmd_line
env.Decider('timestamp-newer')
if timeout is not None:
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, it would be nice if we could come up with a way to do this without using global variables (see here for an explanation of why avoiding globals is a good idea).

One way this might be possible is to create a class, say called Builder, that has an attribute called timeout and that has a method called build_notebook. Then here you can create an instance of builder, set the timeout value on it, and then use the method on the instance below in env.Command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the next commit, intead of using classes, I chose a more functional approach and use only pure functions and define a default value for timeout in build_notebook. This seems more cleaner to me.

Because of the nature of env.Command I pass a new function build_notebook_timeout which is a partial evaluation of build_notebook in case a timeout is set or the build_notebook function in case no timeout is passed.

global TIMEOUT
TIMEOUT = str(timeout)
DEPENDENCIES = json.loads(sp.check_output([sys.executable, "-m", "nbflow"] + directories).decode('UTF-8'))
for script in DEPENDENCIES:
deps = DEPENDENCIES[script]
Expand Down
9 changes: 9 additions & 0 deletions nbflow/tests/test_nbflow.py
View file
Open in desktop
Expand Up @@ -11,6 +11,15 @@ def test_nbflow_no_args(temp_cwd):
run_command([sys.executable, "-m", "nbflow"], retcode=1)


def test_notebook_long_excecution(temp_cwd, sconstruct):
# copy example files
root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "example"))
shutil.copytree(os.path.join(root, "analyses"), "analyses")
shutil.copy(os.path.join(root, "SConstruct"), "SConstruct")
clear_notebooks("analyses")

run_command(["scons","timeout=1"], retcode=2)

def test_example(temp_cwd):
# copy example files
root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "example"))
Expand Down