Apply latest corrections from PR

aiida/backends/control.py

@@ -28,18 +28,26 @@ def repository_maintain(
     dry_run: bool = False,
     backend: Optional[Backend] = None,
     **kwargs,
-) -> dict:
-    """Performs maintenance tasks on the repository."""
+) -> None:
+    """Performs maintenance tasks on the repository.
+
+        :param full:
+            flag to perform operations that require to stop using the maintained profile.
+
+        :param dry_run:
+            flag to only print the actions that would be taken without actually executing them.
+
+        :param backend:
+            specific backend in which to apply the maintenance (defaults to current profile).
+    """
 
     if backend is None:
         backend = get_manager().get_backend()
     repository = backend.get_repository()
 
     unreferenced_objects = get_unreferenced_keyset(aiida_backend=backend)
-    if dry_run:
-        MAINTAIN_LOGGER.info(f'Would delete {len(unreferenced_objects)} unreferenced objects ...')
-    else:
-        MAINTAIN_LOGGER.info(f'Deleting {len(unreferenced_objects)} unreferenced objects ...')
+    MAINTAIN_LOGGER.info(f'Deleting {len(unreferenced_objects)} unreferenced objects ...')
+    if not dry_run:
         repository.delete_objects(list(unreferenced_objects))
 
     MAINTAIN_LOGGER.info('Starting repository-specific operations ...')
