Fix CABLE non-zero exit code behaviour#83
Merged
Conversation
This commit fixes the following bugs in `run_tasks()`: - If `subprocess.run` raises an exception, we still attempt to add attributes to the netcdf file. This change fixes this by continuing to the next task immediately on failure. - The command to run CABLE does not redirect the stderr stream of CABLE. As a result, error messages are printed to the stderr of `benchcab`. This change fixes this by redirecting the CABLE stderr to its stdout stream so that the stderr from CABLE is written to `out.txt`. - The error message "Job failed to submit: <cmd>" is printed when CABLE exits with a non-zero exit code which is misleading. This change updates the error message to state that CABLE has failed and the task name of the failed task. Fixes #56
Collaborator
Author
|
Ideally this PR should go in after #82 has been merged (this allows me to test other branches of CABLE which fail in some configurations instead of using my own branches of CABLE which run successfully). |
ccarouge
approved these changes
May 19, 2023
Member
ccarouge
left a comment
ccarouge
left a comment
There was a problem hiding this comment.
There is a small change in the error message but otherwise it is good to go.
| except subprocess.CalledProcessError as err: | ||
| print("Job failed to submit: ", err.cmd) | ||
| except subprocess.CalledProcessError: | ||
| print(f"Error: CABLE returned a non-zero exit code for task {task_name}") |
Member
There was a problem hiding this comment.
Suggested change
| print(f"Error: CABLE returned a non-zero exit code for task {task_name}") | |
| print(f"Error: CABLE returned an error for task {task_name}") |
I'm not sure everyone knows what "a non-zero exit code" means.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit fixes the following bugs in
run_tasks():subprocess.runraises an exception, we still attempt to add attributes to the netcdf file. This change fixes this by continuing to the next task immediately on failure.benchcab. This change fixes this by redirecting the CABLE stderr to its stdout stream so that the stderr from CABLE is written toout.txt.Fixes #56