Code review comment for lp:~reldan/nova/lp766282

> Hi Jay.
> Glad to see you!
>
> Yeap, I'll do it by the separate path. Let'me fix it.
> But there are no error in my code :)

Cool, thanks :)

> 1) We should not raise HTTPNotImplemented exception here. We have malformed
> request without 'type' field and should raise BadRequest
>
> 73 - try:
> 74 - reboot_type = input_dict['reboot']['type']
> 75 - except Exception:
> 76 - raise faults.Fault(exc.HTTPNotImplemented())
>
> 2) More over - we should not raise faults.Fault(exc.HTTPNotImplemented()), we
> should return faults.Fault(exc.HTTPNotImplemented()) instead of.
>
> 3) I catch KeyError in this place, create right log and return the right fault
> - HttpBadRequest()
>
> 13 + try:
> 14 + context = req.environ['nova.context']
> 15 + return actions[key](context, input_dict, id)
> 16 + except Exception, e:
> 17 + LOG.exception(_("Error in action %(key)s: %(e)s")
> %
> 18 + locals())
> 19 + return faults.Fault(exc.HTTPBadRequest())

Well, you may assert the above is the correct behaviour, and it may well be, but I think you can acknowledge that your patch *changes* the existing behaviour (changes from returning a HTTPNotImplementedError to an HTTPBadRequest). We can discuss what's the "correct" behaviour in your future patch, since it's tangential to this particular bug :)

See you on the other merge prop! ;)

-jay