Merge pull request #768 from minrk/service-url

only set service URL env if there's a URL to set

jupyterhub/services/service.py

@@ -225,8 +225,9 @@ class Service(LoggingConfigurable):
         env['JUPYTERHUB_API_TOKEN'] = self.api_token
         env['JUPYTERHUB_API_URL'] = self.hub_api_url
         env['JUPYTERHUB_BASE_URL'] = self.base_url
-        env['JUPYTERHUB_SERVICE_PREFIX'] = self.server.base_url
-        env['JUPYTERHUB_SERVICE_URL'] = self.url
+        if self.url:
+            env['JUPYTERHUB_SERVICE_URL'] = self.url
+            env['JUPYTERHUB_SERVICE_PREFIX'] = self.server.base_url
 
         self.spawner = _ServiceSpawner(
             cmd=self.command,
@@ -248,7 +249,7 @@ class Service(LoggingConfigurable):
         """Called when the service process unexpectedly exits"""
         self.log.error("Service %s exited with status %i", self.name, self.proc.returncode)
         self.start()
-        
+
     def stop(self):
         """Stop a managed service"""
         if not self.managed:

jupyterhub/tests/conftest.py

@@ -66,9 +66,16 @@ class MockServiceSpawner(jupyterhub.services.service._ServiceSpawner):
     poll_interval = 1
 
 
-@yield_fixture
-def mockservice(request, app):
+def _mockservice(request, app, url=False):
     name = 'mock-service'
+    spec = {
+        'name': name,
+        'command': mockservice_cmd,
+        'admin': True,
+    }
+    if url:
+        spec['url'] = 'http://127.0.0.1:%i' % random_port(),
+
     with mock.patch.object(jupyterhub.services.service, '_ServiceSpawner', MockServiceSpawner):
         app.services = [{
             'name': name,
@@ -88,4 +95,12 @@ def mockservice(request, app):
         # ensure process finishes starting
         with raises(TimeoutExpired):
             service.proc.wait(1)
-        yield service
+    return service
+
+@yield_fixture
+def mockservice(request, app):
+    yield _mockservice(request, app, url=False)
+
+@yield_fixture
+def mockservice_url(request, app):
+    yield _mockservice(request, app, url=True)

jupyterhub/tests/test_services.py

@@ -59,10 +59,11 @@ def test_managed_service(app, mockservice):
     assert service.proc.poll() is None
 
 
-def test_proxy_service(app, mockservice, io_loop):
-    name = mockservice.name
+def test_proxy_service(app, mockservice_url, io_loop):
+    service = mockservice_url
+    name = service.name
     routes = io_loop.run_sync(app.proxy.get_routes)
-    url = public_url(app, mockservice) + '/foo'
+    url = public_url(app, service) + '/foo'
     r = requests.get(url, allow_redirects=False)
     path = '/services/{}/foo'.format(name)
     r.raise_for_status()
@@ -99,3 +100,5 @@ def test_external_service(app, io_loop):
         assert len(resp) >= 1
         assert isinstance(resp[0], dict)
         assert 'name' in resp[0]
+
+