-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Catch a possible error in STATUS files #252
Conversation
6b89683
to
c19e015
Compare
c19e015
to
5858a95
Compare
This should include an issue and at least an explanation about the problem. |
Fixed! |
This PR is doing an educated guess on what could have happened when the traceback in the original issue occured, adds a test to reproduce it, and then catches the exception to handle the error gracefully. |
tests/test_realization.py
Outdated
@@ -253,6 +253,24 @@ def test_status_load(tmpdir): | |||
assert "FORWARD_MODEL" in status | |||
assert np.isnan(status["DURATION"].values[0]) # Unsupported time syntax | |||
|
|||
with open(str(tmpdir.join("realization-0/STATUS")), "w") as status_fh: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This single test function does a lot! Would be it possible to actually split it to some smaller test functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix.
Instead of crashing, we should ignore the line in a garbled STATUS file like we do for other ways it can be garbled.
5858a95
to
c22b875
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one! 🚀
Instead of crashing, we should ignore the line in a garbled STATUS file like we do for other ways it can be garbled.