aiidateam / plumpy

A python workflows library that supports writing Processes with a well defined set of inputs and outputs that can be strung together.
https://plumpy.readthedocs.io
Other
8 stars 17 forks source link

Raise exception if `Process.play()` called on closed process #208

Open chrisjsewell opened 3 years ago

chrisjsewell commented 3 years ago

As of https://github.com/aiidateam/plumpy/commit/7004bd96bbaa678b5486a62677e139216877deef, a paused workchain will hang if it is closed then played. This manifested in an aiida-core test (https://github.com/aiidateam/aiida-core/pull/4751)

Perhaps as a "safety measure" Process.play() should raise an exception if it is called on a closed process, and maybe other methods?

sphuber commented 3 years ago

Just want to mention here that so far it has been a common use case to rely on the behavior that playing a non-paused process should be simply a no-op and not raise. There are some bugs in aiida-core that are yet unsolved that can leave the state between node and actual process to be out of sync. Playing all processes can often coax them back into sync. If incorrect plays are going to start raising this might have undesired side effects here

chrisjsewell commented 3 years ago

hey thanks @sphuber can you review https://github.com/aiidateam/aiida-core/pull/4751 quickly?

chrisjsewell commented 3 years ago

Just want to mention here that so far it has been a common use case to rely on the behavior that playing a non-paused process should be simply a no-op and not raise.

but are these non-paused processes already closed?

If incorrect plays are going to start raising this might have undesired side effects here

yep we could always make it no-op, although this may make it harder to understand why playing a process is not working