Code review comment for lp:~terceiro/lava-scheduler/better-logging

After playing around with this a bit..

I think we should remove the check for "is finished with error"

and instead raise a CriticalError in the dispatcher in job.py:

                    if status == 'fail':
                        # XXX mwhudson, 2013-01-17: I have no idea what this
                        # code is doing.
                        logging.warning(
                            "[ACTION-E] %s is finished with error (%s)." %
                            (cmd['command'], err))
                        err_msg = ("Lava failed at action %s with error:"
                                   "%s\n") % (cmd['command'],
                                              unicode(str(err),
                                                      'ascii', 'replace'))
                        if cmd['command'] == 'lava_test_run':
                            err_msg += "Lava failed on test: %s" % \
                                       params.get('test_name', "Unknown")
                        err_msg = err_msg + traceback.format_exc()
- print err_msg
+ raise CriticalError(err_msg)

Notice in this job:

http://community.validation.linaro.org/scheduler/job/825

OperationFailed: Soft reboot failed
OperationFailed: Soft reboot failed
CriticalError: Could not get the Android test image booted properly
<LAVA_DISPATCHER>2013-09-05 02:20:44 AM WARNING: [ACTION-E] boot_linaro_android_image is finished with error (Failed to boot test image.).

The last line looks a bit cryptic, would it not be better if it were like this? (What the expected output of the code snippet above would be)

OperationFailed: Soft reboot failed
OperationFailed: Soft reboot failed
CriticalError: Could not get the Android test image booted properly
CriticalError: boot_linaro_android_image is finished with error (Failed to boot test image.)