From a8548164cddb4836a0c6e54cd63f306ee92d5dfb Mon Sep 17 00:00:00 2001 From: MinRK Date: Fri, 10 Oct 2014 12:28:27 -0700 Subject: [PATCH] remove Spawner.fromJSON load state on `__init__` instead Makes more sense now that state can persist across server instances (e.g. docker container_id) --- jupyterhub/app.py | 20 +++++++----- jupyterhub/orm.py | 7 ++++- jupyterhub/spawner.py | 73 ++++++++++++++++++++++++++----------------- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 657288bf..7e47a72f 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -448,20 +448,25 @@ class JupyterHubApp(Application): for user in db.query(orm.User): if not user.state: + # without spawner state, server isn't valid + user.server = None user_summaries.append(_user_summary(user)) continue self.log.debug("Loading state for %s from db", user.name) - spawner = self.spawner_class.fromJSON(user.state, user=user, hub=self.hub, config=self.config) + user.spawner = spawner = self.spawner_class( + user=user, hub=self.hub, config=self.config, + ) status = run_sync(spawner.poll) if status is None: - self.log.info("User %s still running", user.name) - user.spawner = spawner + self.log.info("%s still running", user.name) spawner.add_poll_callback(user_stopped, user) spawner.start_polling() else: - self.log.warn("Failed to load state for %s, assuming server is not running.", user.name) - # not running, state is invalid - user.state = {} + # user not running. This is expected if server is None, + # but indicates the user's server died while the Hub wasn't running + # if user.server is defined. + log = self.log.warn if user.server else self.log.debug + log("%s not running.", user.name) user.server = None user_summaries.append(_user_summary(user)) @@ -508,7 +513,8 @@ class JupyterHubApp(Application): '--api-port', str(self.proxy.api_server.port), '--default-target', self.hub.server.host, ] - if self.log_level == logging.DEBUG: + if False: + # if self.log_level == logging.DEBUG: cmd.extend(['--log-level', 'debug']) if self.ssl_key: cmd.extend(['--ssl-key', self.ssl_key]) diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 9aa8a6af..b26bc6c1 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -305,13 +305,17 @@ class User(Base): api_token = self.new_api_token() db.add(api_token) db.commit() - + + spawner = self.spawner = spawner_class( config=config, user=self, hub=hub, api_token=api_token.token, ) + # we are starting a new server, make sure it doesn't restore state + spawner.clear_state() + yield spawner.start() spawner.start_polling() @@ -335,6 +339,7 @@ class User(Base): status = yield self.spawner.poll() if status is None: yield self.spawner.stop() + self.spawner.clear_state() self.state = self.spawner.get_state() self.last_activity = datetime.utcnow() self.server = None diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 78ee31b4..0be6177a 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -80,15 +80,10 @@ class Spawner(LoggingConfigurable): help="""The command used for starting notebooks.""" ) - @classmethod - def fromJSON(cls, state, **kwargs): - """Create a new instance, and load its JSON state - - state will be a dict, loaded from JSON in the database. - """ - inst = cls(**kwargs) - inst.load_state(state) - return inst + def __init__(self, **kwargs): + super(Spawner, self).__init__(**kwargs) + if self.user.state: + self.load_state(self.user.state) def load_state(self, state): """load state from the database @@ -101,9 +96,10 @@ class Spawner(LoggingConfigurable): See Also -------- - get_state + get_state, clear_state """ - pass + if 'api_token' in state: + self.api_token = state['api_token'] def get_state(self): """store the state necessary for load_state @@ -117,7 +113,19 @@ class Spawner(LoggingConfigurable): state: dict a JSONable dict of state """ - return dict(api_token=self.api_token) + state = {} + if self.api_token: + state['api_token'] = self.api_token + return state + + def clear_state(self): + """clear any state that should be cleared when the process stops + + State that should be preserved across server instances should not be cleared. + + Subclasses should call super, to ensure that state is properly cleared. + """ + self.api_token = '' def get_args(self): """Return the arguments to be passed after self.cmd""" @@ -208,7 +216,7 @@ class Spawner(LoggingConfigurable): def wait_for_death(self, timeout=10): """wait for the process to die, up to timeout seconds""" loop = IOLoop.current() - for i in range(int(timeout / self.poll_interval)): + for i in range(int(timeout / self.death_interval)): status = yield self.poll() if status is not None: break @@ -291,15 +299,23 @@ class LocalProcessSpawner(Spawner): raise ValueError("This should be impossible") def load_state(self, state): + """load pid from state""" super(LocalProcessSpawner, self).load_state(state) - self.pid = state['pid'] + if 'pid' in state: + self.pid = state['pid'] def get_state(self): + """add pid to state""" state = super(LocalProcessSpawner, self).get_state() if self.pid: state['pid'] = self.pid return state + def clear_state(self): + """clear pid state""" + super(LocalProcessSpawner, self).clear_state() + self.pid = 0 + def sudo_cmd(self, user): return ['sudo', '-u', user.name] + self.sudo_args @@ -335,38 +351,39 @@ class LocalProcessSpawner(Spawner): status = self.proc.poll() if status is not None: # clear state if the process is done - self.pid = 0 + self.clear_state() raise gen.Return(status) # if we resumed from stored state, - # we don't have the Popen handle anymore + # we don't have the Popen handle anymore, so rely on self.pid + if not self.pid: # no pid, not running + self.clear_state() raise gen.Return(0) + # send signal 0 to check if PID exists # this doesn't work on Windows, but that's okay because we don't support Windows. - try: - os.kill(self.pid, 0) - except OSError as e: - if e.errno == errno.ESRCH: - # no such process, return exitcode == 0, since we don't know the exit status - self.pid = 0 - raise gen.Return(0) - else: - raise + alive = self._signal(0) + if not alive: + self.clear_state() + raise gen.Return(0) else: - # None indicates the process is running raise gen.Return(None) def _signal(self, sig): - """send a signal, and ignore ERSCH because it just means it already died""" + """send a signal, and ignore ERSCH because it just means it already died + + returns bool for whether the process existed to receive the signal. + """ try: os.kill(self.pid, sig) except OSError as e: if e.errno == errno.ESRCH: - return + return False # process is gone else: raise + return True # process exists @gen.coroutine def stop(self, now=False):