Code review comment for lp:~ed-leafe/nova/powerstate

Revision history for this message
Trey Morris (tr3buchet) wrote :

I definitely like this better. The admin_only decorator is way better than checking FLAGS.allow_admin_api. I wonder if there is a way to decorate each exposed function with it instead of get_resources().

Out of curiosity, why keep power_action at all? Why combine only reboot and shutdown into power_action? If all of the actions were going to use power_action, it makes sense, but all of your docstrings say "reboot or shutdown". Maybe it would read better if the docstrings were more accurate and startup used power_action as well.

I also think that raising when calling startup should be moved in the virt/connection area. API shouldn't claim to know underlying technology and block the request, the underlying technology should raise if it can't complete the call.

« Back to merge proposal