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

Revision history for this message
Rick Harris (rconradharris) wrote :

Nice job, Ed. Some small nits:

> + """Starts the host. NOTE: Currently not feasible, since the host

Per Hacking, this should probably be NOTE(dabo):

> + except TypeError as e:

Two spaces between 'TypeError' and 'as'. (Surprised Pep8 didn't catch this)

> 124 + def host_power_action(self, context, instance_id=None, host=None,
> 125 + action=None):

Should this take an instance_id parameter?

Since compute api requires that 'host' and 'action' are specified, could we have manager.py mirror that?

    def host_power_action(self, context, host, action):
        do stuff

In that case,

110 + return self._call_compute_message("host_power_action", context,
111 + instance_id=None, host=host, params={"action": action})

would become:

    return self._call_compute_message("host_power_action", context,
        host=host, action=action)

> 32 + return []

Out of curiosity, should this raise a 4XX (maybe a 401 error) to let the user know that they are doing something wrong if they are trying to access an admin_only api call?

review: Needs Fixing

« Back to merge proposal