From 220b8296a94af2191bbc707675ff4ae0ab217b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Thu, 27 Apr 2023 12:20:50 +0200 Subject: [PATCH] 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. --- .../20230427_121619_regis_config_append.md | 1 + docs/configuration.rst | 11 +-- tests/commands/test_config.py | 40 ++++++++++ tutor/commands/config.py | 74 +++++++++++++++++-- 4 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 changelog.d/20230427_121619_regis_config_append.md diff --git a/changelog.d/20230427_121619_regis_config_append.md b/changelog.d/20230427_121619_regis_config_append.md new file mode 100644 index 0000000..6d50791 --- /dev/null +++ b/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) diff --git a/docs/configuration.rst b/docs/configuration.rst index 244133e..02e5ad9 100644 --- a/docs/configuration.rst +++ b/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 ` 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 `. For instance, to include the `polling xblock from Opencraft `_: -- 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`. 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 diff --git a/tests/commands/test_config.py b/tests/commands/test_config.py index 314d8fe..f344897 100644 --- a/tests/commands/test_config.py +++ b/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: diff --git a/tutor/commands/config.py b/tutor/commands/config.py index 19d7ce8..72d1d2b 100644 --- a/tutor/commands/config.py +++ b/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: