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

Hi!

I can understand why you wanted to clean up the try/except redundant blocks of code, Eldar, and you did a good job for the most part. However, in this particular block, you altered the logic of the code:

72 - def _action_reboot(self, input_dict, req, id):
73 - try:
74 - reboot_type = input_dict['reboot']['type']
75 - except Exception:
76 - raise faults.Fault(exc.HTTPNotImplemented())
77 - try:
78 - # TODO(gundlach): pass reboot_type, support soft reboot in
79 - # virt driver
80 - self.compute_api.reboot(req.environ['nova.context'], id)
81 - except:
82 - return faults.Fault(exc.HTTPUnprocessableEntity())
83 + def _action_reboot(self, context, input_dict, id):
84 + reboot_type = input_dict['reboot']['type']
85 + # TODO(gundlach): pass reboot_type, support soft reboot in
86 + # virt driver
87 + self.compute_api.reboot(context, id)
88 return exc.HTTPAccepted()

Note that originally, if the input_dict did not contain the nested key ['reboot']['type'], an HTTPNotImplemented would be thrown. However, in the updated code, a KeyError will be thrown :)

So, there is a bit of a danger in these kind of cleanups, which is generally why it's good to make them a separate patch... in this particular case, due to the change in logic demonstrated above, I think it's best to follow Brian's request and remove the specific cleanups and leave just the bug fix. Then, submit a separate patch with the code cleanups (just remember to fix the above logic error! ;)

Cheers!
jay