From 4ae264de5e20fea23652edea3d332c517962e391 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 13 Feb 2017 15:55:28 +0100 Subject: [PATCH 1/6] allow customization of spawn command - add shell_cmd for launching with a shell (e.g. `bash -l -c` - add popen_kwargs for overriding keyword args passed to Popen --- jupyterhub/spawner.py | 49 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 2829ddea..156f2049 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -708,6 +708,37 @@ class LocalProcessSpawner(Spawner): """ ).tag(config=True) + popen_kwargs = Dict( + help="""Extra keyword arguments to pass to Popen + + when spawning single-user servers. + + For example:: + + popen_kwargs = dict(shell=True) + + """ + ).tag(config=True) + shell_cmd = Command(minlen=0, + help="""Specify a shell command to launch. + + The single-user command will be appended to this list, + so it sould end with `-c` (for bash) or equivalent. + + For example:: + + c.LocalProcessSpawner.shell_cmd = ['bash', '-l', '-c'] + + to launch with a bash login shell, which would set up the user's own complete environment. + + .. warning:: + + Using shell_cmd gives users control over PATH, etc., + which could change what the jupyterhub-singleuser launch command does. + Only use this for trusted users. + """ + ) + proc = Instance(Popen, allow_none=True, help=""" @@ -782,12 +813,22 @@ class LocalProcessSpawner(Spawner): cmd.extend(self.cmd) cmd.extend(self.get_args()) + if self.shell_cmd: + # using shell_cmd (e.g. bash -c), + # add our cmd list as the last (single) argument: + cmd = self.shell_cmd + [' '.join(pipes.quote(s) for s in cmd)] + self.log.info("Spawning %s", ' '.join(pipes.quote(s) for s in cmd)) + + popen_kwargs = dict( + preexec_fn=self.make_preexec_fn(self.user.name), + start_new_session=True, # don't forward signals + ) + popen_kwargs.update(self.popen_kwargs) + # don't let user config override env + popen_kwargs['env'] = env try: - self.proc = Popen(cmd, env=env, - preexec_fn=self.make_preexec_fn(self.user.name), - start_new_session=True, # don't forward signals - ) + self.proc = Popen(cmd, **popen_kwargs) except PermissionError: # use which to get abspath script = shutil.which(cmd[0]) or cmd[0] From 00aa92f7b664dcb878a6938947bceb9b77bc8d34 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 13 Feb 2017 17:53:30 +0100 Subject: [PATCH 2/6] add env handler to mocksu --- jupyterhub/tests/mocksu.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/mocksu.py b/jupyterhub/tests/mocksu.py index e770c146..ae807c3a 100644 --- a/jupyterhub/tests/mocksu.py +++ b/jupyterhub/tests/mocksu.py @@ -9,7 +9,7 @@ import json import sys from tornado import web, httpserver, ioloop - +from .mockservice import EnvHandler class EchoHandler(web.RequestHandler): def get(self): @@ -23,6 +23,7 @@ def main(args): app = web.Application([ (r'.*/args', ArgsHandler), + (r'.*/env', EnvHandler), (r'.*', EchoHandler), ]) From 7afbe952e63f549b8fa724bf0a3fbab524cd1e6a Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 13 Feb 2017 17:55:50 +0100 Subject: [PATCH 3/6] test take pytest-tornado for a spin it's nice! --- dev-requirements.txt | 1 + jupyterhub/tests/test_spawner.py | 101 +++++++++++++++++++++++-------- 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index a118ce73..a6fcd15e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -2,6 +2,7 @@ mock codecov pytest-cov +pytest-tornado pytest>=2.8 notebook requests-mock diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index 68dfcf43..91dd7d67 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -6,11 +6,14 @@ import logging import os import signal +from subprocess import Popen import sys import tempfile import time from unittest import mock +import pytest +import requests from tornado import gen from .. import spawner as spawnermod @@ -50,54 +53,64 @@ def new_spawner(db, **kwargs): return LocalProcessSpawner(db=db, **kwargs) -def test_spawner(db, io_loop): +@pytest.mark.gen_test +def test_spawner(db, request): spawner = new_spawner(db) - ip, port = io_loop.run_sync(spawner.start) + ip, port = yield spawner.start() assert ip == '127.0.0.1' assert isinstance(port, int) assert port > 0 spawner.user.server.ip = ip spawner.user.server.port = port db.commit() - # wait for the process to get to the while True: loop time.sleep(1) - status = io_loop.run_sync(spawner.poll) + status = yield spawner.poll() assert status is None - io_loop.run_sync(spawner.stop) - status = io_loop.run_sync(spawner.poll) + yield spawner.stop() + status = yield spawner.poll() assert status == 1 -def test_single_user_spawner(db, io_loop): + +@gen.coroutine +def wait_for_spawner(spawner, timeout=10): + """Wait for an http server to show up + + polling at shorter intervals for early termination + """ + deadline = time.monotonic() + timeout + def wait(): + return spawner.user.server.wait_up(timeout=1, http=True) + while time.monotonic() < deadline: + status = yield spawner.poll() + assert status is None + try: + yield wait() + except TimeoutError: + continue + else: + break + yield wait() + + +@pytest.mark.gen_test(run_sync=False) +def test_single_user_spawner(db, request): spawner = new_spawner(db, cmd=['jupyterhub-singleuser']) spawner.api_token = 'secret' - ip, port = io_loop.run_sync(spawner.start) + ip, port = yield spawner.start() assert ip == '127.0.0.1' assert isinstance(port, int) assert port > 0 spawner.user.server.ip = ip spawner.user.server.port = port db.commit() - # wait for http server to come up, - # checking for early termination every 1s - def wait(): - return spawner.user.server.wait_up(timeout=1, http=True) - for i in range(30): - status = io_loop.run_sync(spawner.poll) - assert status is None - try: - io_loop.run_sync(wait) - except TimeoutError: - continue - else: - break - io_loop.run_sync(wait) - status = io_loop.run_sync(spawner.poll) - assert status == None - io_loop.run_sync(spawner.stop) - status = io_loop.run_sync(spawner.poll) + wait_for_spawner(spawner) + status = yield spawner.poll() + assert status is None + yield spawner.stop() + status = yield spawner.poll() assert status == 0 @@ -192,3 +205,39 @@ def test_string_formatting(db): assert s.format_string(s.notebook_dir) == 'user/%s/' % name assert s.format_string(s.default_url) == '/base/%s' % name + +@pytest.mark.gen_test +def test_popen_kwargs(db): + mock_proc = mock.Mock(spec=Popen) + def mock_popen(*args, **kwargs): + mock_proc.args = args + mock_proc.kwargs = kwargs + mock_proc.pid = 5 + return mock_proc + + s = new_spawner(db, popen_kwargs={'shell': True}, cmd='jupyterhub-singleuser') + with mock.patch.object(spawnermod, 'Popen', mock_popen): + yield s.start() + + assert mock_proc.kwargs['shell'] == True + assert mock_proc.args[0][:1] == (['jupyterhub-singleuser']) + + +@pytest.mark.gen_test +def test_shell_cmd(db, tmpdir, request): + f = tmpdir.join('bashrc') + f.write('export TESTVAR=foo\n') + s = new_spawner(db, + cmd=[sys.executable, '-m', 'jupyterhub.tests.mocksu'], + shell_cmd=['bash', '--rcfile', str(f), '-i', '-c'], + ) + (ip, port) = yield s.start() + request.addfinalizer(s.stop) + s.user.server.ip = ip + s.user.server.port = port + db.commit() + yield wait_for_spawner(s) + r = requests.get('http://%s:%i/env' % (ip, port)) + r.raise_for_status() + env = r.json() + assert env['TESTVAR'] == 'foo' From 3996fa00ef3f7f97a130212c874f04fa067200b0 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 15 Feb 2017 11:29:48 +0100 Subject: [PATCH 4/6] turn off database echo in tests it's a bunch of noise --- jupyterhub/tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index ed4acf77..b21bd921 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -27,7 +27,7 @@ def db(): """Get a db session""" global _db if _db is None: - _db = orm.new_session_factory('sqlite:///:memory:', echo=True)() + _db = orm.new_session_factory('sqlite:///:memory:')() user = orm.User( name=getuser(), ) From 67d6de9f68f1e6662dabe31333a4b0779c33dc32 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 15 Feb 2017 11:30:34 +0100 Subject: [PATCH 5/6] don't forget to yield --- jupyterhub/tests/test_spawner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index 91dd7d67..511f23b5 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -106,7 +106,7 @@ def test_single_user_spawner(db, request): spawner.user.server.ip = ip spawner.user.server.port = port db.commit() - wait_for_spawner(spawner) + yield wait_for_spawner(spawner) status = yield spawner.poll() assert status is None yield spawner.stop() From 27410a6c51555acccc0107f5cf342b19db4e8c24 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 15 Feb 2017 11:31:25 +0100 Subject: [PATCH 6/6] remove spurious print --- jupyterhub/spawner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 156f2049..603eafec 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -258,7 +258,6 @@ class Spawner(LoggingConfigurable): @validate('notebook_dir', 'default_url') def _deprecate_percent_u(self, proposal): - print(proposal) v = proposal['value'] if '%U' in v: self.log.warning("%%U for username in %s is deprecated in JupyterHub 0.7, use {username}",