CLI: Validate strict in verdi config set caching.disabled_for (#6197)

Values provided for `verdi config set caching.disabled_for` and `verdi
config set caching.enabled_for` were validated, but without setting
`strict=True` for the `_validate_identifier_pattern` validator. This
keyword was recently added and when set to `True` would actually attempt
to load the entry point or import path.

Without this strict validation, a user could accidentally set an
incorrect value to enable or disable caching for. Most common example is
where a correct entry point was provided but without the entry point
group, e.g., `core.arithmetic.add`. The validator would interpret this
as an import path and not as an entry point, and since strict validation
was turned off, it didn't attempt to actually import it.

The `strict` validation is now turned on causing the example above to
return a validation error. The downside of this approach is that it is
no longer possible to disable or enable caching for an entry point or
import path that cannot be loaded in the current environment. But then
again, the question is what the use would be of such a use-case.

aiida/manage/configuration/config.py

@@ -132,7 +132,7 @@ def validate_caching_identifier_pattern(cls, value: List[str]) -> List[str]:
         """Validate the caching identifier patterns."""
         from aiida.manage.caching import _validate_identifier_pattern
         for identifier in value:
-            _validate_identifier_pattern(identifier=identifier)
+            _validate_identifier_pattern(identifier=identifier, strict=True)
         return value
 
 

aiida/manage/configuration/options.py

@@ -71,7 +71,14 @@ def validate(self, value: Any) -> Any:
                 GlobalOptionsSchema.model_construct(), attribute, value
             )
         except ValidationError as exception:
-            raise ConfigurationError(str(exception)) from exception
+            messages = []
+            for error in exception.errors():
+                try:
+                    messages.append(str(error['ctx']['error']))
+                except KeyError:
+                    messages.append(f"Invalid value for `{error['loc'][0]}`: {error['msg']}")
+
+            raise ConfigurationError('\n'.join(messages)) from exception
 
         # Return the value from the constructed model as this will have casted the value to the right type
         return getattr(result, attribute)

docs/source/howto/run_codes.rst

@@ -844,8 +844,19 @@ Besides the on/off switch set by ``caching.default_enabled``, caching can be con
 In this example, caching is enabled by default, but explicitly disabled for calculations of the ``TemplatereplacerCalculation`` class, identified by its corresponding ``aiida.calculations:core.templatereplacer`` entry point string.
 It also shows how to enable caching for particular calculations (which has no effect here due to the profile-wide default).
 
+.. note::
+
+    It is also possible to provide the full import path of the class in case it does not have a registered entry point.
+    For example, if the class can be imported as ``from some.module.path import SomeClass``, it can be added as:
+
+    .. code-block:: console
+
+        verdi config set caching.enabled_for some.module.path.SomeClass
+
 .. tip:: To set multiple entry-points at once, use a ``,`` delimiter.
 
+.. warning:: If a specified value for ``verdi config set enabled_for/disabled_for`` is an entry point that cannot be loaded, or is a path that cannot be improted, an error is raised.
+
 For the available entry-points in your environment, you can list which are enabled/disabled using:
 
 .. code-block:: console

tests/cmdline/commands/test_config.py

@@ -39,36 +39,39 @@ def test_config_set_option_no_profile(run_cli_command, empty_config):
 def test_config_set_option(run_cli_command, config_with_profile_factory, option_name, is_list):
     """Test the `verdi config set` command when setting an option."""
     config = config_with_profile_factory()
-
-    for option_value in ['value0', 'value1']:
-        options = ['config', 'set', option_name, option_value]
-        run_cli_command(cmd_verdi.verdi, options, use_subprocess=False)
-        if is_list:
-            assert config.get_option(option_name, scope=get_profile().name) == [option_value]
-        else:
-            assert str(config.get_option(option_name, scope=get_profile().name)) == option_value
+    option_value = 'aiida.calculations:core.arithmetic.add'
+    options = ['config', 'set', option_name, option_value]
+    run_cli_command(cmd_verdi.verdi, options, use_subprocess=False)
+    if is_list:
+        assert config.get_option(option_name, scope=get_profile().name) == [option_value]
+    else:
+        assert str(config.get_option(option_name, scope=get_profile().name)) == option_value
 
 
 def test_config_append_option(run_cli_command, config_with_profile_factory):
     """Test the `verdi config set --append` command when appending an option value."""
     config = config_with_profile_factory()
+    prefix = 'aiida.calculations:core.'
     option_name = 'caching.enabled_for'
-    for value in ['x', 'y', 'x', 'y']:
-        options = ['config', 'set', '--append', option_name, value]
+    for value in ['transfer', 'arithmetic.add', 'transfer', 'arithmetic.add']:
+        options = ['config', 'set', '--append', option_name, f'{prefix}{value}']
         run_cli_command(cmd_verdi.verdi, options, use_subprocess=False)
