Add tljh-config remove-item to remove an item from a list

integration-tests/test_hub.py

@@ -36,9 +36,9 @@ async def test_user_code_execute():
 
 
 @pytest.mark.asyncio
-async def test_user_admin_code():
+async def test_user_admin_add():
     """
-    User logs in, starts a server & executes code
+    User is made an admin, logs in and we check if they are in admin group
     """
     # This *must* be localhost, not an IP
     # aiohttp throws away cookies if we are connecting to an IP!
@@ -56,11 +56,51 @@ async def test_user_admin_code():
     async with User(username, hub_url, partial(login_dummy, password='')) as u:
             await u.login()
             await u.ensure_server()
-            await u.start_kernel()
-            await u.assert_code_output("5 * 4", "20", 5, 5)
 
             # Assert that the user exists
             assert pwd.getpwnam(f'jupyter-{username}') is not None
 
             # Assert that the user has admin rights
+            assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem
+
+
+@pytest.mark.asyncio
+async def test_user_admin_remove():
+    """
+    User is made an admin, logs in and we check if they are in admin group.
+
+    Then we remove them from admin group, and check they *aren't* in admin group :D
+    """
+    # This *must* be localhost, not an IP
+    # aiohttp throws away cookies if we are connecting to an IP!
+    hub_url = 'http://localhost'
+    username = secrets.token_hex(8)
+
+    tljh_config_path = [sys.executable, '-m', 'tljh.config']
+
+    assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'add-item', 'users.admin', username)).wait()
+    assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'reload')).wait()
+
+    # FIXME: wait for reload to finish & hub to come up
+    # Should be part of tljh-config reload
+    await asyncio.sleep(1)
+    async with User(username, hub_url, partial(login_dummy, password='')) as u:
+            await u.login()
+            await u.ensure_server()
+
+            # Assert that the user exists
+            assert pwd.getpwnam(f'jupyter-{username}') is not None
+
+            # Assert that the user has admin rights
+            assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem
+
+
+            assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'remove-item', 'users.admin', username)).wait()
+            assert 0 == await (await asyncio.create_subprocess_exec(*tljh_config_path, 'reload')).wait()
+            await asyncio.sleep(1)
+
+            await u.stop_server()
+            await u.ensure_server()
+
+            # Assert that the user does *not* have admin rights
             assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem

tests/test_config.py

@@ -4,6 +4,7 @@ Test configuration commandline tools
 from tljh import config
 from contextlib import redirect_stdout
 import io
+import pytest
 import tempfile
 
 
@@ -81,6 +82,32 @@ def test_add_to_config_multiple():
         'a': {'b': {'c': ['d', 'e']}}
     }
 
+
+def test_remove_from_config():
+    conf = {}
+
+    new_conf = config.add_item_to_config(conf, 'a.b.c', 'd')
+    new_conf = config.add_item_to_config(new_conf, 'a.b.c', 'e')
+    assert new_conf == {
+        'a': {'b': {'c': ['d', 'e']}}
+    }
+
+    new_conf = config.remove_item_from_config(new_conf, 'a.b.c', 'e')
+    assert new_conf == {
+        'a': {'b': {'c': ['d']}}
+    }
+
+def test_remove_from_config_error():
+    with pytest.raises(ValueError):
+        config.remove_item_from_config({}, 'a.b.c', 'e')
+
+    with pytest.raises(ValueError):
+        config.remove_item_from_config({'a': 'b'}, 'a.b', 'e')
+
+    with pytest.raises(ValueError):
+        config.remove_item_from_config({'a': ['b']}, 'a', 'e')
+
+
 def test_show_config():
     """
     Test stdout output when showing config

tljh/config.py

@@ -60,7 +60,6 @@ def add_item_to_config(config, property_path, value):
     for i, cur_path in enumerate(path_components):
         if i == len(path_components) - 1:
             # Final component, it must be a list and we append to it
-            print('l', cur_path, cur_part)
             if cur_path not in cur_part or not isinstance(cur_part[cur_path], list):
                 cur_part[cur_path] = []
             cur_part = cur_part[cur_path]
@@ -69,13 +68,34 @@ def add_item_to_config(config, property_path, value):
         else:
             # If we are asked to create new non-leaf nodes, we will always make them dicts
             # This means setting is *destructive* - will replace whatever is down there!
-            print('p', cur_path, cur_part)
             if cur_path not in cur_part or not isinstance(cur_part[cur_path], dict):
                 cur_part[cur_path] = {}
             cur_part = cur_part[cur_path]
 
     return config_copy
 
+def remove_item_from_config(config, property_path, value):
+    """
+    Add an item to a list in config.
+    """
+    path_components = property_path.split('.')
+
+    # Mutate a copy of the config, not config itself
+    cur_part = config_copy = deepcopy(config)
+    for i, cur_path in enumerate(path_components):
+        if i == len(path_components) - 1:
+            # Final component, it must be a list and we append to it
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], list):
+                raise ValueError(f'{property_path} is not a list')
+            cur_part = cur_part[cur_path]
+            cur_part.remove(value)
+        else:
+            if cur_path not in cur_part or not isinstance(cur_part[cur_path], dict):
+                raise ValueError(f'{property_path} does not exist in config!')
+            cur_part = cur_part[cur_path]
+
+    return config_copy
+
 
 def show_config(config_path):
     """
@@ -173,11 +193,24 @@ def main():
     )
     add_item_parser.add_argument(
         'key_path',
-        help='Dot separated path to configuration key to set'
+        help='Dot separated path to configuration key to add value to'
     )
     add_item_parser.add_argument(
         'value',
-        help='Value ot set the configuration key to'
+        help='Value to add to the configuration key'
+    )
+
+    remove_item_parser = subparsers.add_parser(
+        'remove-item',
+        help='Remove a value from a list for a configuration property'
+    )
+    remove_item_parser.add_argument(
+        'key_path',
+        help='Dot separated path to configuration key to remove value from'
+    )
+    remove_item_parser.add_argument(
+        'value',
+        help='Value to remove from key_path'
     )
 
     reload_parser = subparsers.add_parser(
@@ -200,6 +233,8 @@ def main():
         set_config_value(args.config_path, args.key_path, args.value)
     elif args.action == 'add-item':
         add_config_value(args.config_path, args.key_path, args.value)
+    elif args.action == 'remove-item':
+        add_config_value(args.config_path, args.key_path, args.value)
     elif args.action == 'reload':
         reload_component(args.component)