verdi profile delete: Make it storage plugin agnostic

[sphuber]

Fixes #5515

The verdi profile delete implementation was hardcoded to work only for
the core.psql_dos storage plugin. However, it is possible for profiles
to use different storage plugins, causing the command to except.

The command now forwards to Config.delete_profile. This method gets
the storage plugin defined for the profile and constructs an instance of
it for the given profile. It then calls delete on the storage instance
if the delete_storage argument is set to True.

The new delete method is defined on the StorageBackend abstract base
class. It is intentionally not made abstract, but rather explicitly
raises NotImplementedError. Otherwise existing storage plugins outside
of aiida-core would no longer be instantiable before they implement
the method.

The various options of verdi profile delete to control what parts of
the storage are deleted, are simplified to just a single switch that
controls whether the storage is deleted or not.

The command now also properly detects if the delete profile was the
default, in which case another profile is marked as the new default if
any profiles remain at all.

[sphuber]

This one is dedicated to Mr. ScopeCreep

[sphuber]

@mbercx this is ready for review now

[mbercx]

Thanks @sphuber! I tried my best to break the command, but could only find a few nitpicks and comments.

[sphuber]

@mbercx I pushed a commit with some further improvements

• The without or including storage deletion message is now in bold red.
• Following my own adage of putting all functionality in the API and have verdi be a thin wrapper: I moved the logic for setting the new default inside Config.delete_profile. The only downside is that when the user deletes multiple profiles at once using verdi profile delete that the default may get reassigned multiple times. But I think that is acceptable.
• The Config.delete_profile now prints the storage config, whether the storage is deleted or not. Ideally it was the storage plugin do it themselves, since they know which config attributes are most important and they can format the logger message in the optimal way, but their delete method won't be called if the storage is not deleted, and it is exactly in this case that we want the old configuration to be printed for the user. So I don't see a better solution for now. The upside is that this way, the config is always printed, whereas if we leave it up to the plugin, they could forget to implement this.

[mbercx]

Thanks @sphuber, love the changes. After a discussion with @giovannipizzi, we're still not sure if it's a good idea to change the default behaviour, since:

1. This will technically be a backwards-incompatible change.
2. Even with all the reporting, it's still tedious to clean up a storage after a profile has been deleted in case the user doesn't know how, and could potentially collect a bunch of Postgres databases and then have no idea which one corresponds to which.

Some thoughts to still consider (maybe discuss at the AiiDA meeting today):

1. Can we make the default storage-dependent? I.e. in case of psql_dos the default is true, for sqlite_zip the default is false, but if the user specifies '--delete-storage/--no-delete-storage' that overrides the default.
2. Would it make sense to keep track of the storage configuration of all deleted profiles somewhere, e.g. in a YAML file called deleted_profiles.yaml in the $AIIDA_PATH/.aiida directory.
3. Alternatively/Additionally, we make the default to delete the storage, but a sqlite_zip storage makes a copy of the archive and stores it in a similar location as the sqlite_dos.

[mbercx]

This prompt should only appear when the force flag is not used, right?

❯ verdi profile delete tmp -f --delete-data
Warning: Deleting profile `tmp`, including all data.
Warning: This operation cannot be undone, are you sure you want to continue?Report: Deleted storage directory at `/Users/mbercx/project/core/.aiida/repository/sqlite_dos_5b7189522ae34118b151f65f618d150c`.
Report: Data storage deleted, configuration was: {'filepath': '/Users/mbercx/project/core/.aiida/repository/sqlite_dos_5b7189522ae34118b151f65f618d150c'}
Warning: `tmp` was the default profile, no profiles remain to set as default.
Success: Profile `tmp` was deleted.

[sphuber]

Damn, my desire to have prompts prefixed with the nice Warning: strikes again...

[sphuber]

Fixed. By the way, I changed the flags to --delete-data/--keep-data because I think that is going to be more clear for most users, rather than "storage" which is kind of vague. You agree?

[mbercx]

Yeah, I like it better! 👍

[mbercx]

Mr. Scope Creep is pleased. 😄