From 2f9cd83479a1bdaa78289bfc741dc531f5b98cec Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 21 Jul 2016 11:27:54 +0100 Subject: [PATCH 1/5] config_file_path is no longer required by remoteappmanager --- jupyterhub/jupyterhub_config.py | 5 ----- remoteappmanager/__main__.py | 5 ++++- remoteappmanager/file_config.py | 4 ---- remoteappmanager/spawner.py | 17 ++++++++--------- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 4adfbe13f..13b686986 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -1,5 +1,3 @@ -import os - # Configuration file for jupyterhub. from jupyter_client.localinterfaces import public_ips @@ -16,9 +14,6 @@ if setting_mode == 'virtual_user': c.JupyterHub.spawner_class = 'remoteappmanager.spawner.VirtualUserSpawner' - c.Spawner.config_file_path = os.path.abspath( - './remoteappmanager_config.py') - # Parent directory in which temporary directory is created for # each virtual user # Set this to a drive with well defined capacity quota diff --git a/remoteappmanager/__main__.py b/remoteappmanager/__main__.py index 69df9d2a8..c66d1339a 100644 --- a/remoteappmanager/__main__.py +++ b/remoteappmanager/__main__.py @@ -13,7 +13,10 @@ def main(): try: command_line_config.parse_config() file_config = FileConfig() - file_config.parse_config(command_line_config.config_file) + + if command_line_config.config_file: + file_config.parse_config(command_line_config.config_file) + except Exception as e: print_help() print("Error: {}".format(e)) diff --git a/remoteappmanager/file_config.py b/remoteappmanager/file_config.py index 8966ed8c0..4c2e01a67 100644 --- a/remoteappmanager/file_config.py +++ b/remoteappmanager/file_config.py @@ -98,10 +98,6 @@ def parse_config(self, config_file): """Parses the config file, and assign their values to our local traits. """ - if config_file.strip() == '': - raise tornado.options.Error("Config file must be specified " - "in command line arguments.") - # Keep the file line parser isolated, but use the global one # so that we can get the help of the command line options. file_line_parser = tornado.options.OptionParser() diff --git a/remoteappmanager/spawner.py b/remoteappmanager/spawner.py index 863787910..bf5c5aa5f 100644 --- a/remoteappmanager/spawner.py +++ b/remoteappmanager/spawner.py @@ -38,7 +38,10 @@ def get_args(self): args[iarg] = arg.replace('--base-url=', '--base-urlpath=') args.append("--proxy-api-url={}".format(self.proxy.api_server.url)) - args.append("--config-file={}".format(self.config_file_path)) + + if self.config_file_path: + args.append("--config-file={}".format(self.config_file_path)) + return args def get_env(self): @@ -47,13 +50,6 @@ def get_env(self): env.update(_docker_envvars()) return env - @default("config_file_path") - def _config_file_path_default(self): - """Defines the default position of the configuration file for - our utility. By default, it's the cwd of where we started up.""" - return os.path.join(os.getcwd(), - os.path.basename(self.cmd[0])+'_config.py') - class VirtualUserSpawner(LocalProcessSpawner): #: The instance of the orm Proxy. @@ -112,7 +108,10 @@ def get_args(self): args[iarg] = arg.replace('--base-url=', '--base-urlpath=') args.append("--proxy-api-url={}".format(self.proxy.api_server.url)) - args.append("--config-file={}".format(self.config_file_path)) + + if self.config_file_path: + args.append("--config-file={}".format(self.config_file_path)) + return args def user_env(self, env): From 1c34430ed2ac93b1122dbe57c72059b7ff409934 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 21 Jul 2016 11:35:48 +0100 Subject: [PATCH 2/5] flake8 --- remoteappmanager/spawner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/spawner.py b/remoteappmanager/spawner.py index bf5c5aa5f..a71f8f6a2 100644 --- a/remoteappmanager/spawner.py +++ b/remoteappmanager/spawner.py @@ -2,7 +2,7 @@ import shutil import tempfile -from traitlets import Any, Unicode, default +from traitlets import Any, Unicode from tornado import gen from jupyterhub.spawner import LocalProcessSpawner From 20fdf0b5b01ee76c6a3c9f14aa7161404035ca0c Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 21 Jul 2016 11:51:25 +0100 Subject: [PATCH 3/5] added more tests for spawner cmd --- tests/spawner/test_spawner.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/spawner/test_spawner.py b/tests/spawner/test_spawner.py index e080456b8..270da2fea 100644 --- a/tests/spawner/test_spawner.py +++ b/tests/spawner/test_spawner.py @@ -72,10 +72,10 @@ def new_spawner(spawner_class): return spawner_class(db=db, user=user, hub=hub) -class TestSpawner(testing.AsyncTestCase): +class TestSpawner(TempMixin, testing.AsyncTestCase): def setUp(self): - self.spawner = new_spawner(Spawner) super().setUp() + self.spawner = new_spawner(Spawner) def test_args(self): path = fixtures.get("remoteappmanager_config.py") @@ -85,13 +85,17 @@ def test_args(self): self.assertIn("--config-file={}".format(path), args) self.assertIn("--base-urlpath=/", args) + def test_args_without_config_file_path(self): + args = self.spawner.get_args() + self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) + self.assertFalse(any("--config-file=" in arg for arg in args)) + self.assertIn("--base-urlpath=/", args) + def test_cmd(self): self.assertEqual(self.spawner.cmd, ['remoteappmanager']) def test_default_config_file_path(self): - self.assertEqual(self.spawner.config_file_path, - os.path.join(os.getcwd(), - "remoteappmanager_config.py")) + self.assertEqual(self.spawner.config_file_path, "") def test_env(self): env = self.spawner.get_env() @@ -125,7 +129,7 @@ def test_cmd_spawning(self): exc.output.decode(sys.getdefaultencoding()))) raise - def test_spawner_start_and_stop(self): + def test_spawner_start_and_stop_with_config_file(self): path = fixtures.get("remoteappmanager_config.py") self.spawner.config_file_path = path @@ -136,13 +140,19 @@ def test_spawner_start_and_stop(self): status = self.io_loop.run_sync(self.spawner.poll) self.assertEqual(status, 1) + def test_spawner_start_and_stop_without_config_file(self): + with spawner_start_and_stop(self.io_loop, self.spawner): + status = self.io_loop.run_sync(self.spawner.poll) + self.assertIsNone(status) + + status = self.io_loop.run_sync(self.spawner.poll) + self.assertEqual(status, 1) -class TestVirtualUserSpawner(TempMixin, testing.AsyncTestCase): + +class TestVirtualUserSpawner(TestSpawner): def setUp(self): super().setUp() self.spawner = new_spawner(VirtualUserSpawner) - path = fixtures.get("remoteappmanager_config.py") - self.spawner.config_file_path = path def test_spawner_without_workspace_dir(self): with spawner_start_and_stop(self.io_loop, self.spawner): From c12f49010f5410f7d8fb9a4ec4837fdefa20e04a Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 27 Jul 2016 13:23:20 +0100 Subject: [PATCH 4/5] Updated docs --- doc/source/configuration.rst | 28 +++++++++++++--------------- doc/source/deployment.rst | 4 ++-- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index f0ea8c423..e0bb8889d 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -1,3 +1,5 @@ +.. _configuration: + Configuration ============= @@ -40,28 +42,24 @@ docker setup. .. literalinclude:: remoteappmanager_help.txt When **remoteappmanager** is started from jupyterhub using the spawner, - all the command line options are filled in, with the config file path - default to `remoteappmanager_config.py` in the current directory. - - It is possible to change this location by specifying:: + all the command line options are filled in automatically. - c.Spawner.config_file_path = "/path/to/config.py" - in the `jupyterhub_config.py` file. Note that this config file will be used by - all remoteappmanagers for any user. +2. Config file + The **remoteappmanager** has a number of parameters configurable via a + config file. The path of the config file should be specified in the + spawner in `jupyterhub_config.py`:: -2. Config file + c.Spawner.config_file_path = "/path/to/config.py" - Path to the config file is given by the `--config-file-path` command-line - option. It is a Python file in which default values of attributes in - :py:class:`remoteappmanager.file_config.FileConfig` can be override. + Please refer to :py:class:`remoteappmanager.file_config.FileConfig` for + the configurable parameters. Note that this config file will be used + by all remoteappmanagers for any user. - For example, to use CSV as the database, one could provide a config file - with the following settings:: + For example, to use CSV as the database, `/path/to/config.py` would + contain the followings:: accounting_class = 'remoteappmanager.db.csv_db.CSVAccounting' accounting_kwargs = {'url': '/path/to/csv_file'} - Please refer to :py:class:`remoteappmanager.file_config.FileConfig` for - the configurable parameters. diff --git a/doc/source/deployment.rst b/doc/source/deployment.rst index 916e9fb50..c2c9b703f 100644 --- a/doc/source/deployment.rst +++ b/doc/source/deployment.rst @@ -44,8 +44,8 @@ Deployment of the complete system in a single machine/VM. cd ../jupyterhub - and verify that `jupyterhub_config.py` and `remoteappmanager_config.py` are - correct for your deployment machine setup. + and verify that `jupyterhub_config.py` is correct for your deployment + machine setup (see :ref:`configuration`). Setup docker containers ----------------------- From 7755b28c362993cfa0fbe285600e3cbb1a8482e4 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 27 Jul 2016 13:38:09 +0100 Subject: [PATCH 5/5] skip coverage remoteappmanager.main is tested by test_spawner via subprocess --- remoteappmanager/__main__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/remoteappmanager/__main__.py b/remoteappmanager/__main__.py index c66d1339a..165e73651 100644 --- a/remoteappmanager/__main__.py +++ b/remoteappmanager/__main__.py @@ -1,13 +1,14 @@ -import sys +import sys # pragma: no cover -from remoteappmanager.command_line_config import CommandLineConfig -from remoteappmanager.file_config import FileConfig -from tornado.options import print_help +from remoteappmanager.command_line_config import ( + CommandLineConfig) # pragma: no cover +from remoteappmanager.file_config import FileConfig # pragma: no cover +from tornado.options import print_help # pragma: no cover -from remoteappmanager.application import Application +from remoteappmanager.application import Application # pragma: no cover -def main(): +def main(): # pragma: no cover command_line_config = CommandLineConfig() try: