plumpy.ProcessListener made persistent

[rikigigi]

solves https://github.com/aiidateam/plumpy/issues/273

We implement the persistence of ProcessListener by deriving the class ProcessListener and EventHelper from persistence.Savable. The class EventHelper is moved to a new file because of a circular import with utils and persistence

[rikigigi]

@sphuber I don't understand why 4 test fail here https://github.com/aiidateam/plumpy/actions/runs/6012847906/job/16309013751?pr=274 ... do you have any idea? For me the call_with_super_check is a little bit obscure...

Thank you very much

[sphuber]

The call_with_super_check is a decorator that can be used for a classes method, which ensures that if it is subclassed, the subclass calls super() or it raises an exception. The problem is that it is not quite sure which subclass it is complaining about. I have looked through your changes, and I don't immediately see any place where you have forgotten to call the super. Will have to look into it later.

[rikigigi]

I think that the reason that prevented call_with_super_check from working was the fact that it used a single counter for all function. In my case it was called multiple times in the stack. Unfortunately to make the persistence of the Listener, I had to introduce some ugly hacks in the test suite to avoid circular references and to correctly compare snapshots.

[rikigigi]

The timeout issue is not present in my local run of the test suite :-(

[rikigigi]

did a simple test with https://github.com/aiidateam/aiida-core/pull/5732/ rebased on current develop + https://github.com/sphuber/aiida-s3 and it worked fine, with the ProcessListener being correctly called on the aiida daemon

[codecov[bot]]

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (44d27d1) 90.83% compared to head (8724b83) 90.76%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #274 +/- ## ========================================== - Coverage 90.83% 90.76% -0.06% ========================================== Files 21 22 +1 Lines 2974 2995 +21 ========================================== + Hits 2701 2718 +17 - Misses 273 277 +4 ``` | [Files](https://app.codecov.io/gh/aiidateam/plumpy/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | Coverage Δ | | |---|---|---| | [src/plumpy/base/utils.py](https://app.codecov.io/gh/aiidateam/plumpy/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS9iYXNlL3V0aWxzLnB5) | `100.00% <100.00%> (ø)` | | | [src/plumpy/utils.py](https://app.codecov.io/gh/aiidateam/plumpy/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS91dGlscy5weQ==) | `82.12% <ø> (+0.61%)` | :arrow_up: | | [src/plumpy/processes.py](https://app.codecov.io/gh/aiidateam/plumpy/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS9wcm9jZXNzZXMucHk=) | `92.47% <87.50%> (+0.02%)` | :arrow_up: | | [src/plumpy/process\_listener.py](https://app.codecov.io/gh/aiidateam/plumpy/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS9wcm9jZXNzX2xpc3RlbmVyLnB5) | `85.19% <78.58%> (-8.14%)` | :arrow_down: | | [src/plumpy/event\_helper.py](https://app.codecov.io/gh/aiidateam/plumpy/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS9ldmVudF9oZWxwZXIucHk=) | `80.00% <80.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[rikigigi]

@sphuber just a ping. Are the changes in the call_with_super_check ok? Thank you

[sphuber]

Sorry for the delay @rikigigi . Could you please give a small writeup as to why these hacks were necessary and what they are doing?

[rikigigi]

I reverted the changes on the call_with_super_check, but a small details that prints the name of the correct function in case of error. They were not necessary.

The issue with the tests is simple: the class ProcessSaver(plumpy.ProcessListener) is defined as a Listener. If now the ProcessListener is persistent, when we save the Process in the tests a mess happens because of the saved instance of the class inside the ProcessListener, that have inside it the ProcessListener itself. So What I did is to modify the ProcessSaver to not store directly inside it the Process to save but to use an external global dictionary where we save the Process by instance id. In this way we avoid the circular reference and we don't have to change all the tests.

[sphuber]

Also, please install pre-commit. Inside the repo, simply run pre-commit install. This way the pre-commit linter autoamtically runs when you commit. This will fix the broken CI.

Finally, maybe you want to merge this into https://github.com/aiidateam/plumpy/tree/support/0.21.x instead. aiida-core is currently stuck on v0.21.x. If we merge this into master it will have to be released with v0.22.x instead, and so you won't be able to install it with aiida-core. So you may want to rebase this onto the support/0.21.x branch

[rikigigi]

ok, thank you! I'm working on it.

[rikigigi]

I addressed the issues and created a different PR for 0.21.x support https://github.com/aiidateam/plumpy/pull/277

[sphuber]

I am closing this as I cherry-picked your fix that was released with the v0.21.10 support in #279 so it is now also on master