From 8a286f26e8d303c498ac2eabd49be5f1f4ced9ef Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Thu, 8 Feb 2024 14:07:36 +0100 Subject: [PATCH] CLI: Fix the `ctx.obj.profile` attribute not being initialized (#6279) The `verdi` CLI uses a custom context class `VerdiContext`, which in turn provides a custom implementation for the `obj` property. This dictionary is primarily designed to lazily load the config and assigning it to the `config` attribute. Besides the `config`, the `ctx.obj` can also be used to retrieve the active profile for a `verdi` command. However, this is not initialized by the context itself, but is left to the `ProfileParamType`. This means though that `ctx.obj.profile` can raise an `AttributeError` if no profile is set, for example when a config defines no profiles whatsoever. This behavior was causing `verdi config set --global` to fail for configs without profiles, because it calls `ctx.obj.profile` which would raise an `AttributeError`. Although this was being tested for in `tests/cmdline/commands/test_config.py:test_config_set_option_no_profile` the bug was missed because the `run_cli_command` fixture would manually customize the `ctx.obj` instance to always define the `profile` attribute, even if it was `None`. The `LazyConfigAttributeDict` is updated to not just initialize the `config` key, but also initialize `profile`, which is set to `None` if it doesn't already exist. Since it now handles multiple special keys, it is renamed to `LazyVerdiObjAttributeDict`. The `run_cli_command_runner` function, called by the `run_cli_command` fixture is updated to also use the `LazyVerdiObjAttributeDict` class, instead of the plain `AttributeDict`, and makes sure to only define the `profile` attribute if it is defined. This ensures that the pathway through the test runner is similar to an actual invocation of `verdi`. --- src/aiida/cmdline/groups/verdi.py | 26 ++++++++++++++++---------- tests/conftest.py | 6 ++++-- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/aiida/cmdline/groups/verdi.py b/src/aiida/cmdline/groups/verdi.py index 1f106cb95c..08c9abc856 100644 --- a/src/aiida/cmdline/groups/verdi.py +++ b/src/aiida/cmdline/groups/verdi.py @@ -34,33 +34,39 @@ ) -class LazyConfigAttributeDict(AttributeDict): - """Subclass of ``AttributeDict`` that lazily calls :meth:`aiida.manage.configuration.get_config`.""" +class LazyVerdiObjAttributeDict(AttributeDict): + """Subclass of ``AttributeDict`` that lazily initializes the ``config`` and ``profile`` attributes. - _LAZY_KEY = 'config' + This class guarantees that the ``config`` and ``profile`` attributes never raise an ``AttributeError``. When the + attributes are accessed when they are not set, ``config`` is initialized by the value returned by the method + :meth:`aiida.manage.configuration.get_config`. The ``profile`` attribute is initialized to ``None``. + """ + + _KEY_CONFIG = 'config' + _KEY_PROFILE = 'profile' def __init__(self, ctx: click.Context, dictionary: dict[str, t.Any] | None = None): super().__init__(dictionary) self.ctx = ctx def __getattr__(self, attr: str) -> t.Any: - """Override of ``AttributeDict.__getattr__`` for lazily loading the config key. + """Override of ``AttributeDict.__getattr__`` to lazily initialize the ``config`` and ``profile`` attributes. :param attr: The attribute to return. :returns: The value of the attribute. :raises AttributeError: If the attribute does not correspond to an existing key. :raises click.exceptions.UsageError: If loading of the configuration fails. """ - if attr != self._LAZY_KEY: - return super().__getattr__(attr) + if attr == self._KEY_PROFILE: + self.setdefault(self._KEY_PROFILE, None) - if self._LAZY_KEY not in self: + elif attr == self._KEY_CONFIG and self._KEY_CONFIG not in self: try: - self[self._LAZY_KEY] = get_config(create=True) + self[self._KEY_CONFIG] = get_config(create=True) except ConfigurationError as exception: self.ctx.fail(str(exception)) - return self[self._LAZY_KEY] + return super().__getattr__(attr) class VerdiContext(click.Context): @@ -69,7 +75,7 @@ class VerdiContext(click.Context): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if self.obj is None: - self.obj = LazyConfigAttributeDict(self) + self.obj = LazyVerdiObjAttributeDict(self) class VerdiCommandGroup(click.Group): diff --git a/tests/conftest.py b/tests/conftest.py index a907489b29..d015acd045 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -603,13 +603,15 @@ def run_cli_command_subprocess(command, parameters, user_input, profile_name, su def run_cli_command_runner(command, parameters, user_input, initialize_ctx_obj, kwargs): """Run CLI command through ``click.testing.CliRunner``.""" from aiida.cmdline.commands.cmd_verdi import VerdiCommandGroup - from aiida.common import AttributeDict + from aiida.cmdline.groups.verdi import LazyVerdiObjAttributeDict from click.testing import CliRunner if initialize_ctx_obj: config = get_config() profile = get_profile() - obj = AttributeDict({'config': config, 'profile': profile}) + obj = LazyVerdiObjAttributeDict(None, {'config': config}) + if profile is not None: + obj.profile = profile else: obj = None