Code review comment for lp:~terceiro/lava-dispatcher/master-device-version

On 12/06/2012 08:21 AM, Antonio Terceiro wrote:
> === modified file 'lava_dispatcher/job.py'
> --- lava_dispatcher/job.py 2012-11-24 20:58:07 +0000
> +++ lava_dispatcher/job.py 2012-12-06 14:20:27 +0000
> @@ -148,7 +148,6 @@
>
> metadata = {
> 'target.hostname': self.target,
> - 'target.device_version': self.context.get_device_version(),
> }
>
> if 'device_type' in self.job_data:
> @@ -232,6 +231,7 @@
> self.context.test_data.job_status = 'fail'
> raise
> finally:
> + self.context.test_data.add_metadata({ 'target.device_version': self.context.get_device_version() or 'error'})

Is the get_device_version sufficiently exception-safe? ie - the 3 lines
of code after this are important or the lava-results won't get submitted
properly. I'm not the best at proper error handling, maybe Michael can
speculate. Also - this wraps way past column 80 and is going to make
pylint/pflakes/etc complain.

> if submit_results:
> params = submit_results.get('parameters', {})
> action = lava_commands[submit_results['command']](