ORM: raise when trying to pickle instance of Entity
#5549
Conversation
The ORM entities should not be pickled since it is not clear whether the connection to the storage backend should be included as well and if it were, implementing that in a consistent way across implementations is not feasible. Therefore, we now raise an explicit an `InvalidOperation` when an instance of `Entity` is being pickled. This is achieved by overriding the `__getstate__` dunder method which will be called by the `pickle` module when dumping a class instance.
There was a problem hiding this comment.
Thanks @sphuber
What if the connection to the storage backend was not included?
Would the resulting unpickled orm objects still be usable?
What I don't know is whether anybody has been trying to pickle these so far... this may warrant a question in "Discussions"?
| @@ -217,6 +218,10 @@ def __init__(self, backend_entity: BackendEntityType) -> None: | |||
| self._backend_entity = backend_entity | |||
| call_with_super_check(self.initialize) | |||
|
|
|||
| def __getstate__(self): | |||
| """Prevent an ORM entity instance from being pickled.""" | |||
| raise InvalidOperation('pickling of AiiDA ORM instances is not supported.') | |||
There was a problem hiding this comment.
Can you point the user to another method of serialization that they can use?
As I mentioned in the issue, I think Entties should definitely not be pickled; what they are is essentially "live views" of a stored item (e.g. database row). |
I think the only case we are aware of is of @jbweston which even failed because there is a recursion error. I am not sure what the actual use case was, but it is very likely that this wouldn't have worked anyway. So I think we should officially block this. There are other open issues to provide a base functionality for (de)serializing entities. |
|
@jbweston Could you comment on your use case? I assume it was about transporting some of the orm entities to the compute node via cloudpickle (?). Of course any operations that would involve the database can't work there... |
|
@ltalirz something like that; indeed it's not an important usecase. |
Fixes #5285
The ORM entities should not be pickled since it is not clear whether the
connection to the storage backend should be included as well and if it
were, implementing that in a consistent way across implementations is
not feasible.
Therefore, we now raise an explicit an
InvalidOperationwhen aninstance of
Entityis being pickled. This is achieved by overridingthe
__getstate__dunder method which will be called by thepicklemodule when dumping a class instance.