diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 0bcd610e..b91b156c 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -1220,7 +1220,7 @@ class UserUrlHandler(BaseHandler): self.log.warning( "User %s requested server for %s, which they don't own", current_user.name, - user.name, + user_name, ) target = url_path_join(current_user.url, user_path or '') if self.request.query: @@ -1270,13 +1270,10 @@ class UserUrlHandler(BaseHandler): # if request is expecting JSON, assume it's an API request and fail with 503 # because it won't like the redirect to the pending page - if ( - get_accepted_mimetype( - self.request.headers.get('Accept', ''), - choices=['application/json', 'text/html'], - ) - == 'application/json' - ): + if get_accepted_mimetype( + self.request.headers.get('Accept', ''), + choices=['application/json', 'text/html'], + ) == 'application/json' or 'api' in user_path.split('/'): self._fail_api_request() return @@ -1376,9 +1373,14 @@ class UserRedirectHandler(BaseHandler): @web.authenticated def get(self, path): user = self.current_user - url = url_path_join(user.url, path) + user_url = url_path_join(user.url, path) if self.request.query: - url = url_concat(url, parse_qsl(self.request.query)) + user_url = url_concat(user_url, parse_qsl(self.request.query)) + + url = url_concat( + url_path_join(self.hub.base_url, "spawn", user.name), {"next": user_url} + ) + self.redirect(url) diff --git a/jupyterhub/handlers/pages.py b/jupyterhub/handlers/pages.py index ff2894a1..6217ccf6 100644 --- a/jupyterhub/handlers/pages.py +++ b/jupyterhub/handlers/pages.py @@ -118,8 +118,8 @@ class SpawnHandler(BaseHandler): raise web.HTTPError(404, "No such user: %s" % for_user) if not self.allow_named_servers and user.running: - url = user.server_url(server_name) - self.log.debug("User is running: %s", url) + url = self.get_next_url(user, default=user.server_url(server_name)) + self.log.info("User is running: %s", user.name) self.redirect(url) return @@ -127,10 +127,10 @@ class SpawnHandler(BaseHandler): server_name = '' spawner = user.spawners[server_name] - # resolve `?next=...`, falling back on the /hub/user/server url - # instead of /hub/home + # resolve `?next=...`, falling back on the spawn-pending url # must not be /user/server for named servers, # which may get handled by the default server if they aren't ready yet + next_url = self.get_next_url( user, default=url_path_join( @@ -139,19 +139,28 @@ class SpawnHandler(BaseHandler): ) # spawner is active, redirect back to get progress, etc. - if spawner.active: + if spawner.ready: + self.log.info("Server %s is already running", spawner._log_name) + next_url = self.get_next_url(user, default=user.server_url(server_name)) + + elif spawner.active: + self.log.info("Server %s is already active", spawner._log_name) self.redirect(next_url) return + # Add handler to spawner here so you can access query params in form rendering. + spawner.handler = self spawner_options_form = await spawner.get_options_form() if spawner_options_form: - # Add handler to spawner here so you can access query params in form rendering. - spawner.handler = self + self.log.debug("Serving options form for %s", spawner._log_name) form = self._render_form( for_user=user, spawner_options_form=spawner_options_form ) self.finish(form) else: + self.log.debug( + "Triggering spawn with default options for %s", spawner._log_name + ) # Explicit spawn request: clear _spawn_future # which may have been saved to prevent implicit spawns # after a failure. @@ -210,11 +219,6 @@ class SpawnHandler(BaseHandler): self.hub.base_url, "spawn-pending", user.name, server_name ), ) - - # request spawn and redirect immediately - # this will send folks to the - f = asyncio.ensure_future(self.spawn_single_user(user, server_name)) - await asyncio.wait([f], timeout=1) self.redirect(next_url) diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index c3ac574d..b760b65e 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -44,9 +44,10 @@ async def test_root_redirect(app): next_url = ujoin(app.base_url, 'user/other/test.ipynb') url = '/?' + urlencode({'next': next_url}) r = await get_page(url, app, cookies=cookies) - r.raise_for_status() path = urlparse(r.url).path - assert path == ujoin(app.base_url, 'user/%s/test.ipynb' % name) + assert path == ujoin(app.base_url, 'hub/user/%s/test.ipynb' % name) + # serve "server not running" page, which has status 503 + assert r.status_code == 503 async def test_root_default_url_noauth(app): @@ -122,11 +123,19 @@ async def test_spawn_redirect(app): r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path - assert path == ujoin(app.base_url, 'user/%s/' % name) + # make sure we visited hub/spawn-pending after spawn + # if spawn was really quick, we might get redirected all the way to the running server, + # so check history instead of r.url + history = [urlparse(_.url).path for _ in r.history] + history.append(path) + assert ujoin(app.base_url, 'hub/spawn-pending', name) in history # should have started server status = await u.spawner.poll() assert status is None + # wait for ready signal before checking next redirect + while not u.spawner.ready: + await asyncio.sleep(0.1) # test spawn page when server is already running (just redirect) r = await get_page('spawn', app, cookies=cookies) @@ -143,9 +152,9 @@ async def test_spawn_redirect(app): # test handing of trailing slash on `/user/name` r = await get_page('user/' + name, app, hub=False, cookies=cookies) - r.raise_for_status() path = urlparse(r.url).path - assert path == ujoin(app.base_url, '/user/%s/' % name) + assert path == ujoin(app.base_url, 'hub/user/%s/' % name) + assert r.status_code == 503 async def test_spawn_handler_access(app): @@ -178,10 +187,11 @@ async def test_spawn_admin_access(app, admin_access): name = 'mariel' user = add_user(app.db, app=app, name=name) app.db.commit() - r = await get_page('user/' + name, app, cookies=cookies) + r = await get_page('spawn/' + name, app, cookies=cookies) r.raise_for_status() assert (r.url.split('?')[0] + '/').startswith(public_url(app, user)) r = await get_page('user/{}/env'.format(name), app, hub=False, cookies=cookies) + r.raise_for_status() env = r.json() assert env['JUPYTERHUB_USER'] == name @@ -316,16 +326,16 @@ async def test_user_redirect_deprecated(app): cookies = await app.login_user(name) r = await get_page('/user/baduser', app, cookies=cookies, hub=False) - r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path - assert path == ujoin(app.base_url, '/user/%s/' % name) + assert path == ujoin(app.base_url, 'hub/user/%s/' % name) + assert r.status_code == 503 r = await get_page('/user/baduser/test.ipynb', app, cookies=cookies, hub=False) - r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path - assert path == ujoin(app.base_url, '/user/%s/test.ipynb' % name) + assert path == ujoin(app.base_url, 'hub/user/%s/test.ipynb' % name) + assert r.status_code == 503 r = await get_page('/user/baduser/test.ipynb', app, hub=False) r.raise_for_status() @@ -389,9 +399,11 @@ async def test_login_redirect(app, running, next_url, location): user = app.users['river'] if location: location = ujoin(app.base_url, location) + elif running: + location = ujoin(app.base_url, 'user/river/') else: # use default url - location = user.url + location = ujoin(app.base_url, 'hub/spawn') url = 'login' if next_url: @@ -575,6 +587,10 @@ async def test_announcements(app, announcements): assert ann01 in text cookies = await app.login_user("jones") + # make sure spawner isn't running + # so we see the spawn form + user = app.users["jones"] + await user.stop() with mock.patch.dict( app.tornado_settings, @@ -650,6 +666,12 @@ async def test_token_page(app): r = await get_page("spawn", app, cookies=cookies) r.raise_for_status() + # wait for the server to be running and visit it + while not user.spawner.ready: + await asyncio.sleep(0.1) + r = await get_page("user/" + user.name, app, cookies=cookies) + r.raise_for_status() + r = await get_page("token", app, cookies=cookies) r.raise_for_status() body = extract_body(r) @@ -661,9 +683,11 @@ async def test_token_page(app): async def test_server_not_running_api_request(app): cookies = await app.login_user("bees") r = await get_page("user/bees/api/status", app, hub=False, cookies=cookies) - assert r.status_code == 404 + assert r.status_code == 503 assert r.headers["content-type"] == "application/json" - assert r.json() == {"message": "bees is not running"} + message = r.json()['message'] + assert "Hub home page" in message + assert "/user/bees" in message async def test_metrics_no_auth(app):