Merge pull request #984 from minrk/spawner-shell

allow customization of spawn command

dev-requirements.txt

@@ -2,6 +2,7 @@
 mock
 codecov
 pytest-cov
+pytest-tornado
 pytest>=2.8
 notebook
 requests-mock

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}",
@@ -708,6 +707,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 +812,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))
-        try:
+        
-            self.proc = Popen(cmd, env=env,
+        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, **popen_kwargs)
         except PermissionError:
             # use which to get abspath
             script = shutil.which(cmd[0]) or cmd[0]

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(),
         )

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),
     ])
     

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,9 +53,10 @@ 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
@@ -60,44 +64,53 @@ def test_spawner(db, io_loop):
     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)
+    yield spawner.stop()
-    status = io_loop.run_sync(spawner.poll)
+    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,
+    yield wait_for_spawner(spawner)
-    # checking for early termination every 1s
+    status = yield spawner.poll()
-    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:
+    yield spawner.stop()
-            io_loop.run_sync(wait)
+    status = yield spawner.poll()
-        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)
     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'