Ensure unicity when creating instances of Autogroup (#3650)

The default label of an `Autogroup` instance was based on the current
date and time, with a precision of seconds. If multiple scripts attempted
to create an instance at the same time, a uniqueness error would be thrown
by the database. This commit adds the logic to the `get_or_create_group`
method to be able to deal with this situtation and iteratively generate
unique labels by appending an integer.

The CLI parameters of the `verdi run` command were broken and badly
tested. These have been altered so as to fix the behavior:

 * removed the `--group` flag, which was true by default and so served
   no purpose whatsoever, and replaced it with `--auto-group`. The auto
   grouping is now disabled by default, but can be enabled by this new
   flag.
 * deprecated use of `group-name' in favour of `group-label-prefix`
 * the flags to include or exclude certain node types have been merged
   into just `--exclude` and `--include` which are mutually exclusive.
   By default everything is included and the flags take entry point
   strings of nodes to narrow the scope. The entry point string can
   contain SQL wildcards `%` and `_` to match entry points with the
   `like` operator.

The `Autogroup` interface has been adapted to match the new logic of the
CLI. Finally, an overzealous `isinstance` check has been removed from the
`Node.store` method to check that the `current_autogroup` global, if set,
is of type `Autogroup`, which should speed up the storing of nodes.

Author: giovannipizzi

.ci/workchains.py

@@ -68,8 +68,8 @@ def a_magic_unicorn_appeared(self, node):
     @process_handler(priority=400, exit_codes=ArithmeticAddCalculation.exit_codes.ERROR_NEGATIVE_NUMBER)
     def error_negative_sum(self, node):
         """What even is a negative number, how can I have minus three melons?!."""
-        self.ctx.inputs.x = Int(abs(node.inputs.x.value))
-        self.ctx.inputs.y = Int(abs(node.inputs.y.value))
+        self.ctx.inputs.x = Int(abs(node.inputs.x.value))  # pylint: disable=invalid-name
+        self.ctx.inputs.y = Int(abs(node.inputs.y.value))  # pylint: disable=invalid-name
         return ProcessHandlerReport(True)
 
 

.gitignore

@@ -20,6 +20,7 @@
 .cache
 .pytest_cache
 .coverage
+coverage.xml
 
 # Files created by RPN tests
 .ci/polish/polish_workchains/polish*

.pre-commit-config.yaml

@@ -53,7 +53,6 @@
         aiida/common/datastructures.py|
         aiida/engine/daemon/execmanager.py|
         aiida/engine/processes/calcjobs/tasks.py|
-        aiida/orm/autogroup.py|
         aiida/orm/querybuilder.py|
         aiida/orm/nodes/data/array/bands.py|
         aiida/orm/nodes/data/array/projection.py|
@@ -66,7 +65,6 @@
         aiida/parsers/plugins/arithmetic/add.py|
         aiida/parsers/plugins/templatereplacer/doubler.py|
         aiida/parsers/plugins/templatereplacer/__init__.py|
-        aiida/plugins/entry_point.py|
         aiida/plugins/entry.py|
         aiida/plugins/info.py|
         aiida/plugins/registry.py|

aiida/backends/testbase.py

@@ -99,7 +99,11 @@ def tearDown(self):
 
     def reset_database(self):
         """Reset the database to the default state deleting any content currently stored"""
+        from aiida.orm import autogroup
+
         self.clean_db()
+        if autogroup.CURRENT_AUTOGROUP is not None:
+            autogroup.CURRENT_AUTOGROUP.clear_group_cache()
         self.insert_data()
 
     @classmethod
@@ -109,7 +113,10 @@ def insert_data(cls):
         inserts default data into the database (which is for the moment a
         default computer).
         """
+        from aiida.orm import User
+
         cls.create_user()
+        User.objects.reset()
         cls.create_computer()
 
     @classmethod
@@ -180,7 +187,11 @@ def user_email(cls):  # pylint: disable=no-self-argument
     def tearDownClass(cls, *args, **kwargs):  # pylint: disable=arguments-differ
         # Double check for double security to avoid to run the tearDown
         # if this is not a test profile
+        from aiida.orm import autogroup
+
         check_if_tests_can_run()
+        if autogroup.CURRENT_AUTOGROUP is not None:
+            autogroup.CURRENT_AUTOGROUP.clear_group_cache()
         cls.clean_db()
         cls.clean_repository()
         cls.__backend_instance.tearDownClass_method(*args, **kwargs)

aiida/cmdline/commands/cmd_plugin.py

@@ -13,7 +13,7 @@
 
 from aiida.cmdline.commands.cmd_verdi import verdi
 from aiida.cmdline.utils import decorators, echo
-from aiida.plugins.entry_point import entry_point_group_to_module_path_map
+from aiida.plugins.entry_point import ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP
 
 
 @verdi.group('plugin')
@@ -22,7 +22,7 @@ def verdi_plugin():
 
 
 @verdi_plugin.command('list')
-@click.argument('entry_point_group', type=click.Choice(entry_point_group_to_module_path_map.keys()), required=False)
+@click.argument('entry_point_group', type=click.Choice(ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP.keys()), required=False)
 @click.argument('entry_point', type=click.STRING, required=False)
 @decorators.with_dbenv()
 def plugin_list(entry_point_group, entry_point):
@@ -34,7 +34,7 @@ def plugin_list(entry_point_group, entry_point):
 
     if entry_point_group is None:
         echo.echo_info('Available entry point groups:')
-        for group in sorted(entry_point_group_to_module_path_map.keys()):
+        for group in sorted(ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP.keys()):
             echo.echo('* {}'.format(group))
 
         echo.echo('')

aiida/cmdline/commands/cmd_run.py

@@ -10,13 +10,16 @@
 """`verdi run` command."""
 import contextlib
 import os
+import functools
 import sys
+import warnings
 
 import click
 
 from aiida.cmdline.commands.cmd_verdi import verdi
 from aiida.cmdline.params.options.multivalue import MultipleValueOption
 from aiida.cmdline.utils import decorators, echo
+from aiida.common.warnings import AiidaDeprecationWarning
 
 
 @contextlib.contextmanager
@@ -37,31 +40,56 @@ def update_environment(argv):
         sys.path = _path
 
 
+def validate_entrypoint_string(ctx, param, value):  # pylint: disable=unused-argument,invalid-name
+    """Validate that `value` is a valid entrypoint string."""
+    from aiida.orm import autogroup
+
+    try:
+        autogroup.Autogroup.validate(value)
+    except Exception as exc:
+        raise click.BadParameter(str(exc) + ' ({})'.format(value))
+
+    return value
+
+
 @verdi.command('run', context_settings=dict(ignore_unknown_options=True,))
 @click.argument('scriptname', type=click.STRING)
 @click.argument('varargs', nargs=-1, type=click.UNPROCESSED)
-@click.option('-g', '--group', is_flag=True, default=True, show_default=True, help='Enables the autogrouping')
-@click.option('-n', '--group-name', type=click.STRING, required=False, help='Specify the name of the auto group')
-@click.option('-e', '--exclude', cls=MultipleValueOption, default=[], help='Exclude these classes from auto grouping')
+@click.option('--auto-group', is_flag=True, help='Enables the autogrouping')
+@click.option(
+    '-l',
+    '--auto-group-label-prefix',
+    type=click.STRING,
+    required=False,
+    help='Specify the prefix of the label of the auto group (numbers might be automatically '
+    'appended to generate unique names per run).'
+)
 @click.option(
-    '-i', '--include', cls=MultipleValueOption, default=['all'], help='Include these classes from auto grouping'
+    '-n',
+    '--group-name',
+    type=click.STRING,
+    required=False,
+    help='Specify the name of the auto group [DEPRECATED, USE --auto-group-label-prefix instead]. '
+    'This also enables auto-grouping.'
 )
 @click.option(
-    '-E',
-    '--excludesubclasses',
+    '-e',
+    '--exclude',
     cls=MultipleValueOption,
-    default=[],
-    help='Exclude these classes and their sub classes from auto grouping'
+    default=None,
+    help='Exclude these classes from auto grouping (use full entrypoint strings).',
+    callback=functools.partial(validate_entrypoint_string)
 )
 @click.option(
-    '-I',
-    '--includesubclasses',
+    '-i',
+    '--include',
     cls=MultipleValueOption,
-    default=[],
-    help='Include these classes and their sub classes from auto grouping'
+    default=None,
+    help='Include these classes from auto grouping  (use full entrypoint strings or "all").',
+    callback=validate_entrypoint_string
 )
 @decorators.with_dbenv()
-def run(scriptname, varargs, group, group_name, exclude, excludesubclasses, include, includesubclasses):
+def run(scriptname, varargs, auto_group, auto_group_label_prefix, group_name, exclude, include):
     # pylint: disable=too-many-arguments,exec-used
     """Execute scripts with preloaded AiiDA environment."""
     from aiida.cmdline.utils.shell import DEFAULT_MODULES_LIST
@@ -80,22 +108,27 @@ def run(scriptname, varargs, group, group_name, exclude, excludesubclasses, incl
     for app_mod, model_name, alias in DEFAULT_MODULES_LIST:
         globals_dict['{}'.format(alias)] = getattr(__import__(app_mod, {}, {}, model_name), model_name)
 
-    if group:
-        automatic_group_name = group_name
-        if automatic_group_name is None:
-            from aiida.common import timezone
-
-            automatic_group_name = 'Verdi autogroup on ' + timezone.now().strftime('%Y-%m-%d %H:%M:%S')
+    if group_name:
+        warnings.warn('--group-name is deprecated, use `--auto-group-label-prefix` instead', AiidaDeprecationWarning)  # pylint: disable=no-member
+        if auto_group_label_prefix:
+            raise click.BadParameter(
+                'You cannot specify both --group-name and --auto-group-label-prefix; '
+                'use --auto-group-label-prefix only'
+            )
+        auto_group_label_prefix = group_name
+        # To have the old behavior, with auto-group enabled.
+        auto_group = True
 
+    if auto_group:
         aiida_verdilib_autogroup = autogroup.Autogroup()
+        # Set the ``group_label_prefix`` if defined, otherwise a default prefix will be used
+        if auto_group_label_prefix is not None:
+            aiida_verdilib_autogroup.set_group_label_prefix(auto_group_label_prefix)
         aiida_verdilib_autogroup.set_exclude(exclude)
         aiida_verdilib_autogroup.set_include(include)
-        aiida_verdilib_autogroup.set_exclude_with_subclasses(excludesubclasses)
-        aiida_verdilib_autogroup.set_include_with_subclasses(includesubclasses)
-        aiida_verdilib_autogroup.set_group_name(automatic_group_name)
 
         # Note: this is also set in the exec environment! This is the intended behavior
-        autogroup.current_autogroup = aiida_verdilib_autogroup
+        autogroup.CURRENT_AUTOGROUP = aiida_verdilib_autogroup
 
     # Initialize the variable here, otherwise we get UnboundLocalError in the finally clause if it fails to open
     handle = None

aiida/engine/processes/calcjobs/calcjob.py

@@ -64,7 +64,7 @@ def validate_calc_job(inputs, ctx):
         )
 
 
-def validate_parser(parser_name, ctx):
+def validate_parser(parser_name, ctx):  # pylint: disable=unused-argument
     """Validate the parser.
 
     :raises InputValidationError: if the parser name does not correspond to a loadable `Parser` class.
@@ -78,7 +78,7 @@ def validate_parser(parser_name, ctx):
             raise exceptions.InputValidationError('invalid parser specified: {}'.format(exception))
 
 
-def validate_resources(resources, ctx):
+def validate_resources(resources, ctx):  # pylint: disable=unused-argument
     """Validate the resources.
 
     :raises InputValidationError: if `num_machines` is not specified or is not an integer.

aiida/manage/caching.py

@@ -22,7 +22,7 @@
 from aiida.common import exceptions
 from aiida.common.lang import type_check
 
-from aiida.plugins.entry_point import ENTRY_POINT_STRING_SEPARATOR, entry_point_group_to_module_path_map
+from aiida.plugins.entry_point import ENTRY_POINT_STRING_SEPARATOR, ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP
 
 __all__ = ('get_use_cache', 'enable_caching', 'disable_caching')
 
@@ -248,7 +248,7 @@ def _validate_identifier_pattern(*, identifier):
 
     1.  <group_name><ENTRY_POINT_STRING_SEPARATOR><tail>
 
-        where `group_name` is one of the keys in `entry_point_group_to_module_path_map`
+        where `group_name` is one of the keys in `ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP`
         and `tail` can be anything _except_ `ENTRY_POINT_STRING_SEPARATOR`.
 
     2. a fully qualified Python name
@@ -276,7 +276,7 @@ def _validate_identifier_pattern(*, identifier):
         group_pattern, _ = identifier.split(ENTRY_POINT_STRING_SEPARATOR)
         if not any(
             _match_wildcard(string=group_name, pattern=group_pattern)
-            for group_name in entry_point_group_to_module_path_map
+            for group_name in ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP
         ):
             raise ValueError(
                 common_error_msg + "Group name pattern '{}' does not match any of the AiiDA entry point group names.".
@@ -290,7 +290,7 @@ def _validate_identifier_pattern(*, identifier):
     # aiida.* or aiida.calculations*
     if '*' in identifier:
         group_part, _ = identifier.split('*', 1)
-        if any(group_name.startswith(group_part) for group_name in entry_point_group_to_module_path_map):
+        if any(group_name.startswith(group_part) for group_name in ENTRY_POINT_GROUP_TO_MODULE_PATH_MAP):
             return
     # Finally, check if it could be a fully qualified Python name
     for identifier_part in identifier.split('.'):