diff --git a/jupyterhub/app.py b/jupyterhub/app.py index db47ab69..1258f01f 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -298,9 +298,9 @@ class JupyterHub(Application): return classes load_groups = Dict( - Dict(), + Union([Dict(), List()]), help=""" - Dict of `{'group': {'users':['usernames'], properties : {}}` to load at startup. + Dict of `{'group': {'users':['usernames'], 'properties': {}}` to load at startup. Example:: @@ -311,7 +311,8 @@ class JupyterHub(Application): }, } - This strictly *adds* groups, users and properties to groups. + This strictly *adds* groups and users to groups. + Properties, if defined, replace all existing properties. Loading one set of groups, then starting JupyterHub again with a different set will not remove users or groups from previous launches. @@ -2079,12 +2080,18 @@ class JupyterHub(Application): for username in contents['users']: username = self.authenticator.normalize_username(username) user = await self._get_or_create_user(username) - self.log.debug(f"Adding user {username} to group {name}") - group.users.append(user) + if group not in user.groups: + self.log.debug(f"Adding user {username} to group {name}") + group.users.append(user) + if 'properties' in contents: group_properties = contents['properties'] - self.log.debug(f"Adding properties {group_properties} to group {name}") - group.properties = group_properties + if group.properties != group_properties: + # add equality check to avoid no-op db transactions + self.log.debug( + f"Adding properties to group {name}: {group_properties}" + ) + group.properties = group_properties db.commit() diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 2a524e62..a26d191f 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -233,15 +233,16 @@ def test_cookie_secret_string_(): async def test_load_groups(tmpdir, request): + to_load = { 'blue': { 'users': ['cyclops', 'rogue', 'wolverine'], - 'properties': {'setting1': 'one', 'setting2': 'two'}, }, 'gold': { 'users': ['storm', 'jean-grey', 'colossus'], 'properties': {'setting3': 'three', 'setting4': 'four'}, }, + 'deprecated_list': ['jubilee', 'magik'], } kwargs = {'load_groups': to_load} ssl_enabled = getattr(request.module, "ssl_enabled", False) @@ -258,11 +259,17 @@ async def test_load_groups(tmpdir, request): blue = orm.Group.find(db, name='blue') assert blue is not None assert sorted(u.name for u in blue.users) == sorted(to_load['blue']['users']) - assert sorted(u for u in blue.properties) == sorted(to_load['blue']['properties']) + assert blue.properties == {} gold = orm.Group.find(db, name='gold') assert gold is not None assert sorted(u.name for u in gold.users) == sorted(to_load['gold']['users']) - assert sorted(u for u in gold.properties) == sorted(to_load['gold']['properties']) + assert gold.properties == to_load['gold']['properties'] + deprecated_list = orm.Group.find(db, name='deprecated_list') + assert deprecated_list is not None + assert deprecated_list.properties == {} + assert sorted(u.name for u in deprecated_list.users) == sorted( + to_load['deprecated_list'] + ) async def test_resume_spawners(tmpdir, request):