@@ -50,6 +58,16 @@ def get_unreferenced_keyset(check_consistency: bool = True, aiida_backend: Optio
     """Returns the keyset of objects that exist in the repository but are not tracked by AiiDA.
 
     This should be all the soft-deleted files.
+
+        :param check_consistency:
+            toggle for a check that raises if there are references in the database with no actual object in the
+            underlying repository.
+
+        :param aiida_backend:
+            specific backend in which to apply the operation (defaults to current profile).
+
+        :return:
+            a set with all the objects in the underlying repository that are not referenced in the database.
     """
     from aiida import orm
     MAINTAIN_LOGGER.info('Obtaining unreferenced object keys ...')
@@ -59,17 +77,17 @@ def get_unreferenced_keyset(check_consistency: bool = True, aiida_backend: Optio
 
     repository = aiida_backend.get_repository()
 
-    keyset_backend = set(repository.list_objects())
-    keyset_aiidadb = set(orm.Node.objects(aiida_backend).iter_repo_keys())
+    keyset_repository = set(repository.list_objects())
+    keyset_database = set(orm.Node.objects(aiida_backend).iter_repo_keys())
 
     if check_consistency:
-        keyset_missing = keyset_aiidadb - keyset_backend
+        keyset_missing = keyset_database - keyset_repository
         if len(keyset_missing) > 0:
             raise RuntimeError(
                 'There are objects referenced in the database that are not present in the repository. Aborting!'
             )
 
-    return keyset_backend - keyset_aiidadb
+    return keyset_repository - keyset_database
 
 
 def get_repository_info(statistics: bool = False, backend: Optional[Backend] = None) -> dict:

aiida/cmdline/commands/cmd_storage.py

@@ -89,8 +89,8 @@ def storage_info(statistics):
     from aiida.orm import QueryBuilder
 
     data = {
-        'database': get_database_summary(QueryBuilder, statistics).
-        'repository': get_repository_info(statistics=statistics)
+        'database': get_database_summary(QueryBuilder, statistics),
+        'repository': get_repository_info(statistics=statistics),
     }
 
     echo.echo_dictionary(data, sort_keys=False, fmt='yaml')
@@ -105,15 +105,13 @@ def storage_info(statistics):
 @click.option(
     '--dry-run',
     is_flag=True,
-    help='Run the maintenance in dry-run mode which will print actions that would be taken without actually executing them.'
+    help=
+    'Run the maintenance in dry-run mode which will print actions that would be taken without actually executing them.'
 )
 def storage_maintain(full, dry_run):
     """Performs maintenance tasks on the repository."""
     from aiida.backends.control import repository_maintain
 
-    if dry_run and full:
-        echo.echo_critical('You cannot request both `--dry-run` and `--full` at the same time.')
-
     if full:
         echo.echo_warning(
             '\nIn order to safely perform the full maintenance operations on the internal storage, no other '
@@ -125,7 +123,7 @@ def storage_maintain(full, dry_run):
             '`verdi storage maintain`, without the `--full` flag.\n'
         )
 
-    elif not dry_run:
+    else:
         echo.echo(
             '\nThis command will perform all maintenance operations on the internal storage that can be safely '
             'executed while still running AiiDA. '

aiida/repository/backend/abstract.py

@@ -134,14 +134,12 @@ def maintain(self, dry_run: bool = False, live: bool = True, **kwargs) -> None:
         """Performs maintenance operations.
 
         :param dry_run:
-            flag to only run
+            flag to only print the actions that would be taken without actually executing them.
 
         :param live:
             flag to indicate to the backend whether AiiDA is live or not (i.e. if the profile of the
             backend is currently being used/accessed). The backend is expected then to only allow (and
             thus set by default) the operations that are safe to perform in this state.
-
-        :return: None
         """
 
     @contextlib.contextmanager

aiida/repository/backend/disk_object_store.py

@@ -12,7 +12,7 @@
 
 __all__ = ('DiskObjectStoreRepositoryBackend',)
 
-BYTES_TO_MB = 1 / 1024 ** 2
+BYTES_TO_MB = 1 / 1024**2
 
 
 class DiskObjectStoreRepositoryBackend(AbstractRepositoryBackend):
@@ -125,15 +125,15 @@ def maintain(  # type: ignore # pylint: disable=arguments-differ,too-many-branch
         self,
         dry_run: bool = False,
         live: bool = True,
-        override_pack_loose: bool = None,
-        override_do_repack: bool = None,
-        override_clean_storage: bool = None,
-        override_do_vacuum: bool = None,
+        pack_loose: bool = None,
+        do_repack: bool = None,
+        clean_storage: bool = None,
+        do_vacuum: bool = None,
     ) -> dict:
         """Performs maintenance operations.
 
         :param full:
-            a flag to perform operations that require to stop using the maintained profile.
+            flag to perform operations that require to stop using the maintained profile.
         :param override_pack_loose:
             override flag for forcing the packing of loose files.
         :param override_do_repack:
@@ -148,60 +148,41 @@ def maintain(  # type: ignore # pylint: disable=arguments-differ,too-many-branch
         from aiida.backends.control import MAINTAIN_LOGGER
         DOSTORE_LOGGER = MAINTAIN_LOGGER.getChild('disk_object_store')  # pylint: disable=invalid-name
 
-        pack_loose = True
-        do_repack = not live
-        clean_storage = not live
-        do_vacuum = not live
+        if live and (do_repack or clean_storage or do_vacuum):
+            overrides = {'do_repack': do_repack, 'clean_storage': clean_storage, 'do_vacuum': do_vacuum}
+            keys = ', '.join([key for key, override in overrides if override is True])  # type: ignore
+            raise ValueError(f'The following overrides were enabled but cannot be if `live=True`: {keys}')
 
-        if live and (override_do_repack or override_clean_storage or override_do_vacuum):
-                errmsg = 'a specifically resquest keyword cannot be applied while the profile is in use:\n'
-                if override_do_repack is not None:
-                    errmsg = ' > override_do_repack = {override_do_repack}\n'
-                if override_clean_storage is not None:
-                    errmsg = ' > override_clean_storage = {override_clean_storage}\n'
-                if override_do_vacuum is not None:
-                    errmsg = ' > override_do_vacuum = {override_do_vacuum}\n'
-                raise ValueError(errmsg)
+        pack_loose = True if pack_loose is None else pack_loose
 
-        if override_pack_loose is not None:
-            pack_loose = override_pack_loose
-
-        if override_do_repack is not None:
-            do_repack = override_do_repack
-
-        if override_clean_storage is not None:
-            clean_storage = override_clean_storage
-
-        if override_do_vacuum is not None:
-            do_vacuum = override_do_vacuum
+        if live:
+            do_repack = False
+            clean_storage = False
+            do_vacuum = False
+        else:
+            do_repack = True if do_repack is None else do_repack
+            clean_storage = True if clean_storage is None else clean_storage
+            do_vacuum = True if do_vacuum is None else do_vacuum
 
         if pack_loose:
             files_numb = self.container.count_objects()['loose']
             files_size = self.container.get_total_size()['total_size_loose'] * BYTES_TO_MB
-            if dry_run:
-                DOSTORE_LOGGER.report(f'Would pack all loose files ({files_numb} files occupying {files_size} MB) ...')
-            else:
-                DOSTORE_LOGGER.report(f'Packing all loose files ({files_numb} files occupying {files_size} MB) ...')
+            DOSTORE_LOGGER.report(f'Packing all loose files ({files_numb} files occupying {files_size} MB) ...')
+            if not dry_run:
                 self.container.pack_all_loose()
 
         if do_repack:
             files_numb = self.container.count_objects()['packed']
             files_size = self.container.get_total_size()['total_size_packfiles_on_disk'] * BYTES_TO_MB
-            if dry_run:
-                DOSTORE_LOGGER.report(
-                    f'Would re-pack all pack files ({files_numb} files in packs, occupying {files_size} MB) ...'
-                )
-            else:
-                DOSTORE_LOGGER.report(
-                    f'Re-packing all pack files ({files_numb} files in packs, occupying {files_size} MB) ...'
-                )
+            DOSTORE_LOGGER.report(
+                f'Re-packing all pack files ({files_numb} files in packs, occupying {files_size} MB) ...'
+            )
+            if not dry_run:
                 self.container.repack()
 
         if clean_storage:
-            if dry_run:
-                DOSTORE_LOGGER.report(f'Would clean the repository database (with `vacuum={do_vacuum}`) ...')
-            else:
-                DOSTORE_LOGGER.report(f'Cleaning the repository database (with `vacuum={do_vacuum}`) ...')
+            DOSTORE_LOGGER.report(f'Cleaning the repository database (with `vacuum={do_vacuum}`) ...')
+            if not dry_run:
                 self.container.clean_storage(vacuum=do_vacuum)
 
 

tests/cmdline/commands/test_storage.py

@@ -105,13 +105,6 @@ def mocked_migrate(self):  # pylint: disable=no-self-use
     assert 'passed error message' in result.output
 
 
-def tests_storage_maintain_full_dry(run_cli_command):
-    """Test that ``verdi storage migrate`` detects the cancelling of the interactive prompt."""
-    result = run_cli_command(cmd_storage.storage_maintain, options=['--full', '--dry-run'], raises=True)
-    assert '--dry-run' in result.output.lower()
-    assert '--full' in result.output.lower()
-
-
 def tests_storage_maintain_logging(run_cli_command, monkeypatch, caplog):
     """Test all the information and cases of the storage maintain command."""
     import logging
@@ -141,14 +134,8 @@ def mock_maintain(**kwargs):
     assert ' > dry_run: True' in message_list
 
     with caplog.at_level(logging.INFO):
-        _ = run_cli_command(cmd_storage.storage_maintain, options=['--full'], user_input='Y')
+        run_cli_command(cmd_storage.storage_maintain, options=['--full'], user_input='Y')
 
     message_list = caplog.records[2].msg.splitlines()
     assert ' > full: True' in message_list
     assert ' > dry_run: False' in message_list
-
-    with pytest.raises(AssertionError) as execinfo:
-        _ = run_cli_command(cmd_storage.storage_maintain, options=['--full', '--dry-run'])
-
-    error_message = str(execinfo.value)
-    assert 'cannot request both `--dry-run` and `--full` at the same time' in error_message

tests/repository/backend/test_disk_object_store.py

@@ -250,16 +250,16 @@ def test_get_info(populated_repository):
         {'unpacked': 0, 'packed': 4}
     ),
     (
-        {'live': False, 'override_do_vacuum': False},
+        {'live': False, 'do_vacuum': False},
         {'unpacked': 0, 'packed': 4}
     ),
     (
         {
             'live': False,
-            'override_pack_loose': False,
-            'override_do_repack': False,
-            'override_clean_storage': False,
-            'override_do_vacuum': False,
+            'pack_loose': False,
+            'do_repack': False,
+            'clean_storage': False,
+            'do_vacuum': False,
         },
         {'unpacked': 2, 'packed': 3}
     ),
@@ -276,7 +276,7 @@ def test_maintain(populated_repository, kwargs, output_info):
 @pytest.mark.parametrize('do_vacuum', [True, False])
 def test_maintain_logging(caplog, populated_repository, do_vacuum):
     """Test the logging of the ``maintain`` method."""
-    populated_repository.maintain(live=False, override_do_vacuum=do_vacuum)
+    populated_repository.maintain(live=False, do_vacuum=do_vacuum)
 
     list_of_logmsg = []
     for record in caplog.records:
@@ -294,13 +294,7 @@ def test_maintain_logging(caplog, populated_repository, do_vacuum):
         assert 'vacuum=false' in list_of_logmsg[2].lower()
 
 
-#yapf: disable
-@pytest.mark.parametrize('kwargs', [
-    {'override_do_repack': True},
-    {'override_clean_storage': True},
-    {'override_do_vacuum': True}
-])
-# yapf: enable
+@pytest.mark.parametrize('kwargs', [{'do_repack': True}, {'clean_storage': True}, {'do_vacuum': True}])
 def test_maintain_live_overload(populated_repository, kwargs):
     """Test the ``maintain`` method."""