Allow error message from remote builder with error type indicator. #698
Conversation
|
|
||
| result.errorMsg = ""; | ||
| if (result.stepStatus != bsSuccess) { | ||
| result.errorMsg += " Log: " + chomp(readFile(result.logFile)); |
There was a problem hiding this comment.
We definitely should not store the log file in the database. It's already way to big.
There was a problem hiding this comment.
Good point. Is it worth storing the tail end of the log (10 lines?) for informative purposes, or would you rather there was no log message information stored?
There was a problem hiding this comment.
I investigated this further and have updated this PR. There are two locations where this diagnostic information could be viewed: (1) on the hydra system directly via journalctl, and (2) on the .../build/NUM#tabs-buildsteps page. I had been focused on (1), and hadn't considered (2). For (2) there is already a mechanism for viewing the logfile, so I've updated to only show the logfile in (1) and not add this information to the database.
| break; | ||
| case BuildResult::InputRejected: | ||
| case BuildResult::OutputRejected: | ||
| result.stepStatus = bsFailed; | ||
| result.canCache = true; | ||
| result.errorMsg += " {inp/out rejected}"; |
There was a problem hiding this comment.
Hm, it's not clear to me what this adds over the stepStatus, which is already stored in the database IIRC.
There was a problem hiding this comment.
Added stepStatus information to logging output (not to database for web ui output) and removed this text where not covered by the stepStatus.
|
@edolstra I've update the PR based on your comments. Please review this again when you get a chance. Thanks! |
Original code was unconditionally erasing any error messages received from a remote builder, which made diagnostics more difficult. This change allows remote builder errors to be displayed in Hydra, including an indicator of the type of error that occurred along with the log message information.