Skip to content
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

Merged
merged 2 commits into from
Aug 23, 2024
Merged

Catch a possible error in STATUS files #252

merged 2 commits into from
Aug 23, 2024

Conversation

berland
Copy link
Collaborator

@berland berland commented Aug 22, 2024

Instead of crashing, we should ignore the line in a garbled STATUS file like we do for other ways it can be garbled.

@berland berland self-assigned this Aug 22, 2024
@berland berland force-pushed the avoid_indexerror branch 2 times, most recently from 6b89683 to c19e015 Compare August 22, 2024 13:58
@xjules
Copy link

xjules commented Aug 23, 2024

This should include an issue and at least an explanation about the problem.

@berland berland linked an issue Aug 23, 2024 that may be closed by this pull request
@berland
Copy link
Collaborator Author

berland commented Aug 23, 2024

This should include an issue and at least an explanation about the problem.

Fixed!

@berland
Copy link
Collaborator Author

berland commented Aug 23, 2024

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.

@@ -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:
Copy link

@xjules xjules Aug 23, 2024

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?

Copy link
Collaborator Author

@berland berland Aug 23, 2024

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.
Copy link

@xjules xjules left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! 🚀

@berland berland merged commit 0b1f308 into master Aug 23, 2024
6 checks passed
@berland berland deleted the avoid_indexerror branch August 23, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Observed IndexError from user
2 participants