👌 IMPROVE: Check for recycled circus PID

[dev-zero]

No description provided.

[ramirezfranciscof]

Hey @dev-zero , thanks for the contribution! It seems quite straightforward code-wise, but could you explain a bit the exact objective of this and what kind of problem is addressing in general (maybe you already did this in some issue, in which case, could you link it?)

I guess most of the functionality can be deduced from the change itself but just to be sure intent aligns with execution. 😅

[dev-zero]

@ramirezfranciscof in that part of the code we are looking up whether there's a process for the recorded PID. If there is not we can assume the daemon crashed and we have to restart. If there is we have to be a bit more careful since the PID can be recycled (maybe after a restart of the machine, but also on long-running machines). If a process is found we have to see whether the process is in fact a circus process, and if it is a circus process whether it is started by the same user requesting the start. The last one is a real corner case because it would mean that another user on the same machine is running an aiida daemon and by chance got the same PID as this users aiida daemon before.
Besides, one case is still not caught: a user running multiple aiida daemons from different virtualenvs/conda environments.

[codecov]

Codecov Report

Merging #4858 (9fb013d) into develop (91c1c0b) will decrease coverage by 0.57%.
The diff coverage is 50.00%.

❗ Current head 9fb013d differs from pull request most recent head f9d3a0e. Consider uploading reports for the commit f9d3a0e to get more accurate results

@@             Coverage Diff             @@
##           develop    #4858      +/-   ##
===========================================
- Coverage    80.23%   79.66%   -0.56%     
===========================================
  Files          515      523       +8     
  Lines        36746    37170     +424     
===========================================
+ Hits         29478    29608     +130     
- Misses        7268     7562     +294     

Flag	Coverage Δ
django	74.26% <50.00%> (-0.44%)	⬇️
sqlalchemy	73.18% <50.00%> (-0.44%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/cmdline/utils/daemon.py	70.15% <50.00%> (-0.61%)	⬇️
aiida/__init__.py	59.46% <0.00%> (-30.01%)	⬇️
aiida/orm/computers.py	68.37% <0.00%> (-13.05%)	⬇️
aiida/engine/processes/workchains/restart.py	67.28% <0.00%> (-11.04%)	⬇️
aiida/engine/processes/workchains/workchain.py	85.90% <0.00%> (-7.08%)	⬇️
aiida/tools/graph/deletions.py	83.68% <0.00%> (-6.57%)	⬇️
aiida/schedulers/scheduler.py	76.25% <0.00%> (-6.50%)	⬇️
aiida/engine/processes/workchains/awaitable.py	90.00% <0.00%> (-5.00%)	⬇️
aiida/restapi/common/utils.py	74.08% <0.00%> (-4.90%)	⬇️
aiida/engine/daemon/execmanager.py	62.93% <0.00%> (-4.01%)	⬇️
... and 94 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f99f1e8...f9d3a0e. Read the comment docs.

[unkcpz]

@dev-zero thanks, it looks good to me.

I have another thought about this delete_stale_pid_file check. Since it always sticks right after the client creation (except in cmd_daemon::stop show following, but I guess it's fine to move the line backwards), maybe it is safe to put this check inside get_daemon_client? (discussion required, not required for this PR though)

:

aiida-core/aiida/cmdline/commands/cmd_daemon.py

Lines 191 to 195 in 53c5564

	if not client.is_daemon_running:
	echo.echo('Daemon was not running')
	continue
	delete_stale_pid_file(client)

[dev-zero]

@chrisjsewell can this be merged despite the coverage fail?