diff --git a/CHANGELOG.md b/CHANGELOG.md index e66b24829..6ea6f6b07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and **Merged pull requests**. Critical items to know are: The versions coincide with releases on pip. Only major versions will be released as tags on Github. ## [0.0.x](https://github.com/singularityhub/singularity-hpc/tree/main) (0.0.x) + - fixing bug to config set/get nested fields, default for wrapper scripts true (0.0.57) + - change in behavior, instead of `key:value` for set/add `key value` now also works. - adding support for container.yaml overrides (0.0.56) - `SINGULARITY_CONTAINER` and `PODMAN_CONTAINER` support (0.0.55) - support for views create, delete, edit, install, uninstall (0.0.54) diff --git a/docs/getting_started/installation.rst b/docs/getting_started/installation.rst index 6bfd9756d..c34bcd918 100644 --- a/docs/getting_started/installation.rst +++ b/docs/getting_started/installation.rst @@ -80,9 +80,9 @@ variables in the config: .. code-block:: console - $ shpc config add registry:/ - $ shpc config set module_base:/ - $ shpc config set container_base:/ + $ shpc config add registry / + $ shpc config set module_base / + $ shpc config set container_base / Also importantly, if you are using environment modules (Tcl) and not LMOD, you need @@ -90,7 +90,7 @@ to tell shpc about this (as it defaults to LMOD): .. code-block:: console - $ shpc config set module_sys:tcl + $ shpc config set module_sys tcl You can also easily (manually) update any settings in the ``shpc/settings.yaml`` file: diff --git a/docs/getting_started/user-guide.rst b/docs/getting_started/user-guide.rst index 0210acd16..f49f005b2 100644 --- a/docs/getting_started/user-guide.rst +++ b/docs/getting_started/user-guide.rst @@ -40,8 +40,8 @@ default). .. code-block:: console - $ shpc config set module_sys:tcl - $ shpc config set module_sys:lmod # default + $ shpc config set module_sys tcl + $ shpc config set module_sys lmod # default You can then easily install, load, and use modules: @@ -81,8 +81,8 @@ software, which is typicall lmod or tcl. Here is how to change from the default .. code-block:: console - $ shpc config set module_sys:tcl - $ shpc config set module_sys:lmod # this is the default, which we change back to! + $ shpc config set module_sys tcl + $ shpc config set module_sys lmod # this is the default, which we change back to! Once you have the correct module software indicated, try installing a container: @@ -315,10 +315,10 @@ your install: .. code-block:: console # an absolute path - $ shpc config set module_base:/opt/lmod/modules + $ shpc config set module_base /opt/lmod/modules # or a path relative to a variable location remember to escape the "$" - $ shpc config set module_base:\$root_dir/modules + $ shpc config set module_base \$root_dir/modules This directory will be the base where lua files are added, and containers are stored @@ -351,7 +351,7 @@ different. For example: .. code-block:: console $ mkdir -p /tmp/containers - $ shpc config set container_base:/tmp/containers + $ shpc config set container_base /tmp/containers The same hierarchy will be preserved as to not put all containers in the same @@ -464,7 +464,7 @@ or you can set the global variable to what you want to use (it defaults to lmod) .. code-block:: console - $ shpc config set module_sys:tcl + $ shpc config set module_sys tcl The command line argument, if provided, always over-rides the default. @@ -488,7 +488,7 @@ or for a global setting: .. code-block:: console - $ shpc config set container_tech:podman + $ shpc config set container_tech podman If you would like support for a different container technology that has not been @@ -520,6 +520,26 @@ Container-specific scripts you'll want to include in the container.yaml are desc # use for singularity aliases singularity: singularity.sh +Since these are nested values, to get the current value you can use a ``:`` to separate +the fields, e.g.,: + +.. code-block:: console + + $ shpc config get wrapper_scripts:enabled + wrapper_scripts:enabled False + +And if you want to change the default, just add another level: + +.. code-block:: console + + $ shpc config set wrapper_scripts:enabled true + Updated wrapper_scripts:enabled to be true + +And don't forget you can manually update the file in an editor: + +.. code-block:: console + + $ shpc config edit Since different container technologies might expose different environment variables (e.g., ``SINGULARITY_OPTS`` vs ``PODMAN_OPTS``) they are organized above based on the container technology. If you want to customize the wrapper script, simply replace the relative paths @@ -688,7 +708,7 @@ If you always want to install to an (existing) named view, simply set the ``defa .. code-block:: console - $ shpc config set default_view:mpi + $ shpc config set default_view mpi You should obviously create the view first or you'll get an error message that it does not exist! When you have a default view set, any install that you do will install to the module base and also your view. @@ -717,7 +737,7 @@ for a short period of time to install to it. If you want to disable this, then j .. code-block:: console - $ shpc config set default_view:null + $ shpc config set default_view null And note you can also ask to install to a view "one off": @@ -908,10 +928,10 @@ The following example shows changing the default module_base path from the insta .. code-block:: console # an absolute path - $ shpc config set module_base:/opt/lmod/modules + $ shpc config set module_base /opt/lmod/modules # or a path relative to the install directory, remember to escape the "$" - $ shpc config set module_base:\$install_dir/modules + $ shpc config set module_base \$install_dir/modules And then to get values: @@ -925,8 +945,8 @@ And to add and remove a value to a list: .. code-block:: console - $ shpc config add registry:/tmp/registry - $ shpc config remove registry:/tmp/registry + $ shpc config add registry /tmp/registry + $ shpc config remove registry /tmp/registry You can also open the config in the editor defined in settings at ``config_editor`` @@ -1081,7 +1101,7 @@ Note that you can also set the namespace as any other setting: .. code-block:: console - $ shpc config set namespace:ghcr.io/autamus + $ shpc config set namespace ghcr.io/autamus Namespaces currently work with: diff --git a/shpc/client/__init__.py b/shpc/client/__init__.py index 83d7f3b2a..2414f5b53 100644 --- a/shpc/client/__init__.py +++ b/shpc/client/__init__.py @@ -217,12 +217,13 @@ def get_parser(): "params", nargs="*", help="""Set or get a config value, edit the config, add or remove a list variable, or create a user-specific config. -shpc config set key:value +shpc config set key value +shpc config set key:subkey value shpc config get key shpc edit shpc config inituser -shpc config add registry:/tmp/registry -shpc config remove registry:/tmp/registry""", +shpc config add registry /tmp/registry +shpc config remove registry /tmp/registry""", type=str, ) # Generate markdown docs for a container registry entry diff --git a/shpc/client/config.py b/shpc/client/config.py index ad224a131..3ad39ef71 100644 --- a/shpc/client/config.py +++ b/shpc/client/config.py @@ -35,10 +35,7 @@ def main(args, parser, extra, subparser): return cli.settings.edit() if command in ["set", "add", "remove"]: - - # Update each param - for param in args.params: - cli.settings.update_param(command, param) + cli.settings.update_param(command, args.params) # Save settings cli.settings.save() @@ -47,7 +44,7 @@ def main(args, parser, extra, subparser): elif command == "get": for key in args.params: value = cli.settings.get(key) - value = value or "is unset" + value = "is unset" if value is None else value logger.info("%s %s" % (key.ljust(30), value)) else: diff --git a/shpc/main/settings.py b/shpc/main/settings.py index dac78f547..277d17cb3 100644 --- a/shpc/main/settings.py +++ b/shpc/main/settings.py @@ -202,7 +202,10 @@ def set(self, key, value): """ Set a setting based on key and value. If the key has :, it's nested """ - value = self.parse_boolean(value) + while ":" in key: + value = str(value) + key, extra = key.split(":", 1) + value = f"{extra}:{value}" # List values not allowed for set current = self._settings.get(key) @@ -210,12 +213,14 @@ def set(self, key, value): logger.exit("You cannot use 'set' for a list. Use add/remove instead.") # This is a reference to a dictionary (object) setting + # We assume only one level of nesting allowed if isinstance(value, str) and ":" in value: subkey, value = value.split(":") value = self.parse_boolean(value) value = self.parse_null(value) self._settings[key][subkey] = value else: + value = self.parse_boolean(value) value = self.parse_null(value) self._settings[key] = value @@ -296,13 +301,27 @@ def update_param(self, command, param): """ Given a parameter, update the configuration on the fly if it's in set/add/remove """ - if ":" not in param: - logger.warning( - "Param %s is missing a :, should be key:value pair. Skipping." % param - ) - return + # If we are given a list, assume is key and value at end + if isinstance(param, list): + # If one given, assume old format + if len(param) == 1: + param = param[0] + elif len(param) == 2: + key, value = param + elif len(param) != 2: + logger.exit( + f"When providing a list, it must be a [key, value]. Found {param}" + ) + + # With a string, assume splittling by : + if isinstance(param, str): + if ":" not in param: + logger.exit( + "Param %s is missing a :, should be key:value pair. Skipping." + % param + ) + key, value = param.rsplit(":", 1) - key, value = param.split(":", 1) if command == "set": self.set(key, value) logger.info("Updated %s to be %s" % (key, value)) diff --git a/shpc/settings.yml b/shpc/settings.yml index 557695697..703359384 100644 --- a/shpc/settings.yml +++ b/shpc/settings.yml @@ -70,7 +70,7 @@ wrapper_shell: /bin/bash # If you want a custom wrapper script for a one-off container, define it there with the script # stored alongside the container.yaml. wrapper_scripts: - enabled: false + enabled: true # use for docker aliases (set to null to disable) docker: docker.sh @@ -99,3 +99,4 @@ container_features: # defaults to ~/.Xauthority if set to true and the container has x11: true home: # one of null, or a single path or src:dest path. # home: true in a container.yaml will use this path, if defines +updated_at: '2022-07-08T11:31:49Z' diff --git a/shpc/tests/test_config.py b/shpc/tests/test_config.py new file mode 100644 index 000000000..eebd3991d --- /dev/null +++ b/shpc/tests/test_config.py @@ -0,0 +1,122 @@ +#!/usr/bin/python + +# Copyright (C) 2022 Vanessa Sochat. +# This Source Code Form is subject to the terms of the +# Mozilla Public License, v. 2.0. If a copy of the MPL was not distributed +# with this file, You can obtain one at http://mozilla.org/MPL/2.0/. + +from .helpers import init_client +import pytest + + +def test_config(tmp_path): + """ + Test config get, set, etc. + """ + client = init_client(str(tmp_path), "lmod", "singularity") + + # Set/get one level + assert not client.settings.get("container_features:gpu") + client.settings.set("container_features", "gpu:nvidia") + assert client.settings.get("container_features:gpu") == "nvidia" + + # Set null + client.settings.set("container_features:gpu", None) + assert not client.settings.get("container_features:gpu") + + client.settings.set("container_features:gpu", "null") + assert not client.settings.get("container_features:gpu") + + client.settings.set("container_features:gpu", "none") + assert not client.settings.get("container_features:gpu") + + # Boolean + assert client.settings.get("wrapper_scripts:enabled") == True + client.settings.set("wrapper_scripts:enabled", False) + assert client.settings.get("wrapper_scripts:enabled") == False + client.settings.set("wrapper_scripts:enabled", "false") + assert client.settings.get("wrapper_scripts:enabled") == False + client.settings.set("wrapper_scripts:enabled", "TRUE") + assert client.settings.get("wrapper_scripts:enabled") == True + + +@pytest.mark.parametrize( + "command,name,param,default,result", + [ + ( + "set", + "container_features:gpu", + "container_features:gpu:nvidia", + None, + "nvidia", + ), + ( + "set", + "container_features:gpu", + ["container_features:gpu", "nvidia"], + None, + "nvidia", + ), + ( + "set", + "wrapper_scripts:templates", + "wrapper_scripts:templates:value", + None, + "value", + ), + ( + "set", + "wrapper_scripts:templates", + ["wrapper_scripts:templates", "value"], + None, + "value", + ), + ], +) +def test_update_param(tmp_path, command, name, param, default, result): + """ + Test general update_param used by client + """ + client = init_client(str(tmp_path), "lmod", "singularity") + + # Set/get one level + assert client.settings.get(name) == default + client.settings.update_param(command, param) + assert client.settings.get(name) == result + + +def test_add_remove(tmp_path): + """ + Test add/remove (from list) + """ + client = init_client(str(tmp_path), "lmod", "singularity") + + # Default is a single list + registry = client.settings.get("registry") + assert isinstance(registry, list) + assert len(registry) == 1 + assert registry[0].endswith("registry") + + # Add a registry location + client.settings.update_param("add", ["registry", "/tmp/registry"]) + registry = client.settings.get("registry") + assert len(registry) == 2 + assert "/tmp/registry" in registry + + # Remove from list + client.settings.update_param("remove", ["registry", "/tmp/registry"]) + registry = client.settings.get("registry") + assert len(registry) == 1 + assert "/tmp/registry" not in registry + + # Add a registry location (old format) + client.settings.update_param("add", "registry:/tmp/registry") + registry = client.settings.get("registry") + assert len(registry) == 2 + assert "/tmp/registry" in registry + + # Remove a registry location (old format) + client.settings.update_param("remove", "registry:/tmp/registry") + registry = client.settings.get("registry") + assert len(registry) == 1 + assert "/tmp/registry" not in registry