From e9fc629285c6b0cb8dc42ecb88c4a1b556a49ea9 Mon Sep 17 00:00:00 2001
From: Min RK <benjaminrk@gmail.com>
Date: Wed, 21 Sep 2016 12:39:07 +0200
Subject: [PATCH] only set service URL env if there's a URL to set

These fields are only relevant for services with a web endpoint
---
 jupyterhub/services/service.py    |  7 ++++---
 jupyterhub/tests/conftest.py      | 21 ++++++++++++++++++---
 jupyterhub/tests/test_services.py |  9 ++++++---
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py
index bfa1736b..bf22ac94 100644
--- a/jupyterhub/services/service.py
+++ b/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:
diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py
index 0497dbb1..7be4639f 100644
--- a/jupyterhub/tests/conftest.py
+++ b/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)
diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py
index ff5a8ed1..1b43922c 100644
--- a/jupyterhub/tests/test_services.py
+++ b/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]
+
+