From b5cea43ce1554f9ab0bf8f012f8ef15de90cdc7a Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 15:48:19 +0100 Subject: [PATCH 01/19] add code --- aiida/cmdline/commands/cmd_code.py | 23 +++- aiida/cmdline/commands/cmd_group.py | 36 +++++- aiida/cmdline/commands/cmd_node.py | 28 +++-- aiida/manage/database/delete/nodes.py | 168 ++++++++++++++++++-------- tests/cmdline/commands/test_node.py | 5 + tests/test_nodes.py | 79 ++++-------- 6 files changed, 216 insertions(+), 123 deletions(-) diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index caf2f7e46c..bf65998615 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -9,6 +9,7 @@ ########################################################################### """`verdi code` command.""" from functools import partial +import logging import click import tabulate @@ -192,16 +193,28 @@ def delete(codes, verbose, dry_run, force): Note that codes are part of the data provenance, and deleting a code will delete all calculations using it. """ - from aiida.manage.database.delete.nodes import delete_nodes + from aiida.common.log import override_log_formatter_context + from aiida.manage.database.delete.nodes import delete_nodes, DELETE_LOGGER - verbosity = 1 + verbosity = logging.INFO if force: - verbosity = 0 + verbosity = logging.WARNING elif verbose: - verbosity = 2 + verbosity = logging.DEBUG node_pks_to_delete = [code.pk for code in codes] - delete_nodes(node_pks_to_delete, dry_run=dry_run, verbosity=verbosity, force=force) + + def _dry_run_callback(pks): + if dry_run: + return True + if force: + return False + echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') + return not click.confirm('Shall I continue?') + + DELETE_LOGGER.setLevel(verbosity) + with override_log_formatter_context('%(message)s'): + delete_nodes(node_pks_to_delete, dry_run=_dry_run_callback) @verdi_code.command() diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index ee773b7daf..05cc6548cc 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -9,6 +9,7 @@ ########################################################################### """`verdi group` commands""" import warnings +import logging import click from aiida.common.exceptions import UniquenessError @@ -17,6 +18,7 @@ from aiida.cmdline.params import options, arguments from aiida.cmdline.utils import echo from aiida.cmdline.utils.decorators import with_dbenv +from aiida.common.links import GraphTraversalRules @verdi.group('group') @@ -66,22 +68,50 @@ def group_remove_nodes(group, nodes, clear, force): ' [deprecated: No longer has any effect. Will be removed in 2.0.0]' ) @options.FORCE() +@click.option( + '--delete-nodes', is_flag=True, default=False, help='Delete all nodes in the group along with the group itself.' +) +@options.graph_traversal_rules(GraphTraversalRules.DELETE.value) +@options.DRY_RUN() +@options.VERBOSE() @with_dbenv() -def group_delete(group, clear, force): +def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversal_rules): """Delete a group. Note that this command only deletes groups - nodes contained in the group will remain untouched. """ + from aiida.common.log import override_log_formatter_context + from aiida.manage.database.delete.nodes import delete_group_nodes, DELETE_LOGGER from aiida import orm - label = group.label - if clear: warnings.warn('`--clear` is deprecated and no longer has any effect.', AiidaDeprecationWarning) # pylint: disable=no-member + label = group.label + verbosity = logging.INFO + if force: + verbosity = logging.WARNING + elif verbose: + verbosity = logging.DEBUG + DELETE_LOGGER.setLevel(verbosity) + if not force: click.confirm(f'Are you sure to delete Group<{label}>?', abort=True) + if delete_nodes: + + def _dry_run_callback(pks): + if not pks or force: + return False + echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') + return not click.confirm('Shall I continue?') + + with override_log_formatter_context('%(message)s'): + _, nodes_deleted = delete_group_nodes([group.pk], dry_run=dry_run or _dry_run_callback, **traversal_rules) + if not nodes_deleted: + # don't delete the group if the nodes were not deleted + return + orm.Group.objects.delete(group.pk) echo.echo_success(f'Group<{label}> deleted.') diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 51ab802d37..a5e22a0632 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -9,6 +9,7 @@ ########################################################################### """`verdi node` command.""" +import logging import shutil import pathlib @@ -302,32 +303,41 @@ def tree(nodes, depth): @options.FORCE() @options.graph_traversal_rules(GraphTraversalRules.DELETE.value) @with_dbenv() -def node_delete(identifier, dry_run, verbose, force, **kwargs): +def node_delete(identifier, dry_run, verbose, force, **traversal_rules): """Delete nodes from the provenance graph. This will not only delete the nodes explicitly provided via the command line, but will also include the nodes necessary to keep a consistent graph, according to the rules outlined in the documentation. You can modify some of those rules using options of this command. """ + from aiida.common.log import override_log_formatter_context from aiida.orm.utils.loaders import NodeEntityLoader - from aiida.manage.database.delete.nodes import delete_nodes + from aiida.manage.database.delete.nodes import delete_nodes, DELETE_LOGGER - verbosity = 1 + verbosity = logging.INFO if force: - verbosity = 0 + verbosity = logging.WARNING elif verbose: - verbosity = 2 + verbosity = logging.DEBUG + DELETE_LOGGER.setLevel(verbosity) pks = [] for obj in identifier: # we only load the node if we need to convert from a uuid/label - if isinstance(obj, int): - pks.append(obj) - else: + try: + pks.append(int(obj)) + except ValueError: pks.append(NodeEntityLoader.load_entity(obj).pk) - delete_nodes(pks, dry_run=dry_run, verbosity=verbosity, force=force, **kwargs) + def _dry_run_callback(pks): + if not pks or force: + return False + echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') + return not click.confirm('Shall I continue?') + + with override_log_formatter_context('%(message)s'): + delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) @verdi_node.command('rehash') diff --git a/aiida/manage/database/delete/nodes.py b/aiida/manage/database/delete/nodes.py index e9c89a6828..c6ea5bb539 100644 --- a/aiida/manage/database/delete/nodes.py +++ b/aiida/manage/database/delete/nodes.py @@ -7,16 +7,27 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -"""Function to delete nodes from the database.""" -from typing import Iterable +"""Functions to delete entities from the database.""" +import logging +from typing import Callable, Iterable, Optional, Set, Tuple, Union +import warnings -import click -from aiida.cmdline.utils import echo +from aiida.backends.utils import delete_nodes_and_connections +from aiida.common.log import AIIDA_LOGGER +from aiida.common.warnings import AiidaDeprecationWarning +from aiida.orm import Group, Node, QueryBuilder, load_node +from aiida.tools.graph.graph_traversers import get_nodes_delete + +DELETE_LOGGER = AIIDA_LOGGER.getChild('delete') def delete_nodes( - pks: Iterable[int], verbosity: int = 0, dry_run: bool = False, force: bool = False, **traversal_rules: bool -): + pks: Iterable[int], + verbosity: Optional[int] = None, + dry_run: Union[bool, Callable[[Set[int]], bool]] = True, + force: Optional[bool] = None, + **traversal_rules: bool +) -> Tuple[Set[int], bool]: """Delete nodes by a list of pks. This command will delete not only the specified nodes, but also the ones that are @@ -38,87 +49,138 @@ def delete_nodes( nodes will be deleted as well, and then any CALC node that may have those as inputs, and so on. - :param pks: a list of the PKs of the nodes to delete - :param force: do not ask for confirmation to delete nodes. - :param verbosity: 0 prints nothing, - 1 prints just sums and total, - 2 prints individual nodes. + :param pks: a list of starting PKs of the nodes to delete + (the full set will be based on the traversal rules) :param dry_run: - Just perform a dry run and do not delete anything. - Print statistics according to the verbosity level set. - :param force: Do not ask for confirmation to delete nodes + If True, return the pks to delete without deleting anything. + If False, delete the pks without confirmation + If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` :param traversal_rules: graph traversal rules. See :const:`aiida.common.links.GraphTraversalRules` what rule names are toggleable and what the defaults are. + + :returns: (pks to delete, whether they were deleted) + """ # pylint: disable=too-many-arguments,too-many-branches,too-many-locals,too-many-statements - from aiida.backends.utils import delete_nodes_and_connections - from aiida.orm import Node, QueryBuilder, load_node - from aiida.tools.graph.graph_traversers import get_nodes_delete + + if verbosity is not None: + warnings.warn( + 'The verbosity option is deprecated and will be removed in `aiida-core==2.0.0`. ' + 'Set the level of DELETE_LOGGER instead', AiidaDeprecationWarning + ) # pylint: disable=no-member + + if force is not None: + warnings.warn( + 'The force option is deprecated and will be removed in `aiida-core==2.0.0`. ' + 'Use dry_run instead', AiidaDeprecationWarning + ) # pylint: disable=no-member + if force is True: + dry_run = False def _missing_callback(_pks: Iterable[int]): for _pk in _pks: - echo.echo_warning(f'warning: node with pk<{_pk}> does not exist, skipping') + DELETE_LOGGER.warning(f'warning: node with pk<{_pk}> does not exist, skipping') pks_set_to_delete = get_nodes_delete(pks, get_links=False, missing_callback=_missing_callback, **traversal_rules)['nodes'] - # An empty set might be problematic for the queries done below. - if not pks_set_to_delete: - if verbosity: - echo.echo('Nothing to delete') - return - - if verbosity > 0: - echo.echo( - 'I {} delete {} node{}'.format( - 'would' if dry_run else 'will', len(pks_set_to_delete), 's' if len(pks_set_to_delete) > 1 else '' - ) - ) - if verbosity > 1: + DELETE_LOGGER.info('Deleting %s node(s)', len(pks_set_to_delete)) + + if pks_set_to_delete and DELETE_LOGGER.level == logging.DEBUG: builder = QueryBuilder().append( Node, filters={'id': { 'in': pks_set_to_delete }}, project=('uuid', 'id', 'node_type', 'label') ) - echo.echo(f"The nodes I {'would' if dry_run else 'will'} delete:") + DELETE_LOGGER.debug('Node(s) to delete:') for uuid, pk, type_string, label in builder.iterall(): try: short_type_string = type_string.split('.')[-2] except IndexError: short_type_string = type_string - echo.echo(f' {uuid} {pk} {short_type_string} {label}') - - if dry_run: - if verbosity > 0: - echo.echo('\nThis was a dry run, exiting without deleting anything') - return - - # Asking for user confirmation here - if force: - pass - else: - echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks_set_to_delete)} NODES! THIS CANNOT BE UNDONE!') - if not click.confirm('Shall I continue?'): - echo.echo('Exiting without deleting') - return + DELETE_LOGGER.debug(f' {uuid} {pk} {short_type_string} {label}') + + if dry_run is True: + DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') + return (pks_set_to_delete, False) + + # confirm deletion + if callable(dry_run) and dry_run(pks_set_to_delete): + DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') + return (pks_set_to_delete, False) + + if not pks_set_to_delete: + return (pks_set_to_delete, True) # Recover the list of folders to delete before actually deleting the nodes. I will delete the folders only later, # so that if there is a problem during the deletion of the nodes in the DB, I don't delete the folders repositories = [load_node(pk)._repository for pk in pks_set_to_delete] # pylint: disable=protected-access - if verbosity > 0: - echo.echo('Starting node deletion...') + DELETE_LOGGER.info('Starting node deletion...') delete_nodes_and_connections(pks_set_to_delete) - if verbosity > 0: - echo.echo('Nodes deleted from database, deleting files from the repository now...') + DELETE_LOGGER.info('Nodes deleted from database, deleting files from the repository now...') # If we are here, we managed to delete the entries from the DB. # I can now delete the folders for repository in repositories: repository.erase(force=True) - if verbosity > 0: - echo.echo('Deletion completed.') + DELETE_LOGGER.info('Deletion of nodes completed.') + + return (pks_set_to_delete, True) + + +def delete_group_nodes( + pks: Iterable[int], + dry_run: Union[bool, Callable[[Set[int]], bool]] = True, + **traversal_rules: bool +) -> Tuple[Set[int], bool]: + """Delete nodes contained in a list of groups (not the groups themselves!). + + This command will delete not only the nodes, but also the ones that are + linked to these and should be also deleted in order to keep a consistent provenance + according to the rules explained in the concepts section of the documentation. + In summary: + + 1. If a DATA node is deleted, any process nodes linked to it will also be deleted. + + 2. If a CALC node is deleted, any incoming WORK node (callers) will be deleted as + well whereas any incoming DATA node (inputs) will be kept. Outgoing DATA nodes + (outputs) will be deleted by default but this can be disabled. + + 3. If a WORK node is deleted, any incoming WORK node (callers) will be deleted as + well, but all DATA nodes will be kept. Outgoing WORK or CALC nodes will be kept by + default, but deletion of either of both kind of connected nodes can be enabled. + + These rules are 'recursive', so if a CALC node is deleted, then its output DATA + nodes will be deleted as well, and then any CALC node that may have those as + inputs, and so on. + + :param pks: a list of the groups + + :param dry_run: + If True, return the pks to delete without deleting anything. + If False, delete the pks without confirmation + If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` + + :param traversal_rules: graph traversal rules. See :const:`aiida.common.links.GraphTraversalRules` what rule names + are toggleable and what the defaults are. + + :returns: (node pks to delete, whether they were deleted) + + """ + group_node_query = QueryBuilder().append( + Group, + filters={ + 'id': { + 'in': list(pks) + } + }, + tag='groups', + ).append(Node, project='id', with_group='groups') + group_node_query.distinct() + node_pks = group_node_query.all(flat=True) + return delete_nodes(node_pks, dry_run=dry_run, **traversal_rules) diff --git a/tests/cmdline/commands/test_node.py b/tests/cmdline/commands/test_node.py index a99de1b532..619241e9b0 100644 --- a/tests/cmdline/commands/test_node.py +++ b/tests/cmdline/commands/test_node.py @@ -651,3 +651,8 @@ def test_basics(self): with self.assertRaises(NotExistent): orm.load_node(newnodepk) + + def test_missing_pk(self): + """Check that no exception is raised when a non-existent pk is given (just warns).""" + result = self.cli_runner.invoke(cmd_node.node_delete, ['999']) + self.assertClickResultNoException(result) diff --git a/tests/test_nodes.py b/tests/test_nodes.py index f032336afb..c1f45e4d65 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -18,7 +18,6 @@ from aiida.backends.testbase import AiidaTestCase from aiida.common.exceptions import InvalidOperation, ModificationNotAllowed, StoringNotAllowed, ValidationError from aiida.common.links import LinkType -from aiida.common.utils import Capturing from aiida.manage.database.delete.nodes import delete_nodes @@ -1544,8 +1543,7 @@ def _check_existence(self, uuids_check_existence, uuids_check_deleted): def test_deletion_non_existing_pk(): """Verify that passing a non-existing pk should not raise.""" non_existing_pk = -1 - with Capturing(): - delete_nodes([non_existing_pk], force=True) + delete_nodes([non_existing_pk], dry_run=False) # TEST BASIC CASES @@ -1642,15 +1640,13 @@ def test_delete_cases(self): di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di]] uuids_check_deleted = [n.uuid for n in [dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([w1.pk], force=True) + delete_nodes([w1.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di]] uuids_check_deleted = [n.uuid for n in [dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([w2.pk], force=True) + delete_nodes([w2.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # By default, targetting a calculation will have the same effect because @@ -1659,15 +1655,13 @@ def test_delete_cases(self): di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di]] uuids_check_deleted = [n.uuid for n in [dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([c1.pk], force=True) + delete_nodes([c1.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di]] uuids_check_deleted = [n.uuid for n in [dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([c2.pk], force=True) + delete_nodes([c2.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # By default, targetting a data node will also have the same effect because @@ -1676,22 +1670,19 @@ def test_delete_cases(self): di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in []] uuids_check_deleted = [n.uuid for n in [di, dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([di.pk], force=True) + delete_nodes([di.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di]] uuids_check_deleted = [n.uuid for n in [dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([dm.pk], force=True) + delete_nodes([dm.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di]] uuids_check_deleted = [n.uuid for n in [dm, do, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([do.pk], force=True) + delete_nodes([do.pk], dry_run=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # Data deletion within the highest level workflow can be prevented by @@ -1700,15 +1691,13 @@ def test_delete_cases(self): di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di, dm, do]] uuids_check_deleted = [n.uuid for n in [c1, c2, w1, w2]] - with Capturing(): - delete_nodes([w2.pk], force=True, create_forward=False) + delete_nodes([w2.pk], dry_run=False, create_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [dm, do]] uuids_check_deleted = [n.uuid for n in [di, c1, c2, w1, w2]] - with Capturing(): - delete_nodes([di.pk], force=True, create_forward=False) + delete_nodes([di.pk], dry_run=False, create_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # On the other hand, the whole data provenance can be protected by @@ -1719,15 +1708,13 @@ def test_delete_cases(self): di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di, dm, do, c1, c2]] uuids_check_deleted = [n.uuid for n in [w1, w2]] - with Capturing(): - delete_nodes([w2.pk], force=True, call_calc_forward=False) + delete_nodes([w2.pk], dry_run=False, call_calc_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di, dm, c1]] uuids_check_deleted = [n.uuid for n in [do, c2, w1, w2]] - with Capturing(): - delete_nodes([c2.pk], force=True, call_calc_forward=False) + delete_nodes([c2.pk], dry_run=False, call_calc_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # Another posibility which also exists, though may have more limited @@ -1739,22 +1726,19 @@ def test_delete_cases(self): di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di, dm, c1, w1]] uuids_check_deleted = [n.uuid for n in [do, c2, w2]] - with Capturing(): - delete_nodes([w2.pk], force=True, call_work_forward=False) + delete_nodes([w2.pk], dry_run=False, call_work_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di, dm, do, c1, w1]] uuids_check_deleted = [n.uuid for n in [c2, w2]] - with Capturing(): - delete_nodes([w2.pk], force=True, call_work_forward=False, create_forward=False) + delete_nodes([w2.pk], dry_run=False, call_work_forward=False, create_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) di, dm, do, c1, c2, w1, w2 = self._create_simple_graph() uuids_check_existence = [n.uuid for n in [di, dm, do, c1, c2, w1]] uuids_check_deleted = [n.uuid for n in [w2]] - with Capturing(): - delete_nodes([w2.pk], force=True, call_work_forward=False, call_calc_forward=False) + delete_nodes([w2.pk], dry_run=False, call_work_forward=False, call_calc_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) @staticmethod @@ -1850,15 +1834,13 @@ def test_indep2w(self): dia, doa, pca, pwa, dib, dob, pcb, pwb, pw0 = self._create_indep2w_graph() uuids_check_existence = [n.uuid for n in [dia, dib]] uuids_check_deleted = [n.uuid for n in [doa, pca, pwa, dob, pcb, pwb, pw0]] - with Capturing(): - delete_nodes((pca.pk,), force=True, create_forward=True, call_calc_forward=True, call_work_forward=True) + delete_nodes((pca.pk,), dry_run=False, create_forward=True, call_calc_forward=True, call_work_forward=True) self._check_existence(uuids_check_existence, uuids_check_deleted) dia, doa, pca, pwa, dib, dob, pcb, pwb, pw0 = self._create_indep2w_graph() uuids_check_existence = [n.uuid for n in [dia, dib]] uuids_check_deleted = [n.uuid for n in [doa, pca, pwa, dob, pcb, pwb, pw0]] - with Capturing(): - delete_nodes((pwa.pk,), force=True, create_forward=True, call_calc_forward=True, call_work_forward=True) + delete_nodes((pwa.pk,), dry_run=False, create_forward=True, call_calc_forward=True, call_work_forward=True) self._check_existence(uuids_check_existence, uuids_check_deleted) # In this particular case where the workflow (pwa) only calls a single @@ -1872,15 +1854,13 @@ def test_indep2w(self): dia, doa, pca, pwa, dib, dob, pcb, pwb, pw0 = self._create_indep2w_graph() uuids_check_existence = [n.uuid for n in [dia, dib, dob, pcb, pwb]] uuids_check_deleted = [n.uuid for n in [doa, pca, pwa, pw0]] - with Capturing(): - delete_nodes((pca.pk,), force=True, create_forward=True, call_calc_forward=True, call_work_forward=False) + delete_nodes((pca.pk,), dry_run=False, create_forward=True, call_calc_forward=True, call_work_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) dia, doa, pca, pwa, dib, dob, pcb, pwb, pw0 = self._create_indep2w_graph() uuids_check_existence = [n.uuid for n in [dia, dib, dob, pcb, pwb]] uuids_check_deleted = [n.uuid for n in [doa, pca, pwa, pw0]] - with Capturing(): - delete_nodes((pwa.pk,), force=True, create_forward=True, call_calc_forward=True, call_work_forward=False) + delete_nodes((pwa.pk,), dry_run=False, create_forward=True, call_calc_forward=True, call_work_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # The best and most controlled way to deal with this situation would be @@ -1897,12 +1877,10 @@ def test_indep2w(self): uuids_check_existence2 = [n.uuid for n in [dia, dib, dob, pcb, pwb]] uuids_check_deleted2 = [n.uuid for n in [doa, pca, pwa, pw0]] - with Capturing(): - delete_nodes((pw0.pk,), force=True, create_forward=False, call_calc_forward=False, call_work_forward=False) + delete_nodes((pw0.pk,), dry_run=False, create_forward=False, call_calc_forward=False, call_work_forward=False) self._check_existence(uuids_check_existence1, uuids_check_deleted1) - with Capturing(): - delete_nodes((pwa.pk,), force=True, create_forward=True, call_calc_forward=True, call_work_forward=True) + delete_nodes((pwa.pk,), dry_run=False, create_forward=True, call_calc_forward=True, call_work_forward=True) self._check_existence(uuids_check_existence2, uuids_check_deleted2) @staticmethod @@ -1972,8 +1950,7 @@ def test_loop_cases(self): di1, di2, di3, do1, pws, pcs, pwm = self._create_looped_graph() uuids_check_existence = [n.uuid for n in [di1, di2]] uuids_check_deleted = [n.uuid for n in [di3, do1, pcs, pws, pwm]] - with Capturing(): - delete_nodes([di3.pk], force=True, create_forward=True, call_calc_forward=True, call_work_forward=True) + delete_nodes([di3.pk], dry_run=False, create_forward=True, call_calc_forward=True, call_work_forward=True) self._check_existence(uuids_check_existence, uuids_check_deleted) # When disabling the call_calc and call_work forward rules, deleting @@ -1982,8 +1959,7 @@ def test_loop_cases(self): di1, di2, di3, do1, pws, pcs, pwm = self._create_looped_graph() uuids_check_existence = [n.uuid for n in [di1, di2, do1, pcs]] uuids_check_deleted = [n.uuid for n in [di3, pws, pwm]] - with Capturing(): - delete_nodes([di3.pk], force=True, create_forward=True, call_calc_forward=False, call_work_forward=False) + delete_nodes([di3.pk], dry_run=False, create_forward=True, call_calc_forward=False, call_work_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # Of course, deleting the selected input will cause all the procedure to @@ -1991,8 +1967,7 @@ def test_loop_cases(self): di1, di2, di3, do1, pws, pcs, pwm = self._create_looped_graph() uuids_check_existence = [n.uuid for n in [di2, di3]] uuids_check_deleted = [n.uuid for n in [di1, do1, pws, pcs, pwm]] - with Capturing(): - delete_nodes([di1.pk], force=True, create_forward=True, call_calc_forward=False, call_work_forward=False) + delete_nodes([di1.pk], dry_run=False, create_forward=True, call_calc_forward=False, call_work_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) # Deleting with these settings the workflow that chooses inputs should @@ -2000,8 +1975,7 @@ def test_loop_cases(self): di1, di2, di3, do1, pws, pcs, pwm = self._create_looped_graph() uuids_check_existence = [n.uuid for n in [di1, di2, di3, do1, pcs]] uuids_check_deleted = [n.uuid for n in [pws, pwm]] - with Capturing(): - delete_nodes([pws.pk], force=True, create_forward=True, call_calc_forward=False, call_work_forward=False) + delete_nodes([pws.pk], dry_run=False, create_forward=True, call_calc_forward=False, call_work_forward=False) self._check_existence(uuids_check_existence, uuids_check_deleted) @staticmethod @@ -2042,6 +2016,5 @@ def test_long_case(self): node_list = self._create_long_graph(10) uuids_check_existence = [n.uuid for n in node_list[:3]] uuids_check_deleted = [n.uuid for n in node_list[3:]] - with Capturing(): - delete_nodes((node_list[3].pk,), force=True, create_forward=True) + delete_nodes((node_list[3].pk,), dry_run=False, create_forward=True) self._check_existence(uuids_check_existence, uuids_check_deleted) From ca22ad530f984875bd0f5f37101b9da96969e188 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:10:11 +0100 Subject: [PATCH 02/19] add tests for API --- tests/test_nodes.py | 53 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/test_nodes.py b/tests/test_nodes.py index c1f45e4d65..a2a214e5d6 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -18,7 +18,7 @@ from aiida.backends.testbase import AiidaTestCase from aiida.common.exceptions import InvalidOperation, ModificationNotAllowed, StoringNotAllowed, ValidationError from aiida.common.links import LinkType -from aiida.manage.database.delete.nodes import delete_nodes +from aiida.manage.database.delete.nodes import delete_nodes, delete_group_nodes class TestNodeIsStorable(AiidaTestCase): @@ -1545,6 +1545,33 @@ def test_deletion_non_existing_pk(): non_existing_pk = -1 delete_nodes([non_existing_pk], dry_run=False) + def test_deletion_dry_run_true(self): + """Verify that a dry run should not delete the node.""" + node = orm.Data().store() + node_pk = node.pk + deleted_pks, was_deleted = delete_nodes([node_pk], dry_run=True) + self.assertTrue(not was_deleted) + self.assertSetEqual(deleted_pks, {node_pk}) + orm.load_node(node_pk) + + def test_deletion_dry_run_callback(self): + """Verify that a dry_run callback works.""" + from aiida.common.exceptions import NotExistent + node = orm.Data().store() + node_pk = node.pk + callback_pks = [] + + def _callback(pks): + callback_pks.extend(pks) + return False + + deleted_pks, was_deleted = delete_nodes([node_pk], dry_run=_callback) + self.assertTrue(was_deleted) + self.assertSetEqual(deleted_pks, {node_pk}) + with self.assertRaises(NotExistent): + orm.load_node(node_pk) + self.assertListEqual(callback_pks, [node_pk]) + # TEST BASIC CASES @@ -2018,3 +2045,27 @@ def test_long_case(self): uuids_check_deleted = [n.uuid for n in node_list[3:]] delete_nodes((node_list[3].pk,), dry_run=False, create_forward=True) self._check_existence(uuids_check_existence, uuids_check_deleted) + + def test_delete_group_nodes(self): + """Test deleting all nodes in a group.""" + group = orm.Group(label='agroup').store() + nodes = [orm.Data().store() for _ in range(2)] + node_pks = {node.pk for node in nodes} + node_uuids = {node.uuid for node in nodes} + group.add_nodes(nodes) + deleted_pks, was_deleted = delete_group_nodes([group.pk], dry_run=False) + self.assertTrue(was_deleted) + self.assertSetEqual(deleted_pks, node_pks) + self._check_existence([], node_uuids) + + def test_delete_group_nodes_dry_run_true(self): + """Verify that a dry run should not delete the node.""" + group = orm.Group(label='agroup2').store() + nodes = [orm.Data().store() for _ in range(2)] + node_pks = {node.pk for node in nodes} + node_uuids = {node.uuid for node in nodes} + group.add_nodes(nodes) + deleted_pks, was_deleted = delete_group_nodes([group.pk], dry_run=True) + self.assertTrue(not was_deleted) + self.assertSetEqual(deleted_pks, node_pks) + self._check_existence(node_uuids, []) From 6353de7476c89bf114aa54e62d8a74b929318ea2 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:19:52 +0100 Subject: [PATCH 03/19] improve log messages --- aiida/cmdline/commands/cmd_group.py | 4 +++- aiida/manage/database/delete/nodes.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 05cc6548cc..738451a99f 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -95,8 +95,10 @@ def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversa verbosity = logging.DEBUG DELETE_LOGGER.setLevel(verbosity) - if not force: + if not (force or dry_run): click.confirm(f'Are you sure to delete Group<{label}>?', abort=True) + elif dry_run: + echo.echo_info(f'Would have deleted Group<{label}>.') if delete_nodes: diff --git a/aiida/manage/database/delete/nodes.py b/aiida/manage/database/delete/nodes.py index c6ea5bb539..f3091841e4 100644 --- a/aiida/manage/database/delete/nodes.py +++ b/aiida/manage/database/delete/nodes.py @@ -86,7 +86,7 @@ def _missing_callback(_pks: Iterable[int]): pks_set_to_delete = get_nodes_delete(pks, get_links=False, missing_callback=_missing_callback, **traversal_rules)['nodes'] - DELETE_LOGGER.info('Deleting %s node(s)', len(pks_set_to_delete)) + DELETE_LOGGER.info('%s Node(s) marked for deletion', len(pks_set_to_delete)) if pks_set_to_delete and DELETE_LOGGER.level == logging.DEBUG: builder = QueryBuilder().append( From 4bdf261f3985b910e8df1780253fa682c1717895 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:28:02 +0100 Subject: [PATCH 04/19] don't make force warning only log --- aiida/cmdline/commands/cmd_group.py | 6 +----- aiida/cmdline/commands/cmd_node.py | 11 +++++------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 738451a99f..340b6bad1b 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -88,11 +88,7 @@ def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversa warnings.warn('`--clear` is deprecated and no longer has any effect.', AiidaDeprecationWarning) # pylint: disable=no-member label = group.label - verbosity = logging.INFO - if force: - verbosity = logging.WARNING - elif verbose: - verbosity = logging.DEBUG + verbosity = logging.DEBUG if verbose else logging.INFO DELETE_LOGGER.setLevel(verbosity) if not (force or dry_run): diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index a5e22a0632..f5a3750e32 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -314,11 +314,7 @@ def node_delete(identifier, dry_run, verbose, force, **traversal_rules): from aiida.orm.utils.loaders import NodeEntityLoader from aiida.manage.database.delete.nodes import delete_nodes, DELETE_LOGGER - verbosity = logging.INFO - if force: - verbosity = logging.WARNING - elif verbose: - verbosity = logging.DEBUG + verbosity = logging.DEBUG if verbose else logging.INFO DELETE_LOGGER.setLevel(verbosity) pks = [] @@ -337,7 +333,10 @@ def _dry_run_callback(pks): return not click.confirm('Shall I continue?') with override_log_formatter_context('%(message)s'): - delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) + _, was_deleted = delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) + + if was_deleted: + echo.echo_success(f'Finished deletion.') @verdi_node.command('rehash') From 6ce0398d883809ab86654e2a5c54a888103f27f0 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:35:30 +0100 Subject: [PATCH 05/19] donlt delete group on dry-run --- aiida/cmdline/commands/cmd_group.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 340b6bad1b..8a759bef1f 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -88,13 +88,15 @@ def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversa warnings.warn('`--clear` is deprecated and no longer has any effect.', AiidaDeprecationWarning) # pylint: disable=no-member label = group.label + klass = group.__class__.__name__ + verbosity = logging.DEBUG if verbose else logging.INFO DELETE_LOGGER.setLevel(verbosity) if not (force or dry_run): - click.confirm(f'Are you sure to delete Group<{label}>?', abort=True) + click.confirm(f'Are you sure to delete {klass}<{label}>?', abort=True) elif dry_run: - echo.echo_info(f'Would have deleted Group<{label}>.') + echo.echo_info(f'Would have deleted {klass}<{label}>.') if delete_nodes: @@ -110,8 +112,9 @@ def _dry_run_callback(pks): # don't delete the group if the nodes were not deleted return - orm.Group.objects.delete(group.pk) - echo.echo_success(f'Group<{label}> deleted.') + if not dry_run: + orm.Group.objects.delete(group.pk) + echo.echo_success(f'{klass}<{label}> deleted.') @verdi_group.command('relabel') From 0f6dc67c9ce954f27f7f796b810dfc0b54a44421 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:37:06 +0100 Subject: [PATCH 06/19] move clear to bottom of help --- aiida/cmdline/commands/cmd_group.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 8a759bef1f..66fbe1c6c4 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -63,10 +63,6 @@ def group_remove_nodes(group, nodes, clear, force): @verdi_group.command('delete') @arguments.GROUP() -@options.GROUP_CLEAR( - help='Remove all nodes before deleting the group itself.' + - ' [deprecated: No longer has any effect. Will be removed in 2.0.0]' -) @options.FORCE() @click.option( '--delete-nodes', is_flag=True, default=False, help='Delete all nodes in the group along with the group itself.' @@ -74,6 +70,10 @@ def group_remove_nodes(group, nodes, clear, force): @options.graph_traversal_rules(GraphTraversalRules.DELETE.value) @options.DRY_RUN() @options.VERBOSE() +@options.GROUP_CLEAR( + help='Remove all nodes before deleting the group itself.' + + ' [deprecated: No longer has any effect. Will be removed in 2.0.0]' +) @with_dbenv() def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversal_rules): """Delete a group. From d6d9154fff12454745b2d159a6320a2f349e6815 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:39:01 +0100 Subject: [PATCH 07/19] improve help string --- aiida/cmdline/commands/cmd_group.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 66fbe1c6c4..69d80abdd1 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -76,10 +76,7 @@ def group_remove_nodes(group, nodes, clear, force): ) @with_dbenv() def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversal_rules): - """Delete a group. - - Note that this command only deletes groups - nodes contained in the group will remain untouched. - """ + """Delete a group and (optionally) the nodes it contains.""" from aiida.common.log import override_log_formatter_context from aiida.manage.database.delete.nodes import delete_group_nodes, DELETE_LOGGER from aiida import orm From 4deb19375ac710bbee3ce6d3ca5171396dbe158a Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:49:12 +0100 Subject: [PATCH 08/19] add cli tests for `verdi group delete` --- tests/cmdline/commands/test_group.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/cmdline/commands/test_group.py b/tests/cmdline/commands/test_group.py index a8b53370a5..9bc4c92e8c 100644 --- a/tests/cmdline/commands/test_group.py +++ b/tests/cmdline/commands/test_group.py @@ -131,6 +131,12 @@ def test_delete(self): """Test `verdi group delete` command.""" orm.Group(label='group_test_delete_01').store() orm.Group(label='group_test_delete_02').store() + orm.Group(label='group_test_delete_03').store() + + # dry run + result = self.cli_runner.invoke(cmd_group.group_delete, ['--dry-run', 'group_test_delete_01']) + self.assertClickResultNoException(result) + orm.load_group(label='group_test_delete_01') result = self.cli_runner.invoke(cmd_group.group_delete, ['--force', 'group_test_delete_01']) self.assertClickResultNoException(result) @@ -142,6 +148,7 @@ def test_delete(self): node_01 = orm.CalculationNode().store() node_02 = orm.CalculationNode().store() + node_pks = {node_01.pk, node_02.pk} # Add some nodes and then use `verdi group delete` to delete a group that contains nodes group = orm.load_group(label='group_test_delete_02') @@ -154,6 +161,21 @@ def test_delete(self): with self.assertRaises(exceptions.NotExistent): orm.load_group(label='group_test_delete_02') + # check nodes still exist + for pk in node_pks: + orm.load_node(pk) + + # delete the group and the nodes it contains + group = orm.load_group(label='group_test_delete_03') + group.add_nodes([node_01, node_02]) + result = self.cli_runner.invoke(cmd_group.group_delete, ['--force', '--delete-nodes', 'group_test_delete_03']) + self.assertClickResultNoException(result) + + # check nodes no longer exist + for pk in node_pks: + with self.assertRaises(exceptions.NotExistent): + orm.load_node(pk) + def test_show(self): """Test `verdi group show` command.""" result = self.cli_runner.invoke(cmd_group.group_show, ['dummygroup1']) From 17f60afb3c633a27c66f70a5a5421fbf9eb1b86d Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 17:52:01 +0100 Subject: [PATCH 09/19] add to test --- tests/cmdline/commands/test_group.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/cmdline/commands/test_group.py b/tests/cmdline/commands/test_group.py index 9bc4c92e8c..0a4f3c0933 100644 --- a/tests/cmdline/commands/test_group.py +++ b/tests/cmdline/commands/test_group.py @@ -171,7 +171,9 @@ def test_delete(self): result = self.cli_runner.invoke(cmd_group.group_delete, ['--force', '--delete-nodes', 'group_test_delete_03']) self.assertClickResultNoException(result) - # check nodes no longer exist + # check group and nodes no longer exist + with self.assertRaises(exceptions.NotExistent): + orm.load_group(label='group_test_delete_03') for pk in node_pks: with self.assertRaises(exceptions.NotExistent): orm.load_node(pk) From 81b4cb54b2b8a7cfd3f5c9a46fe160900c7ce259 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 18:07:15 +0100 Subject: [PATCH 10/19] fix pre-commit --- aiida/cmdline/commands/cmd_group.py | 2 +- aiida/cmdline/commands/cmd_node.py | 2 +- docs/source/reference/command_line.rst | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 69d80abdd1..18c9d55716 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -32,7 +32,7 @@ def verdi_group(): @arguments.NODES() @with_dbenv() def group_add_nodes(group, force, nodes): - """Add nodes to the a group.""" + """Add nodes to a group.""" if not force: click.confirm(f'Do you really want to add {len(nodes)} nodes to Group<{group.label}>?', abort=True) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index f5a3750e32..2fffaf73c0 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -336,7 +336,7 @@ def _dry_run_callback(pks): _, was_deleted = delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) if was_deleted: - echo.echo_success(f'Finished deletion.') + echo.echo_success('Finished deletion.') @verdi_node.command('rehash') diff --git a/docs/source/reference/command_line.rst b/docs/source/reference/command_line.rst index 84f6989dad..e2a1b5ecd1 100644 --- a/docs/source/reference/command_line.rst +++ b/docs/source/reference/command_line.rst @@ -277,10 +277,10 @@ Below is a list with all available subcommands. --help Show this message and exit. Commands: - add-nodes Add nodes to the a group. + add-nodes Add nodes to a group. copy Duplicate a group. create Create an empty group with a given name. - delete Delete a group. + delete Delete a group and (optionally) the nodes it contains. description Change the description of a group. list Show a list of existing groups. path Inspect groups of nodes, with delimited label paths. From e95ae72ce7ef0b0f2c588cbf6f145f54c33c9a52 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Nov 2020 18:31:36 +0100 Subject: [PATCH 11/19] update `verdi code delete` --- aiida/cmdline/commands/cmd_code.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index bf65998615..f8309acbb6 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -196,25 +196,23 @@ def delete(codes, verbose, dry_run, force): from aiida.common.log import override_log_formatter_context from aiida.manage.database.delete.nodes import delete_nodes, DELETE_LOGGER - verbosity = logging.INFO - if force: - verbosity = logging.WARNING - elif verbose: - verbosity = logging.DEBUG + verbosity = logging.DEBUG if verbose else logging.INFO + DELETE_LOGGER.setLevel(verbosity) node_pks_to_delete = [code.pk for code in codes] def _dry_run_callback(pks): - if dry_run: - return True - if force: + if not pks or force: return False echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') return not click.confirm('Shall I continue?') DELETE_LOGGER.setLevel(verbosity) with override_log_formatter_context('%(message)s'): - delete_nodes(node_pks_to_delete, dry_run=_dry_run_callback) + _, was_deleted = delete_nodes(node_pks_to_delete, dry_run=dry_run or _dry_run_callback) + + if was_deleted: + echo.echo_success('Finished deletion.') @verdi_code.command() From 3d0f94ec5ae01de6f1ab183648588ae2dadb36bb Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Tue, 5 Jan 2021 02:08:11 +0000 Subject: [PATCH 12/19] =?UTF-8?q?=F0=9F=91=8C=20IMPROVE:=20Expose=20functi?= =?UTF-8?q?ons=20from=20`aiida.manage.database`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit and add to documentation --- aiida/manage/database/__init__.py | 6 ++++++ aiida/manage/database/delete/__init__.py | 6 ++++++ aiida/manage/database/delete/nodes.py | 13 ++++++++++-- docs/source/howto/data.rst | 26 +++++++++++++++++------- pyproject.toml | 2 +- tests/test_nodes.py | 2 +- 6 files changed, 44 insertions(+), 11 deletions(-) diff --git a/aiida/manage/database/__init__.py b/aiida/manage/database/__init__.py index 2776a55f97..cba6bb0a5a 100644 --- a/aiida/manage/database/__init__.py +++ b/aiida/manage/database/__init__.py @@ -7,3 +7,9 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### +# pylint: disable=wildcard-import,undefined-variable,redefined-builtin,cyclic-import +"""Functions for managing the database.""" + +from .delete import * + +__all__ = delete.__all__ diff --git a/aiida/manage/database/delete/__init__.py b/aiida/manage/database/delete/__init__.py index 2776a55f97..0399139cbd 100644 --- a/aiida/manage/database/delete/__init__.py +++ b/aiida/manage/database/delete/__init__.py @@ -7,3 +7,9 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### +# pylint: disable=wildcard-import,undefined-variable,redefined-builtin,cyclic-import +"""Functions for deleting entities from the database.""" + +from .nodes import * + +__all__ = nodes.__all__ diff --git a/aiida/manage/database/delete/nodes.py b/aiida/manage/database/delete/nodes.py index f3091841e4..b4508aa5da 100644 --- a/aiida/manage/database/delete/nodes.py +++ b/aiida/manage/database/delete/nodes.py @@ -18,6 +18,8 @@ from aiida.orm import Group, Node, QueryBuilder, load_node from aiida.tools.graph.graph_traversers import get_nodes_delete +__all__ = ('DELETE_LOGGER', 'delete_nodes', 'delete_group_nodes') + DELETE_LOGGER = AIIDA_LOGGER.getChild('delete') @@ -28,7 +30,7 @@ def delete_nodes( force: Optional[bool] = None, **traversal_rules: bool ) -> Tuple[Set[int], bool]: - """Delete nodes by a list of pks. + """Delete nodes given a list of "starting" PKs. This command will delete not only the specified nodes, but also the ones that are linked to these and should be also deleted in order to keep a consistent provenance @@ -49,6 +51,12 @@ def delete_nodes( nodes will be deleted as well, and then any CALC node that may have those as inputs, and so on. + .. deprecated:: 1.6.0 + The `verbosity` keyword will be removed in `v2.0.0`, set the level of `DELETE_LOGGER` instead. + + .. deprecated:: 1.6.0 + The `force` keyword will be removed in `v2.0.0`, use the `dry_run` option instead. + :param pks: a list of starting PKs of the nodes to delete (the full set will be based on the traversal rules) @@ -57,7 +65,8 @@ def delete_nodes( If False, delete the pks without confirmation If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` - :param traversal_rules: graph traversal rules. See :const:`aiida.common.links.GraphTraversalRules` what rule names + :param traversal_rules: graph traversal rules. + See :const:`aiida.common.links.GraphTraversalRules` for what rule names are toggleable and what the defaults are. :returns: (pks to delete, whether they were deleted) diff --git a/docs/source/howto/data.rst b/docs/source/howto/data.rst index ddae0d6c75..4f256ff7a1 100644 --- a/docs/source/howto/data.rst +++ b/docs/source/howto/data.rst @@ -507,10 +507,15 @@ From the command line interface: Are you sure to delete Group? [y/N]: y Success: Group deleted. -.. important:: - Any deletion operation related to groups won't affect the nodes themselves. - For example if you delete a group, the nodes that belonged to the group will remain in the database. - The same happens if you remove nodes from the group -- they will remain in the database but won't belong to the group anymore. +Any deletion operation related to groups, by default, will not affect the nodes themselves. +For example if you delete a group, the nodes that belonged to the group will remain in the database. +The same happens if you remove nodes from the group -- they will remain in the database but won't belong to the group anymore. + +If you also wish to delete the nodes, when deleting the group, use the ``--delete-nodes``: + +.. code-block:: console + + $ verdi group delete another_group --delete-nodes Copy one group into another ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -764,7 +769,7 @@ Deleting data By default, every time you run or submit a new calculation, AiiDA will create for you new nodes in the database, and will never replace or delete data. There are cases, however, when it might be useful to delete nodes that are not useful anymore, for instance test runs or incorrect/wrong data and calculations. -For this case, AiiDA provides the ``verdi node delete`` command to remove the nodes from the provenance graph. +For this case, AiiDA provides the ``verdi node delete`` command and the :py:func:`~aiida.manage.database.delete.nodes.delete_nodes` function, to remove the nodes from the provenance graph: .. caution:: Once the data is deleted, there is no way to recover it (unless you made a backup). @@ -780,6 +785,13 @@ In addition, there are a number of additional rules that are not mandatory to en For instance, you can set ``--create-forward`` if, when deleting a calculation, you want to delete also the data it produced (using instead ``--no-create-forward`` will delete the calculation only, keeping the output data: note that this effectively strips out the provenance information of the output data). The full list of these flags is available from the help command ``verdi node delete -h``. +.. code-block:: python + + from aiida.manage.database import delete_nodes + pks_to_be_deleted = delete_nodes( + [1, 2, 3], dry_run=True, create_forward=True, call_calc_forward=True, call_work_forward=True + ) + Deleting computers ------------------ To delete a computer, you can use ``verdi computer delete``. @@ -807,8 +819,8 @@ This command will delete both the file repository and the database. It is not possible to restore a deleted profile unless it was previously backed up! -Transfering data -================ +Transferring data +================= .. danger:: diff --git a/pyproject.toml b/pyproject.toml index 939a30965f..cae3660373 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -95,7 +95,7 @@ deps = py36: -rrequirements/requirements-py-3.6.txt py37: -rrequirements/requirements-py-3.7.txt py38: -rrequirements/requirements-py-3.8.txt - py38: -rrequirements/requirements-py-3.9.txt + py39: -rrequirements/requirements-py-3.9.txt passenv = RUN_APIDOC setenv = update: RUN_APIDOC = False diff --git a/tests/test_nodes.py b/tests/test_nodes.py index a2a214e5d6..1299317aa9 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -18,7 +18,7 @@ from aiida.backends.testbase import AiidaTestCase from aiida.common.exceptions import InvalidOperation, ModificationNotAllowed, StoringNotAllowed, ValidationError from aiida.common.links import LinkType -from aiida.manage.database.delete.nodes import delete_nodes, delete_group_nodes +from aiida.manage.database import delete_nodes, delete_group_nodes class TestNodeIsStorable(AiidaTestCase): From 4b50e318f0a125ae6ed16a5803a4332803545e0d Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Tue, 5 Jan 2021 02:17:36 +0000 Subject: [PATCH 13/19] pin minor sphinx version --- setup.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.json b/setup.json index 6054209dde..9442e1b619 100644 --- a/setup.json +++ b/setup.json @@ -74,7 +74,7 @@ "docutils==0.15.2", "pygments~=2.5", "pydata-sphinx-theme~=0.4.0", - "sphinx~=3.2", + "sphinx~=3.2.1", "sphinxcontrib-details-directive~=0.1.0", "sphinx-panels~=0.5.0", "sphinx-copybutton~=0.3.0", From 9b9e07dac4196cd9c389cc35576bc701e45fa7e7 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Tue, 5 Jan 2021 02:35:18 +0000 Subject: [PATCH 14/19] fix tests --- aiida/manage/database/delete/nodes.py | 7 +++++-- pyproject.toml | 10 ++++++++++ requirements/requirements-py-3.9.txt | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/aiida/manage/database/delete/nodes.py b/aiida/manage/database/delete/nodes.py index b4508aa5da..ae238a50a1 100644 --- a/aiida/manage/database/delete/nodes.py +++ b/aiida/manage/database/delete/nodes.py @@ -15,8 +15,6 @@ from aiida.backends.utils import delete_nodes_and_connections from aiida.common.log import AIIDA_LOGGER from aiida.common.warnings import AiidaDeprecationWarning -from aiida.orm import Group, Node, QueryBuilder, load_node -from aiida.tools.graph.graph_traversers import get_nodes_delete __all__ = ('DELETE_LOGGER', 'delete_nodes', 'delete_group_nodes') @@ -74,6 +72,9 @@ def delete_nodes( """ # pylint: disable=too-many-arguments,too-many-branches,too-many-locals,too-many-statements + from aiida.orm import Node, QueryBuilder, load_node + from aiida.tools.graph.graph_traversers import get_nodes_delete + if verbosity is not None: warnings.warn( 'The verbosity option is deprecated and will be removed in `aiida-core==2.0.0`. ' @@ -181,6 +182,8 @@ def delete_group_nodes( :returns: (node pks to delete, whether they were deleted) """ + from aiida.orm import Group, Node, QueryBuilder + group_node_query = QueryBuilder().append( Group, filters={ diff --git a/pyproject.toml b/pyproject.toml index cae3660373..a2a957cce8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -87,6 +87,16 @@ setenv = sqla: AIIDA_TEST_BACKEND = sqlalchemy commands = pytest {posargs} +[testenv:py{36,37,38,39}-verdi] +deps = + py36: -rrequirements/requirements-py-3.6.txt + py37: -rrequirements/requirements-py-3.7.txt + py38: -rrequirements/requirements-py-3.8.txt + py39: -rrequirements/requirements-py-3.9.txt +setenv = + AIIDA_TEST_BACKEND = django +commands = verdi {posargs} + [testenv:py{36,37,38,39}-docs-{clean,update}] description = clean: Build the documentation (remove any existing build) diff --git a/requirements/requirements-py-3.9.txt b/requirements/requirements-py-3.9.txt index 6f3d65c319..32564e4c39 100644 --- a/requirements/requirements-py-3.9.txt +++ b/requirements/requirements-py-3.9.txt @@ -126,7 +126,7 @@ simplejson==3.17.2 six==1.15.0 snowballstemmer==2.0.0 spglib==1.16.0 -Sphinx==3.3.0 +Sphinx==3.2.1 sphinx-copybutton==0.3.1 sphinx-notfound-page==0.5 sphinx-panels==0.5.2 From 65a99de86f1704bd971530aab57f081df859c2e7 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 6 Jan 2021 15:56:34 +0000 Subject: [PATCH 15/19] move functions to `aiida.tools.graph.deletions` --- aiida/cmdline/commands/cmd_code.py | 2 +- aiida/cmdline/commands/cmd_group.py | 2 +- aiida/cmdline/commands/cmd_node.py | 2 +- aiida/manage/database/__init__.py | 6 - aiida/manage/database/delete/__init__.py | 6 - aiida/manage/database/delete/nodes.py | 178 +-------------------- aiida/tools/__init__.py | 5 +- aiida/tools/graph/__init__.py | 5 + aiida/tools/graph/deletions.py | 195 +++++++++++++++++++++++ docs/source/howto/data.rst | 4 +- tests/test_nodes.py | 2 +- 11 files changed, 215 insertions(+), 192 deletions(-) create mode 100644 aiida/tools/graph/deletions.py diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index f8309acbb6..03bde2785a 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -194,7 +194,7 @@ def delete(codes, verbose, dry_run, force): Note that codes are part of the data provenance, and deleting a code will delete all calculations using it. """ from aiida.common.log import override_log_formatter_context - from aiida.manage.database.delete.nodes import delete_nodes, DELETE_LOGGER + from aiida.tools import delete_nodes, DELETE_LOGGER verbosity = logging.DEBUG if verbose else logging.INFO DELETE_LOGGER.setLevel(verbosity) diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index 18c9d55716..e98c0a3a5a 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -78,7 +78,7 @@ def group_remove_nodes(group, nodes, clear, force): def group_delete(group, clear, delete_nodes, dry_run, force, verbose, **traversal_rules): """Delete a group and (optionally) the nodes it contains.""" from aiida.common.log import override_log_formatter_context - from aiida.manage.database.delete.nodes import delete_group_nodes, DELETE_LOGGER + from aiida.tools import delete_group_nodes, DELETE_LOGGER from aiida import orm if clear: diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 2fffaf73c0..6eb095edeb 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -312,7 +312,7 @@ def node_delete(identifier, dry_run, verbose, force, **traversal_rules): """ from aiida.common.log import override_log_formatter_context from aiida.orm.utils.loaders import NodeEntityLoader - from aiida.manage.database.delete.nodes import delete_nodes, DELETE_LOGGER + from aiida.tools import delete_nodes, DELETE_LOGGER verbosity = logging.DEBUG if verbose else logging.INFO DELETE_LOGGER.setLevel(verbosity) diff --git a/aiida/manage/database/__init__.py b/aiida/manage/database/__init__.py index cba6bb0a5a..2776a55f97 100644 --- a/aiida/manage/database/__init__.py +++ b/aiida/manage/database/__init__.py @@ -7,9 +7,3 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -# pylint: disable=wildcard-import,undefined-variable,redefined-builtin,cyclic-import -"""Functions for managing the database.""" - -from .delete import * - -__all__ = delete.__all__ diff --git a/aiida/manage/database/delete/__init__.py b/aiida/manage/database/delete/__init__.py index 0399139cbd..2776a55f97 100644 --- a/aiida/manage/database/delete/__init__.py +++ b/aiida/manage/database/delete/__init__.py @@ -7,9 +7,3 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -# pylint: disable=wildcard-import,undefined-variable,redefined-builtin,cyclic-import -"""Functions for deleting entities from the database.""" - -from .nodes import * - -__all__ = nodes.__all__ diff --git a/aiida/manage/database/delete/nodes.py b/aiida/manage/database/delete/nodes.py index ae238a50a1..ddb7c0fe37 100644 --- a/aiida/manage/database/delete/nodes.py +++ b/aiida/manage/database/delete/nodes.py @@ -7,18 +7,8 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -"""Functions to delete entities from the database.""" -import logging +"""Functions to delete nodes from the database, preserving provenance integrity.""" from typing import Callable, Iterable, Optional, Set, Tuple, Union -import warnings - -from aiida.backends.utils import delete_nodes_and_connections -from aiida.common.log import AIIDA_LOGGER -from aiida.common.warnings import AiidaDeprecationWarning - -__all__ = ('DELETE_LOGGER', 'delete_nodes', 'delete_group_nodes') - -DELETE_LOGGER = AIIDA_LOGGER.getChild('delete') def delete_nodes( @@ -30,169 +20,11 @@ def delete_nodes( ) -> Tuple[Set[int], bool]: """Delete nodes given a list of "starting" PKs. - This command will delete not only the specified nodes, but also the ones that are - linked to these and should be also deleted in order to keep a consistent provenance - according to the rules explained in the concepts section of the documentation. - In summary: - - 1. If a DATA node is deleted, any process nodes linked to it will also be deleted. - - 2. If a CALC node is deleted, any incoming WORK node (callers) will be deleted as - well whereas any incoming DATA node (inputs) will be kept. Outgoing DATA nodes - (outputs) will be deleted by default but this can be disabled. - - 3. If a WORK node is deleted, any incoming WORK node (callers) will be deleted as - well, but all DATA nodes will be kept. Outgoing WORK or CALC nodes will be kept by - default, but deletion of either of both kind of connected nodes can be enabled. - - These rules are 'recursive', so if a CALC node is deleted, then its output DATA - nodes will be deleted as well, and then any CALC node that may have those as - inputs, and so on. - - .. deprecated:: 1.6.0 - The `verbosity` keyword will be removed in `v2.0.0`, set the level of `DELETE_LOGGER` instead. - .. deprecated:: 1.6.0 - The `force` keyword will be removed in `v2.0.0`, use the `dry_run` option instead. - - :param pks: a list of starting PKs of the nodes to delete - (the full set will be based on the traversal rules) - - :param dry_run: - If True, return the pks to delete without deleting anything. - If False, delete the pks without confirmation - If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` - - :param traversal_rules: graph traversal rules. - See :const:`aiida.common.links.GraphTraversalRules` for what rule names - are toggleable and what the defaults are. - - :returns: (pks to delete, whether they were deleted) - - """ - # pylint: disable=too-many-arguments,too-many-branches,too-many-locals,too-many-statements - - from aiida.orm import Node, QueryBuilder, load_node - from aiida.tools.graph.graph_traversers import get_nodes_delete - - if verbosity is not None: - warnings.warn( - 'The verbosity option is deprecated and will be removed in `aiida-core==2.0.0`. ' - 'Set the level of DELETE_LOGGER instead', AiidaDeprecationWarning - ) # pylint: disable=no-member - - if force is not None: - warnings.warn( - 'The force option is deprecated and will be removed in `aiida-core==2.0.0`. ' - 'Use dry_run instead', AiidaDeprecationWarning - ) # pylint: disable=no-member - if force is True: - dry_run = False - - def _missing_callback(_pks: Iterable[int]): - for _pk in _pks: - DELETE_LOGGER.warning(f'warning: node with pk<{_pk}> does not exist, skipping') - - pks_set_to_delete = get_nodes_delete(pks, get_links=False, missing_callback=_missing_callback, - **traversal_rules)['nodes'] - - DELETE_LOGGER.info('%s Node(s) marked for deletion', len(pks_set_to_delete)) - - if pks_set_to_delete and DELETE_LOGGER.level == logging.DEBUG: - builder = QueryBuilder().append( - Node, filters={'id': { - 'in': pks_set_to_delete - }}, project=('uuid', 'id', 'node_type', 'label') - ) - DELETE_LOGGER.debug('Node(s) to delete:') - for uuid, pk, type_string, label in builder.iterall(): - try: - short_type_string = type_string.split('.')[-2] - except IndexError: - short_type_string = type_string - DELETE_LOGGER.debug(f' {uuid} {pk} {short_type_string} {label}') - - if dry_run is True: - DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') - return (pks_set_to_delete, False) - - # confirm deletion - if callable(dry_run) and dry_run(pks_set_to_delete): - DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') - return (pks_set_to_delete, False) - - if not pks_set_to_delete: - return (pks_set_to_delete, True) - - # Recover the list of folders to delete before actually deleting the nodes. I will delete the folders only later, - # so that if there is a problem during the deletion of the nodes in the DB, I don't delete the folders - repositories = [load_node(pk)._repository for pk in pks_set_to_delete] # pylint: disable=protected-access - - DELETE_LOGGER.info('Starting node deletion...') - delete_nodes_and_connections(pks_set_to_delete) - - DELETE_LOGGER.info('Nodes deleted from database, deleting files from the repository now...') - - # If we are here, we managed to delete the entries from the DB. - # I can now delete the folders - for repository in repositories: - repository.erase(force=True) - - DELETE_LOGGER.info('Deletion of nodes completed.') - - return (pks_set_to_delete, True) - - -def delete_group_nodes( - pks: Iterable[int], - dry_run: Union[bool, Callable[[Set[int]], bool]] = True, - **traversal_rules: bool -) -> Tuple[Set[int], bool]: - """Delete nodes contained in a list of groups (not the groups themselves!). - - This command will delete not only the nodes, but also the ones that are - linked to these and should be also deleted in order to keep a consistent provenance - according to the rules explained in the concepts section of the documentation. - In summary: - - 1. If a DATA node is deleted, any process nodes linked to it will also be deleted. - - 2. If a CALC node is deleted, any incoming WORK node (callers) will be deleted as - well whereas any incoming DATA node (inputs) will be kept. Outgoing DATA nodes - (outputs) will be deleted by default but this can be disabled. - - 3. If a WORK node is deleted, any incoming WORK node (callers) will be deleted as - well, but all DATA nodes will be kept. Outgoing WORK or CALC nodes will be kept by - default, but deletion of either of both kind of connected nodes can be enabled. - - These rules are 'recursive', so if a CALC node is deleted, then its output DATA - nodes will be deleted as well, and then any CALC node that may have those as - inputs, and so on. - - :param pks: a list of the groups - - :param dry_run: - If True, return the pks to delete without deleting anything. - If False, delete the pks without confirmation - If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` - - :param traversal_rules: graph traversal rules. See :const:`aiida.common.links.GraphTraversalRules` what rule names - are toggleable and what the defaults are. - - :returns: (node pks to delete, whether they were deleted) + This function has been moved and will be removed in `v2.0.0`. + It should now be imported using `from aiida.tools import delete_nodes` """ - from aiida.orm import Group, Node, QueryBuilder + from aiida.tools import delete_nodes as _delete - group_node_query = QueryBuilder().append( - Group, - filters={ - 'id': { - 'in': list(pks) - } - }, - tag='groups', - ).append(Node, project='id', with_group='groups') - group_node_query.distinct() - node_pks = group_node_query.all(flat=True) - return delete_nodes(node_pks, dry_run=dry_run, **traversal_rules) + return _delete(pks, verbosity, dry_run, force, **traversal_rules) diff --git a/aiida/tools/__init__.py b/aiida/tools/__init__.py index fe4146ea57..ffdf77d6e5 100644 --- a/aiida/tools/__init__.py +++ b/aiida/tools/__init__.py @@ -25,5 +25,8 @@ from .data.array.kpoints import * from .data.structure import * from .dbimporters import * +from .graph import * -__all__ = (calculations.__all__ + data.array.kpoints.__all__ + data.structure.__all__ + dbimporters.__all__) +__all__ = ( + calculations.__all__ + data.array.kpoints.__all__ + data.structure.__all__ + dbimporters.__all__ + graph.__all__ +) diff --git a/aiida/tools/graph/__init__.py b/aiida/tools/graph/__init__.py index 2776a55f97..c095d1619a 100644 --- a/aiida/tools/graph/__init__.py +++ b/aiida/tools/graph/__init__.py @@ -7,3 +7,8 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### +# pylint: disable=wildcard-import,undefined-variable +"""Provides tools for traversing the provenance graph.""" +from .deletions import * + +__all__ = deletions.__all__ diff --git a/aiida/tools/graph/deletions.py b/aiida/tools/graph/deletions.py new file mode 100644 index 0000000000..0aaefb49a6 --- /dev/null +++ b/aiida/tools/graph/deletions.py @@ -0,0 +1,195 @@ +# -*- coding: utf-8 -*- +########################################################################### +# Copyright (c), The AiiDA team. All rights reserved. # +# This file is part of the AiiDA code. # +# # +# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # +# For further information on the license, see the LICENSE.txt file # +# For further information please visit http://www.aiida.net # +########################################################################### +"""Functions to delete entities from the database, preserving provenance integrity.""" +import logging +from typing import Callable, Iterable, Optional, Set, Tuple, Union +import warnings + +from aiida.backends.utils import delete_nodes_and_connections +from aiida.common.log import AIIDA_LOGGER +from aiida.common.warnings import AiidaDeprecationWarning +from aiida.orm import Group, Node, QueryBuilder, load_node +from aiida.tools.graph.graph_traversers import get_nodes_delete + +__all__ = ('DELETE_LOGGER', 'delete_nodes', 'delete_group_nodes') + +DELETE_LOGGER = AIIDA_LOGGER.getChild('delete') + + +def delete_nodes( + pks: Iterable[int], + verbosity: Optional[int] = None, + dry_run: Union[bool, Callable[[Set[int]], bool]] = True, + force: Optional[bool] = None, + **traversal_rules: bool +) -> Tuple[Set[int], bool]: + """Delete nodes given a list of "starting" PKs. + + This command will delete not only the specified nodes, but also the ones that are + linked to these and should be also deleted in order to keep a consistent provenance + according to the rules explained in the concepts section of the documentation. + In summary: + + 1. If a DATA node is deleted, any process nodes linked to it will also be deleted. + + 2. If a CALC node is deleted, any incoming WORK node (callers) will be deleted as + well whereas any incoming DATA node (inputs) will be kept. Outgoing DATA nodes + (outputs) will be deleted by default but this can be disabled. + + 3. If a WORK node is deleted, any incoming WORK node (callers) will be deleted as + well, but all DATA nodes will be kept. Outgoing WORK or CALC nodes will be kept by + default, but deletion of either of both kind of connected nodes can be enabled. + + These rules are 'recursive', so if a CALC node is deleted, then its output DATA + nodes will be deleted as well, and then any CALC node that may have those as + inputs, and so on. + + .. deprecated:: 1.6.0 + The `verbosity` keyword will be removed in `v2.0.0`, set the level of `DELETE_LOGGER` instead. + + .. deprecated:: 1.6.0 + The `force` keyword will be removed in `v2.0.0`, use the `dry_run` option instead. + + :param pks: a list of starting PKs of the nodes to delete + (the full set will be based on the traversal rules) + + :param dry_run: + If True, return the pks to delete without deleting anything. + If False, delete the pks without confirmation + If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` + + :param traversal_rules: graph traversal rules. + See :const:`aiida.common.links.GraphTraversalRules` for what rule names + are toggleable and what the defaults are. + + :returns: (pks to delete, whether they were deleted) + + """ + # pylint: disable=too-many-arguments,too-many-branches,too-many-locals,too-many-statements + + if verbosity is not None: + warnings.warn( + 'The verbosity option is deprecated and will be removed in `aiida-core==2.0.0`. ' + 'Set the level of DELETE_LOGGER instead', AiidaDeprecationWarning + ) # pylint: disable=no-member + + if force is not None: + warnings.warn( + 'The force option is deprecated and will be removed in `aiida-core==2.0.0`. ' + 'Use dry_run instead', AiidaDeprecationWarning + ) # pylint: disable=no-member + if force is True: + dry_run = False + + def _missing_callback(_pks: Iterable[int]): + for _pk in _pks: + DELETE_LOGGER.warning(f'warning: node with pk<{_pk}> does not exist, skipping') + + pks_set_to_delete = get_nodes_delete(pks, get_links=False, missing_callback=_missing_callback, + **traversal_rules)['nodes'] + + DELETE_LOGGER.info('%s Node(s) marked for deletion', len(pks_set_to_delete)) + + if pks_set_to_delete and DELETE_LOGGER.level == logging.DEBUG: + builder = QueryBuilder().append( + Node, filters={'id': { + 'in': pks_set_to_delete + }}, project=('uuid', 'id', 'node_type', 'label') + ) + DELETE_LOGGER.debug('Node(s) to delete:') + for uuid, pk, type_string, label in builder.iterall(): + try: + short_type_string = type_string.split('.')[-2] + except IndexError: + short_type_string = type_string + DELETE_LOGGER.debug(f' {uuid} {pk} {short_type_string} {label}') + + if dry_run is True: + DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') + return (pks_set_to_delete, False) + + # confirm deletion + if callable(dry_run) and dry_run(pks_set_to_delete): + DELETE_LOGGER.info('This was a dry run, exiting without deleting anything') + return (pks_set_to_delete, False) + + if not pks_set_to_delete: + return (pks_set_to_delete, True) + + # Recover the list of folders to delete before actually deleting the nodes. I will delete the folders only later, + # so that if there is a problem during the deletion of the nodes in the DB, I don't delete the folders + repositories = [load_node(pk)._repository for pk in pks_set_to_delete] # pylint: disable=protected-access + + DELETE_LOGGER.info('Starting node deletion...') + delete_nodes_and_connections(pks_set_to_delete) + + DELETE_LOGGER.info('Nodes deleted from database, deleting files from the repository now...') + + # If we are here, we managed to delete the entries from the DB. + # I can now delete the folders + for repository in repositories: + repository.erase(force=True) + + DELETE_LOGGER.info('Deletion of nodes completed.') + + return (pks_set_to_delete, True) + + +def delete_group_nodes( + pks: Iterable[int], + dry_run: Union[bool, Callable[[Set[int]], bool]] = True, + **traversal_rules: bool +) -> Tuple[Set[int], bool]: + """Delete nodes contained in a list of groups (not the groups themselves!). + + This command will delete not only the nodes, but also the ones that are + linked to these and should be also deleted in order to keep a consistent provenance + according to the rules explained in the concepts section of the documentation. + In summary: + + 1. If a DATA node is deleted, any process nodes linked to it will also be deleted. + + 2. If a CALC node is deleted, any incoming WORK node (callers) will be deleted as + well whereas any incoming DATA node (inputs) will be kept. Outgoing DATA nodes + (outputs) will be deleted by default but this can be disabled. + + 3. If a WORK node is deleted, any incoming WORK node (callers) will be deleted as + well, but all DATA nodes will be kept. Outgoing WORK or CALC nodes will be kept by + default, but deletion of either of both kind of connected nodes can be enabled. + + These rules are 'recursive', so if a CALC node is deleted, then its output DATA + nodes will be deleted as well, and then any CALC node that may have those as + inputs, and so on. + + :param pks: a list of the groups + + :param dry_run: + If True, return the pks to delete without deleting anything. + If False, delete the pks without confirmation + If callable, a function that return True/False, based on the pks, e.g. ``dry_run=lambda pks: True`` + + :param traversal_rules: graph traversal rules. See :const:`aiida.common.links.GraphTraversalRules` what rule names + are toggleable and what the defaults are. + + :returns: (node pks to delete, whether they were deleted) + + """ + group_node_query = QueryBuilder().append( + Group, + filters={ + 'id': { + 'in': list(pks) + } + }, + tag='groups', + ).append(Node, project='id', with_group='groups') + group_node_query.distinct() + node_pks = group_node_query.all(flat=True) + return delete_nodes(node_pks, dry_run=dry_run, **traversal_rules) diff --git a/docs/source/howto/data.rst b/docs/source/howto/data.rst index 4f256ff7a1..8f17b34be5 100644 --- a/docs/source/howto/data.rst +++ b/docs/source/howto/data.rst @@ -769,7 +769,7 @@ Deleting data By default, every time you run or submit a new calculation, AiiDA will create for you new nodes in the database, and will never replace or delete data. There are cases, however, when it might be useful to delete nodes that are not useful anymore, for instance test runs or incorrect/wrong data and calculations. -For this case, AiiDA provides the ``verdi node delete`` command and the :py:func:`~aiida.manage.database.delete.nodes.delete_nodes` function, to remove the nodes from the provenance graph: +For this case, AiiDA provides the ``verdi node delete`` command and the :py:func:`~aiida.tools.graph.deletions.delete_nodes` function, to remove the nodes from the provenance graph: .. caution:: Once the data is deleted, there is no way to recover it (unless you made a backup). @@ -787,7 +787,7 @@ The full list of these flags is available from the help command ``verdi node del .. code-block:: python - from aiida.manage.database import delete_nodes + from aiida.tools import delete_nodes pks_to_be_deleted = delete_nodes( [1, 2, 3], dry_run=True, create_forward=True, call_calc_forward=True, call_work_forward=True ) diff --git a/tests/test_nodes.py b/tests/test_nodes.py index 1299317aa9..530ecad83b 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -18,7 +18,7 @@ from aiida.backends.testbase import AiidaTestCase from aiida.common.exceptions import InvalidOperation, ModificationNotAllowed, StoringNotAllowed, ValidationError from aiida.common.links import LinkType -from aiida.manage.database import delete_nodes, delete_group_nodes +from aiida.tools import delete_nodes, delete_group_nodes class TestNodeIsStorable(AiidaTestCase): From 62385f5a7c5570b3c13e9cb3c48404850c87421e Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 6 Jan 2021 16:06:55 +0000 Subject: [PATCH 16/19] add deprecation warning --- aiida/manage/database/delete/nodes.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/aiida/manage/database/delete/nodes.py b/aiida/manage/database/delete/nodes.py index ddb7c0fe37..03a7edc47f 100644 --- a/aiida/manage/database/delete/nodes.py +++ b/aiida/manage/database/delete/nodes.py @@ -9,6 +9,7 @@ ########################################################################### """Functions to delete nodes from the database, preserving provenance integrity.""" from typing import Callable, Iterable, Optional, Set, Tuple, Union +import warnings def delete_nodes( @@ -25,6 +26,12 @@ def delete_nodes( It should now be imported using `from aiida.tools import delete_nodes` """ + from aiida.common.warnings import AiidaDeprecationWarning from aiida.tools import delete_nodes as _delete + warnings.warn( + 'This function has been moved and will be removed in `v2.0.0`.' + 'It should now be imported using `from aiida.tools import delete_nodes`', AiidaDeprecationWarning + ) # pylint: disable=no-member + return _delete(pks, verbosity, dry_run, force, **traversal_rules) From c8c153ab7c609a9185a10e3c59ac824d576e6bcb Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 6 Jan 2021 16:24:00 +0000 Subject: [PATCH 17/19] pin pylint-django < 2.4.0 --- setup.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.json b/setup.json index 9442e1b619..ff7303dc57 100644 --- a/setup.json +++ b/setup.json @@ -98,7 +98,7 @@ "packaging==20.3", "pre-commit~=2.2", "pylint~=2.5.0", - "pylint-django~=2.0", + "pylint-django>=2.0,<2.4.0", "tomlkit~=0.7.0" ], "tests": [ From 8c9975933ad68042c97e6e53279b81e678443cb1 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Thu, 7 Jan 2021 20:58:55 +0000 Subject: [PATCH 18/19] Apply suggestions from code review Co-authored-by: Marnik Bercx --- aiida/cmdline/commands/cmd_code.py | 2 +- aiida/cmdline/commands/cmd_group.py | 2 +- aiida/cmdline/commands/cmd_node.py | 2 +- aiida/tools/graph/deletions.py | 2 +- docs/source/howto/data.rst | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index 03bde2785a..d5e916013b 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -205,7 +205,7 @@ def _dry_run_callback(pks): if not pks or force: return False echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') - return not click.confirm('Shall I continue?') + return not click.confirm('Shall I continue?', abort=True) DELETE_LOGGER.setLevel(verbosity) with override_log_formatter_context('%(message)s'): diff --git a/aiida/cmdline/commands/cmd_group.py b/aiida/cmdline/commands/cmd_group.py index e98c0a3a5a..f623fe74c0 100644 --- a/aiida/cmdline/commands/cmd_group.py +++ b/aiida/cmdline/commands/cmd_group.py @@ -101,7 +101,7 @@ def _dry_run_callback(pks): if not pks or force: return False echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') - return not click.confirm('Shall I continue?') + return not click.confirm('Shall I continue?', abort=True) with override_log_formatter_context('%(message)s'): _, nodes_deleted = delete_group_nodes([group.pk], dry_run=dry_run or _dry_run_callback, **traversal_rules) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 6eb095edeb..1be70e7ee4 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -330,7 +330,7 @@ def _dry_run_callback(pks): if not pks or force: return False echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') - return not click.confirm('Shall I continue?') + return not click.confirm('Shall I continue?', abort=True) with override_log_formatter_context('%(message)s'): _, was_deleted = delete_nodes(pks, dry_run=dry_run or _dry_run_callback, **traversal_rules) diff --git a/aiida/tools/graph/deletions.py b/aiida/tools/graph/deletions.py index 0aaefb49a6..b151f7d3c8 100644 --- a/aiida/tools/graph/deletions.py +++ b/aiida/tools/graph/deletions.py @@ -34,7 +34,7 @@ def delete_nodes( This command will delete not only the specified nodes, but also the ones that are linked to these and should be also deleted in order to keep a consistent provenance - according to the rules explained in the concepts section of the documentation. + according to the rules explained in the Topics - Provenance section of the documentation. In summary: 1. If a DATA node is deleted, any process nodes linked to it will also be deleted. diff --git a/docs/source/howto/data.rst b/docs/source/howto/data.rst index 8f17b34be5..98eea01c89 100644 --- a/docs/source/howto/data.rst +++ b/docs/source/howto/data.rst @@ -511,7 +511,7 @@ Any deletion operation related to groups, by default, will not affect the nodes For example if you delete a group, the nodes that belonged to the group will remain in the database. The same happens if you remove nodes from the group -- they will remain in the database but won't belong to the group anymore. -If you also wish to delete the nodes, when deleting the group, use the ``--delete-nodes``: +If you also wish to delete the nodes, when deleting the group, use the ``--delete-nodes`` option: .. code-block:: console @@ -769,7 +769,7 @@ Deleting data By default, every time you run or submit a new calculation, AiiDA will create for you new nodes in the database, and will never replace or delete data. There are cases, however, when it might be useful to delete nodes that are not useful anymore, for instance test runs or incorrect/wrong data and calculations. -For this case, AiiDA provides the ``verdi node delete`` command and the :py:func:`~aiida.tools.graph.deletions.delete_nodes` function, to remove the nodes from the provenance graph: +For this case, AiiDA provides the ``verdi node delete`` command and the :py:func:`~aiida.tools.graph.deletions.delete_nodes` function, to remove the nodes from the provenance graph. .. caution:: Once the data is deleted, there is no way to recover it (unless you made a backup). From d6eec5e7d82454b202713c0b22744cd37ec2e4b1 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Thu, 7 Jan 2021 21:02:13 +0000 Subject: [PATCH 19/19] remove duplicate --- aiida/cmdline/commands/cmd_code.py | 1 - 1 file changed, 1 deletion(-) diff --git a/aiida/cmdline/commands/cmd_code.py b/aiida/cmdline/commands/cmd_code.py index d5e916013b..b431271c70 100644 --- a/aiida/cmdline/commands/cmd_code.py +++ b/aiida/cmdline/commands/cmd_code.py @@ -207,7 +207,6 @@ def _dry_run_callback(pks): echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') return not click.confirm('Shall I continue?', abort=True) - DELETE_LOGGER.setLevel(verbosity) with override_log_formatter_context('%(message)s'): _, was_deleted = delete_nodes(node_pks_to_delete, dry_run=dry_run or _dry_run_callback)