From 5e55753baae71ab22108e37f45d9d4e7890a87af Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 4 May 2017 12:16:47 +0200 Subject: [PATCH] various cleanup to get most tests passing (yay!) --- .flake8 | 3 ++- jupyterhub/apihandlers/users.py | 2 +- jupyterhub/app.py | 14 ++++++++------ jupyterhub/dbutil.py | 1 - jupyterhub/handlers/base.py | 3 ++- jupyterhub/objects.py | 2 +- jupyterhub/orm.py | 10 ---------- jupyterhub/proxy.py | 4 ++-- jupyterhub/services/service.py | 8 ++++++-- jupyterhub/tests/mocking.py | 6 +++--- jupyterhub/tests/test_orm.py | 8 ++++++-- jupyterhub/tests/test_spawner.py | 3 ++- jupyterhub/user.py | 25 ++++++++++++++++--------- 13 files changed, 49 insertions(+), 40 deletions(-) diff --git a/.flake8 b/.flake8 index 48e1d856..ba05051a 100644 --- a/.flake8 +++ b/.flake8 @@ -4,9 +4,10 @@ # W: style warnings # C: complexity # F401: module imported but unused +# F403: import * # F811: redefinition of unused `name` from line `N` # F841: local variable assigned but never used -ignore = E, C, W, F401, F811, F841 +ignore = E, C, W, F401, F403, F811, F841 exclude = .cache, diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 78af7c97..4433dde1 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -224,7 +224,7 @@ class UserCreateNamedServerAPIHandler(APIHandler): def post(self, name): user = self.find_user(name) if user is None: - raise HTTPError(404, "No such user %r" % name) + raise web.HTTPError(404, "No such user %r" % name) if user.running: # include notify, so that a server that died is noticed immediately state = yield user.spawner.poll_and_notify() diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 22e77c29..cad13bbb 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1107,10 +1107,11 @@ class JupyterHub(Application): def init_proxy(self): """Load the Proxy config""" # FIXME: handle deprecated config here - public_url = 'http{s}://{ip}:{port}'.format( + public_url = 'http{s}://{ip}:{port}{base_url}'.format( s='s' if self.ssl_cert else '', ip=self.ip, port=self.port, + base_url=self.base_url, ) self.proxy = self.proxy_class( db=self.db, @@ -1309,21 +1310,22 @@ class JupyterHub(Application): users_count = 0 active_users_count = 0 for prefix, route in routes.items(): - if 'user' not in route['data']: + route_data = route['data'] + if 'user' not in route_data: # not a user route, ignore it continue users_count += 1 - if 'last_activity' not in route['data']: + if 'last_activity' not in route_data: # no last activity data (possibly proxy other than CHP) continue - user = orm.User.find(self.db, route['user']) + user = orm.User.find(self.db, route_data['user']) if user is None: self.log.warning("Found no user for route: %s", route) continue try: - dt = datetime.strptime(route['last_activity'], ISO8601_ms) + dt = datetime.strptime(route_data['last_activity'], ISO8601_ms) except Exception: - dt = datetime.strptime(route['last_activity'], ISO8601_s) + dt = datetime.strptime(route_data['last_activity'], ISO8601_s) user.last_activity = max(user.last_activity, dt) # FIXME: Make this configurable duration. 30 minutes for now! if (datetime.now() - user.last_activity).total_seconds() < 30 * 60: diff --git a/jupyterhub/dbutil.py b/jupyterhub/dbutil.py index e2ee2b8e..f104eadd 100644 --- a/jupyterhub/dbutil.py +++ b/jupyterhub/dbutil.py @@ -89,5 +89,4 @@ def _alembic(*args): if __name__ == '__main__': - import sys _alembic(*sys.argv[1:]) diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 886f3eb1..3f7948f0 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -17,8 +17,9 @@ from tornado.web import RequestHandler from tornado import gen, web from .. import orm -from ..user import User +from ..objects import Server from ..spawner import LocalProcessSpawner +from ..user import User from ..utils import url_path_join # pattern for the authentication token header diff --git a/jupyterhub/objects.py b/jupyterhub/objects.py index ede2a5b6..b4019aaf 100644 --- a/jupyterhub/objects.py +++ b/jupyterhub/objects.py @@ -43,7 +43,7 @@ class Server(HasTraits): port = 443 else: port = 80 - return cls(proto=proto, ip=ip, port=port) + return cls(proto=proto, ip=ip, port=port, base_url=urlinfo.path) @default('port') def _default_port(self): diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index f92d157b..ee626f12 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -148,16 +148,6 @@ class User(Base): # group mapping groups = relationship('Group', secondary='user_group_map', back_populates='users') - @property - def server(self): - """Returns the first element of servers. - Returns None if the list is empty. - """ - if len(self.servers) == 0: - return None - else: - return self.servers[0] - def __repr__(self): if self.server: return "<{cls}({name}@{ip}:{port})>".format( diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 3c9eeccc..b55a9a02 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -365,11 +365,11 @@ class ConfigurableHTTPProxy(Proxy): def api_request(self, path, method='GET', body=None, client=None): """Make an authenticated API request of the proxy.""" client = client or AsyncHTTPClient() - url = url_path_join(self.api_url, path) + url = url_path_join(self.api_url, 'api/routes', path) if isinstance(body, dict): body = json.dumps(body) - self.log.debug("Fetching %s %s", method, url) + self.log.debug("Proxy: Fetching %s %s", method, url) req = HTTPRequest(url, method=method, headers={'Authorization': 'token {}'.format( diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py index 916f8e15..1e5479a6 100644 --- a/jupyterhub/services/service.py +++ b/jupyterhub/services/service.py @@ -52,6 +52,7 @@ from traitlets import ( from traitlets.config import LoggingConfigurable from .. import orm +from ..objects import Server from ..traitlets import Command from ..spawner import LocalProcessSpawner, set_user_setuid from ..utils import url_path_join @@ -60,7 +61,7 @@ class _MockUser(HasTraits): name = Unicode() server = Instance(orm.Server, allow_none=True) state = Dict() - service = Instance(__module__ + '.Service') + service = Instance(__name__ + '.Service') host = Unicode() @property @@ -221,7 +222,10 @@ class Service(LoggingConfigurable): @property def server(self): - return self.orm.server + if self.orm.server: + return Server(orm_server=self.orm.server) + else: + return None @property def prefix(self): diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 03c35e55..660efd74 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -4,7 +4,6 @@ import os import sys from tempfile import NamedTemporaryFile import threading - from unittest import mock import requests @@ -18,6 +17,7 @@ from traitlets import default from ..app import JupyterHub from ..auth import PAMAuthenticator from .. import orm +from ..objects import Server from ..spawner import LocalProcessSpawner from ..singleuser import SingleUserNotebookApp from ..utils import random_port, url_path_join @@ -207,7 +207,7 @@ def public_host(app): if app.subdomain_host: return app.subdomain_host else: - return urlparse(app.proxy.public_url).host + return Server.from_url(app.proxy.public_url).host def public_url(app, user_or_service=None, path=''): @@ -220,7 +220,7 @@ def public_url(app, user_or_service=None, path=''): prefix = user_or_service.server.base_url else: host = public_host(app) - prefix = app.proxy.public_server.base_url + prefix = Server.from_url(app.proxy.public_url).base_url if path: return host + url_path_join(prefix, path) else: diff --git a/jupyterhub/tests/test_orm.py b/jupyterhub/tests/test_orm.py index e2e364a0..d873cef2 100644 --- a/jupyterhub/tests/test_orm.py +++ b/jupyterhub/tests/test_orm.py @@ -7,6 +7,7 @@ import pytest from tornado import gen from .. import orm +from .. import objects from ..user import User from .mocking import MockSpawner @@ -20,6 +21,9 @@ def test_server(db): assert server.proto == 'http' assert isinstance(server.port, int) assert isinstance(server.cookie_name, str) + + # test wrapper + server = objects.Server(orm_server=server) assert server.host == 'http://127.0.0.1:%i' % server.port assert server.url == server.host + '/' assert server.bind_url == 'http://*:%i/' % server.port @@ -29,9 +33,9 @@ def test_server(db): def test_user(db): - user = orm.User(name='kaylee', + user = User(orm.User(name='kaylee', state={'pid': 4234}, - ) + )) server = orm.Server() user.servers.append(server) db.add(user) diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index c22539bc..6ca5581d 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -17,6 +17,7 @@ import requests from tornado import gen from ..user import User +from ..objects import Hub from .. import spawner as spawnermod from ..spawner import LocalProcessSpawner from .. import orm @@ -43,8 +44,8 @@ def setup(): def new_spawner(db, **kwargs): kwargs.setdefault('cmd', [sys.executable, '-c', _echo_sleep]) + kwargs.setdefault('hub', Hub()) kwargs.setdefault('user', User(db.query(orm.User).first(), {})) - kwargs.setdefault('hub', db.query(orm.Hub).first()) kwargs.setdefault('notebook_dir', os.getcwd()) kwargs.setdefault('default_url', '/user/{username}/lab') kwargs.setdefault('INTERRUPT_TIMEOUT', 1) diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 1219ca0d..3822feb2 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -9,9 +9,10 @@ from sqlalchemy import inspect from tornado import gen from tornado.log import app_log -from .utils import url_path_join, default_server_name, new_token +from .utils import url_path_join, default_server_name from . import orm +from .objects import Server from traitlets import HasTraits, Any, Dict, observe, default from .spawner import LocalProcessSpawner @@ -157,6 +158,13 @@ class User(HasTraits): if self.server is None: return False return True + + @property + def server(self): + if len(self.servers) == 0: + return None + else: + return Server(orm_server=self.servers[0]) @property def escaped_name(self): @@ -228,14 +236,13 @@ class User(HasTraits): cookie_name=self.cookie_name, base_url=base_url, ) - db.add(orm_server) - db.commit() - server = Server(orm_server=orm_server) - self.servers.append(server) + self.servers.append(orm_server) api_token = self.new_api_token() db.commit() + server = Server(orm_server=orm_server) + spawner = self.spawner # Passing server_name to the spawner spawner.server_name = server_name @@ -279,7 +286,7 @@ class User(HasTraits): ip_port = yield gen.with_timeout(timedelta(seconds=spawner.start_timeout), f) if ip_port: # get ip, port info from return value of start() - self.server.ip, self.server.port = ip_port + server.ip, server.port = ip_port else: # prior to 0.7, spawners had to store this info in user.server themselves. # Handle < 0.7 behavior with a warning, assuming info was stored in db by the Spawner. @@ -317,14 +324,14 @@ class User(HasTraits): db.commit() self.waiting_for_response = True try: - yield self.server.wait_up(http=True, timeout=spawner.http_timeout) + yield server.wait_up(http=True, timeout=spawner.http_timeout) except Exception as e: if isinstance(e, TimeoutError): self.log.warning( "{user}'s server never showed up at {url} " "after {http_timeout} seconds. Giving up".format( user=self.name, - url=self.server.url, + url=server.url, http_timeout=spawner.http_timeout, ) ) @@ -332,7 +339,7 @@ class User(HasTraits): else: e.reason = 'error' self.log.error("Unhandled error waiting for {user}'s server to show up at {url}: {error}".format( - user=self.name, url=self.server.url, error=e, + user=self.name, url=server.url, error=e, )) try: yield self.stop()