Make storage backends pluginnable and dynamically loaded through entry points #5501
Conversation
sphuber
force-pushed
the
feature/5495/storage-backend-entry-points
branch
5 times, most recently
from
2d1ce6e to
1e3e230
Compare
| def downgrade(self, config: ConfigType) -> None: | ||
| for profile in config.get('profiles', {}).values(): | ||
| backend = profile.get('storage', {}).get('backend', None) | ||
| if backend is not None and backend.startswith('core.'): | ||
| profile.setdefault('storage', {})['backend'] = backend[5:] |
There was a problem hiding this comment.
Mmm I'm not sure this is the way we want to do the downgrade. I'm thinking of the case that I have a v9-config with storage['backend'] = core.new_backend, and I downgrade to a version with v8-config to check something and come back. The downgrade will take away the core, but when upgrading it will not recognize the new_backend and just leave it as storage['backend'] = new_backend, which is now broken.
The generic recognition you are doing is good in general, but I think here we need to specifically only take out the core from 'psql_dos' and 'sqlite_zip' like in the upgrade.
It would also be nice to have a warning here too if the profile is neither of those two saying something like "downgrading to an older version of config where storage backends were not pluginable, your current storage won't work here" or something like that.
There was a problem hiding this comment.
Fair, I have made the downgrade only explicit for the known storage backends at this point. I have also added sqlite_temp which was missing.
| @@ -248,6 +248,29 @@ def SchedulerFactory(entry_point_name: str, load: bool = True) -> Optional[Union | |||
| raise_invalid_type_error(entry_point_name, entry_point_group, valid_classes) | |||
|
|
|||
|
|
|||
| def StorageFactory(entry_point_name: str, load: bool = True) -> Optional[Union[EntryPoint, 'StorageBackend']]: | |||
There was a problem hiding this comment.
We need to add some tests for this in the tests.plugins.test_factories.py, right?
(btw I just noticed there are no tests for the GroupFactory either. Not saying to add them in this PR though, hehe, just mentioning)
There was a problem hiding this comment.
Added the tests for the StorageFactory
|
yep, apart from the comments by @ramirezfranciscof, basically good cheers 👍 |
This will allow plugins to dynamically provide custom storage backend implementations. Each profile already defines the class to be used in the `storage.backend` key, however, so far the conversion of that string identifier to the corresponding class was hard coded. Here we add the `aiida.storage` entry point group that registers the two storage backend implementations currently provided by `aiida-core`, namely, `psql_dos` and `sqlite_zip`. The `Profile.storage_cls` property now treats the `storage.backend` key as an entry point and tries to load the corresponding class using the `aiida.plugins.StorageFactory`.
Existing profiles should have the key `storage.backend` be prefixed with `core.` to indicate that those entry points ship with `aiida-core`.
sphuber
force-pushed
the
feature/5495/storage-backend-entry-points
branch
from
1e3e230 to
fa87187
Compare
Fixes #5495
This PR makes storage backends for profiles pluginnable. It achieves it by creating a new entry point group
aiida.storagewhich will allow the registering of implementations ofaiida.orm.implementation.storage_backend.StorageBackend. It adds thePsqlDosStorageBackendandSqliteZipStorageBackendunder thecore.psql_dosandcore.sql_zipentry points, respectively, that ship already withaiida-core. Instead of hard-coding the conversion of thestorage.backendkey from aProfileto these classes, theaiida.manage.configuration.profile.Profile.storage_clsmethod will now use the newaiida.plugins.factories.StorageFactoryto load the class associated with the entry point. This new feature will allow plugins to provide alternative storage backends, as for exampleaiida-s3which provides a storage backend using AWS S3 for the file repository as opposed to the local file system.