Fix ForwardModelStep handle_process_timeout... timeout
#9446
Conversation
jonathan-eq
added
the
release-notes:bug-fix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9446 +/- ##
==========================================
- Coverage 91.86% 91.85% -0.02%
==========================================
Files 433 434 +1
Lines 26657 26724 +67
==========================================
+ Hits 24488 24546 +58
- Misses 2169 2178 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -217,7 +217,9 @@ def _run(self) -> Generator[Start | Exited | Running | None]: | |||
| exit_code = process.wait(timeout=self.MEMORY_POLL_PERIOD) | |||
| except TimeoutExpired: | |||
| potential_exited_msg = ( | |||
| self.handle_process_timeout_and_create_exited_msg(exit_code, proc) | |||
| self.handle_process_timeout_and_create_exited_msg( | |||
There was a problem hiding this comment.
I need some explanation what this function actually does?
There was a problem hiding this comment.
It handles how we should react on a timeout while polling the process status. If it returns Exited, we yield it and return, otherwise we try again.
It should fail if the running_time goes above what is configured for the forward_model, but we currently reset the running time every time we timeout, so it never reaches this code path.
The timeout function should kill the entire process group if it is different than its own process group, then return an exited message with an error attached.
This fixes the bug where some code was unreachable after the refactoring in commit #4dc894ca63687476e091f582df5a42045190f7bd
jonathan-eq
force-pushed
the
fix-refactoring-bug
branch
from
01f71bc to
2f265bd
Compare
This fixes the bug where some code was unreachable after the refactoring in commit 4dc894c
Approach
🔧
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"')When applicable