feat: config save --append/--remove KEY=VAL options

This paves the way for persisting bind-mounts across runs, since this
setting will be a list.

changelog.d/20230427_121619_regis_config_append.md

@@ -0,0 +1 @@
+- [Feature] Add a `config save -a/--append -A/--remove` options to conveniently append and remove values to/from list entries. (by @regisb)

docs/configuration.rst

@@ -359,19 +359,12 @@ Installing extra xblocks and requirements
 
 Would you like to include custom xblocks, or extra requirements to your Open edX platform? Additional requirements can be added to the ``OPENEDX_EXTRA_PIP_REQUIREMENTS`` parameter in the :ref:`config file <configuration>` or to the ``env/build/openedx/requirements/private.txt`` file. The difference between them, is that ``private.txt`` file, even though it could be used for both, :ref:`should be used for installing extra xblocks or requirements from private repositories <extra_private_xblocks>`. For instance, to include the `polling xblock from Opencraft <https://github.com/open-craft/xblock-poll/>`_:
 
-- add the following to the ``config.yml``::
+    tutor config save --append OPENEDX_EXTRA_PIP_REQUIREMENTS=git+https://github.com/open-craft/xblock-poll.git
 
-    OPENEDX_EXTRA_PIP_REQUIREMENTS:
-    - "git+https://github.com/open-craft/xblock-poll.git"
-
-.. warning::
-   Specifying extra requirements through ``config.yml`` overwrites :ref:`the default extra requirements<openedx_extra_pip_requirements>`. You might need to add them to the list if your configuration depends on them.
-
-- or add the dependency to ``private.txt``::
+Alternatively, add the dependency to ``private.txt``::
 
     echo "git+https://github.com/open-craft/xblock-poll.git" >> "$(tutor config printroot)/env/build/openedx/requirements/private.txt"
 
-
 Then, the ``openedx`` docker image must be rebuilt::
 
     tutor images build openedx

tests/commands/test_config.py

@@ -1,5 +1,6 @@
 import unittest
 
+from tutor import config as tutor_config
 from tests.helpers import temporary_root
 
 from .base import TestCommandMixin
@@ -59,6 +60,45 @@ class ConfigTests(unittest.TestCase, TestCommandMixin):
         self.assertEqual(0, result.exit_code)
         self.assertTrue(result.output)
 
+    def test_config_append(self) -> None:
+        with temporary_root() as root:
+            self.invoke_in_root(
+                root, ["config", "save", "--append=TEST=value"], catch_exceptions=False
+            )
+            config1 = tutor_config.load(root)
+            self.invoke_in_root(
+                root, ["config", "save", "--append=TEST=value"], catch_exceptions=False
+            )
+            config2 = tutor_config.load(root)
+            self.invoke_in_root(
+                root, ["config", "save", "--remove=TEST=value"], catch_exceptions=False
+            )
+            config3 = tutor_config.load(root)
+        # Value is appended
+        self.assertEqual(["value"], config1["TEST"])
+        # Value is not appended a second time
+        self.assertEqual(["value"], config2["TEST"])
+        # Value is removed
+        self.assertEqual([], config3["TEST"])
+
+    def test_config_append_with_existing_default(self) -> None:
+        with temporary_root() as root:
+            self.invoke_in_root(
+                root,
+                [
+                    "config",
+                    "save",
+                    "--append=OPENEDX_EXTRA_PIP_REQUIREMENTS=my-package==1.0.0",
+                ],
+                catch_exceptions=False,
+            )
+            config = tutor_config.load(root)
+        assert isinstance(config["OPENEDX_EXTRA_PIP_REQUIREMENTS"], list)
+        self.assertEqual(2, len(config["OPENEDX_EXTRA_PIP_REQUIREMENTS"]))
+        self.assertEqual(
+            "my-package==1.0.0", config["OPENEDX_EXTRA_PIP_REQUIREMENTS"][1]
+        )
+
 
 class PatchesTests(unittest.TestCase, TestCommandMixin):
     def test_config_patches_list(self) -> None:

tutor/commands/config.py

@@ -34,9 +34,8 @@ class ConfigKeyParamType(click.ParamType):
             for key, _value in self._shell_complete_config_items(ctx, incomplete)
         ]
 
