Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ORM: Add the Entity.get_collection classmethod #6107

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Sep 1, 2023

Fixes #6105

@sphuber
Copy link
Contributor Author

sphuber commented Sep 1, 2023

@rikigigi

@rikigigi
Copy link
Member

rikigigi commented Sep 1, 2023

Thank you @sphuber ! I will do some test soon

@rikigigi
Copy link
Member

rikigigi commented Sep 2, 2023

I feel that we are still missing many points where we use the global backend and not the instance backend, see for example sphuber#14 that makes it only for my use case that is the initialization of a ready-to use aiida database.

@sphuber sphuber force-pushed the fix/6105/get-collection-specify-backend branch from 5bad5d8 to 3364209 Compare September 3, 2023 14:37
@sphuber
Copy link
Contributor Author

sphuber commented Sep 3, 2023

Thanks @rikigigi . I have merged your PR. I have found some additional places as well and merged that with your commit. I have now thoroughly searched all the code in aiida.orm to check that the collection is always properly instantiated with the correct backend, and if an entity creates another entity it passes its own backend. I think this should fix the majority of cases, even though it remains of course possible that we have missed some

@sphuber sphuber force-pushed the fix/6105/get-collection-specify-backend branch 2 times, most recently from bf49bc4 to ebfd4d5 Compare September 5, 2023 07:06
sphuber and others added 3 commits September 5, 2023 12:10
This is an alternative for the `collection` class property. Where the
`collection` property uses the current default storage backend, the
`get_collection` allows to specify another specific backend.

The original design intended to support this use-case by allowing a
`Collection` instance to be "called" with a specific backend:

    Entity.collection(backend)

The `.collection` returns a `Collection` instance which is then called
with passing the `backend`, which would return a new `Collection` for
the same entity type, but with the other backend. However, this doesn't
always work, because the `collection` property will load the backend
from the default profile, which will except if not loaded. Although this
scenario is unlikely for normal usage, application developers may use
AiiDA's API with multiple active backends where no default profile is
defined.

The reason for a new method instead of changing the `collection`
property is that that would be backwards incompatible.
The classproperty will try to load the default storage backend first,
before recreating the collection with the specified backend. Not only is
this inefficient as the collection is recreated if the `backend` is not
the current default one, but it can also fail in situations where there
is no default profile is available and the caller wants to directly
specify the backend.
Whenever an ORM entity instance instantiates another entry, it should
explicitly pass its own backend as the storage backend to use. Similarly,
functions that accept a storage backend as an argument, should
consistently pass whenever instantiating a new entity or its collection.

Co-Authored-By: Riccardo Bertossa <[email protected]>
@sphuber sphuber force-pushed the fix/6105/get-collection-specify-backend branch from ebfd4d5 to 480ccd8 Compare September 5, 2023 10:10
@sphuber sphuber merged commit 96667c8 into aiidateam:main Sep 5, 2023
10 checks passed
@sphuber sphuber deleted the fix/6105/get-collection-specify-backend branch September 5, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Computer.configure does not use the Computer instance backend (.collection issue?)
2 participants