-    assert sorted(config.get_option(option_name, scope=get_profile().name)) == ['x', 'y']
+    assert sorted(config.get_option(option_name,
+                                    scope=get_profile().name)) == [f'{prefix}arithmetic.add', f'{prefix}transfer']
 
 
 def test_config_remove_option(run_cli_command, config_with_profile_factory):
     """Test the `verdi config set --remove` command when removing an option value."""
     config = config_with_profile_factory()
 
     option_name = 'caching.disabled_for'
-    config.set_option(option_name, ['x', 'x', 'y', 'x'], scope=get_profile().name)
+    prefix = 'aiida.calculations:core.'
+    option_value = [f'{prefix}{value}' for value in ('transfer', 'transfer', 'arithmetic.add', 'transfer')]
+    config.set_option(option_name, option_value, scope=get_profile().name)
 
-    options = ['config', 'set', '--remove', option_name, 'x']
+    options = ['config', 'set', '--remove', option_name, f'{prefix}transfer']
     run_cli_command(cmd_verdi.verdi, options, use_subprocess=False)
-    assert config.get_option(option_name, scope=get_profile().name) == ['y']
+    assert config.get_option(option_name, scope=get_profile().name) == [f'{prefix}arithmetic.add']
 
 
 def test_config_get_option(run_cli_command, config_with_profile_factory):
@@ -174,6 +177,26 @@ def test_config_caching(run_cli_command, config_with_profile_factory):
     assert result.output.strip() == ''
 
 
+@pytest.mark.parametrize(
+    'value, raises', (
+        ('aiida.calculations:core.arithmetic.add', False),
+        ('aiida.calculations:core.arithmetic.invalid', True),
+        ('core.arithmetic.invalid', True),
+        ('aiida.calculations.arithmetic.add.ArithmeticAddCalculation', False),
+        ('aiida.calculations.arithmetic.invalid.ArithmeticAddCalculation', True),
+    )
+)
+def test_config_set_caching_enabled(run_cli_command, config_with_profile_factory, value, raises):
+    """Test `verdi config set caching.enabled_for`"""
+    config_with_profile_factory()
+    options = ['config', 'set', 'caching.enabled_for', value]
+    result = run_cli_command(cmd_verdi.verdi, options, raises=raises, use_subprocess=False)
+    if raises:
+        assert 'Critical: Invalid identifier pattern' in result.output
+    else:
+        assert 'Success: \'caching.enabled_for\' set to' in result.output
+
+
 def test_config_downgrade(run_cli_command, config_with_profile_factory):
     """Test `verdi config downgrade`"""
     config_with_profile_factory()

tests/manage/test_caching_config.py

@@ -72,7 +72,7 @@ def test_merge_deprecated_yaml(tmp_path):
         cache_dictionary = {
             'default': {
                 'default': True,
-                'enabled': ['aiida.calculations:quantumespresso.pw'],
+                'enabled': ['aiida.calculations:core.arithmetic.add'],
                 'disabled': ['aiida.calculations:core.templatereplacer']
             }
         }
@@ -83,7 +83,7 @@ def test_merge_deprecated_yaml(tmp_path):
         load_profile('default')
 
         assert get_config_option('caching.default_enabled') is True
-        assert get_config_option('caching.enabled_for') == ['aiida.calculations:quantumespresso.pw']
+        assert get_config_option('caching.enabled_for') == ['aiida.calculations:core.arithmetic.add']
         assert get_config_option('caching.disabled_for') == ['aiida.calculations:core.templatereplacer']
         # should have now been moved to cache_config.yml.<DATETIME>
         assert not tmp_path.joinpath('cache_config.yml').exists()
@@ -236,18 +236,18 @@ def test_enable_caching_global(configure_caching):
     """
     Check that using enable_caching for a specific identifier works.
     """
-    specific_identifier = 'some_ident'
+    specific_identifier = 'aiida.calculations.arithmetic.add.ArithmeticAddCalculation'
     with configure_caching(config_dict={'default_enabled': False, 'disabled_for': [specific_identifier]}):
         with enable_caching():
-            assert get_use_cache(identifier='some_other_ident')
+            assert get_use_cache(identifier='aiida.calculations.transfer.TransferCalculation')
             assert get_use_cache(identifier=specific_identifier)
 
 
 def test_disable_caching_specific(configure_caching):
     """
     Check that using disable_caching for a specific identifier works.
     """
-    identifier = 'some_ident'
+    identifier = 'aiida.calculations.arithmetic.add.ArithmeticAddCalculation'
     with configure_caching({'default_enabled': True}):
         with disable_caching(identifier=identifier):
             assert not get_use_cache(identifier=identifier)
@@ -257,10 +257,10 @@ def test_disable_caching_global(configure_caching):
     """
     Check that using disable_caching for a specific identifier works.
     """
-    specific_identifier = 'some_ident'
+    specific_identifier = 'aiida.calculations.arithmetic.add.ArithmeticAddCalculation'
     with configure_caching(config_dict={'default_enabled': True, 'enabled_for': [specific_identifier]}):
         with disable_caching():
-            assert not get_use_cache(identifier='some_other_ident')
+            assert not get_use_cache(identifier='aiida.calculations.transfer.TransferCalculation')
             assert not get_use_cache(identifier=specific_identifier)