-    @staticmethod
     def _shell_complete_config_items(
-        ctx: click.Context, incomplete: str
+        self, ctx: click.Context, incomplete: str
     ) -> list[tuple[str, ConfigValue]]:
         # Here we want to auto-complete the name of the config key. For that we need to
         # figure out the list of enabled plugins, and for that we need the project root.
@@ -48,9 +47,16 @@ class ConfigKeyParamType(click.ParamType):
         ).get("root", "")
         config = tutor_config.load_full(root)
         return [
-            (key, value) for key, value in config.items() if key.startswith(incomplete)
+            (key, value)
+            for key, value in self._candidate_config_items(config)
+            if key.startswith(incomplete)
         ]
 
+    def _candidate_config_items(
+        self, config: Config
+    ) -> t.Iterable[tuple[str, ConfigValue]]:
+        yield from config.items()
+
 
 class ConfigKeyValParamType(ConfigKeyParamType):
     """
@@ -91,6 +97,19 @@ class ConfigKeyValParamType(ConfigKeyParamType):
         return []
 
 
+class ConfigListKeyValParamType(ConfigKeyValParamType):
+    """
+    Same as the parent class, but for keys of type `list`.
+    """
+
+    def _candidate_config_items(
+        self, config: Config
+    ) -> t.Iterable[tuple[str, ConfigValue]]:
+        for key, val in config.items():
+            if isinstance(val, list):
+                yield key, val
+
+
 @click.command(help="Create and save configuration interactively")
 @click.option("-i", "--interactive", is_flag=True, help="Run interactively")
 @click.option(
@@ -102,6 +121,24 @@ class ConfigKeyValParamType(ConfigKeyParamType):
     metavar="KEY=VAL",
     help="Set a configuration value (can be used multiple times)",
 )
+@click.option(
+    "-a",
+    "--append",
+    "append_vars",
+    type=ConfigListKeyValParamType(),
+    multiple=True,
+    metavar="KEY=VAL",
+    help="Append an item to a configuration value of type list. The value will only be added it it is not already present. (can be used multiple times)",
+)
+@click.option(
+    "-A",
+    "--remove",
+    "remove_vars",
+    type=ConfigListKeyValParamType(),
+    multiple=True,
+    metavar="KEY=VAL",
+    help="Remove an item from a configuration value of type list (can be used multiple times)",
+)
 @click.option(
     "-U",
     "--unset",
@@ -117,16 +154,43 @@ class ConfigKeyValParamType(ConfigKeyParamType):
 def save(
     context: Context,
     interactive: bool,
-    set_vars: Config,
+    set_vars: tuple[str, t.Any],
+    append_vars: tuple[str, t.Any],
+    remove_vars: tuple[str, t.Any],
     unset_vars: list[str],
     env_only: bool,
 ) -> None:
     config = tutor_config.load_minimal(context.root)
+    config_full = tutor_config.load_full(context.root)
     if interactive:
         interactive_config.ask_questions(config)
     if set_vars:
-        for key, value in dict(set_vars).items():
+        for key, value in set_vars:
             config[key] = env.render_unknown(config, value)
+    if append_vars:
+        for key, value in append_vars:
+            if key not in config:
+                config[key] = config_full.get(key, [])
+            values = config[key]
+            if not isinstance(values, list):
+                raise exceptions.TutorError(
+                    f"Could not append value to '{key}': current setting is of type '{values.__class__.__name__}', expected list."
+                )
+            if not isinstance(value, str):
+                raise exceptions.TutorError(
+                    f"Could not append value to '{key}': appended value is of type '{value.__class__.__name__}', expected str."
+                )
+            if value not in values:
+                values.append(value)
+    if remove_vars:
+        for key, value in remove_vars:
+            values = config.get(key, [])
+            if not isinstance(values, list):
+                raise exceptions.TutorError(
+                    f"Could not remove value from '{key}': current setting is of type '{values.__class__.__name__}', expected list."
+                )
+            while value in values:
+                values.remove(value)
     for key in unset_vars:
         config.pop(key, None)
     if not env_only: