fix: kill nix-eval-jobs subprocess on evaluation failure - #908
Conversation
Signed-off-by: WHOIM1205 <rathourprateek8@gmail.com>
|
I think all this does is terminate |
|
HEY @fricklerhandwerk covers failure during ingestion and ensures the subprocess is cleaned up properly let me know if anything else should be added |
| return proc | ||
|
|
||
|
|
||
| def _setup_mocks(): |
There was a problem hiding this comment.
All these helper procedures should be proper fixtures. This is both more readable (idiomatic, shows up in test arguments as dependencies) and easier to work with (naturally composes). For instance, returning a tuple is cumbersome to extend in the future. With fixtures we'd just add another fixture.
|
Now this is an interesting build failure to fix: https://github.com/NixOS/nix-security-tracker/actions/runs/23434730709/job/68187732968?pr=908#step:5:4175 It may amount to adding a package to the Nix expression, but please make sure this builds before pushing more commits. |
Use AsyncMock for perform_evaluation and realtime_batch_process_attributes so await resolves correctly. Convert helpers to pytest fixtures and use arefresh_from_db in async context. Signed-off-by: WHOIM1205 <rathourprateek8@gmail.com>
|
hey @fricklerhandwerk fixed the async mocks and moved the helpers into fixtures should be passing now |
CI does nothing more but |
|
With #909 merged we can reuse quite a bit of the test setup, but you'd need to rebase. This sync/async stuff is a bit messy to get right, but we'll get there. It's going in the right direction already. |
Description
This PR fixes a critical subprocess lifecycle bug where
nix-eval-jobsprocesses were left running as orphans whenever an exception occurred during evaluation ingestion.Previously, failures inside the ingestion loop (e.g. DB errors, integrity issues, timeouts) would mark the evaluation as
CRASHEDbut never terminate the spawned subprocess, leading to severe resource leaks and inconsistent system state.Changes
evaluation_entrypoint()innix_evaluation.pyto properly manage subprocess lifecycletry/finallyblockeval_process.kill()andawait eval_process.wait()are always called on failure pathsreturncode is None)Impact
nix-eval-jobsprocesses consuming CPU and memoryFixes a critical ingestion reliability issue and improves overall system stability under failure conditions.