Code review comment for lp:~chemikadze/nova/contrib-extention-networks

Sorry for the delay in review,

66 + def _network_get_autodetect(self, context, id):

I don't love the need for this function, but I suppose it's a design decision to allow for multiple types of network identifiers to be passed to show/delete/index/etc. It's probably not something I'd hold the merge prop up on, but just stating that I'd rather only have 1 type of input instead of 2 or 3.

122 + except exception.NetworkNotFound:
123 + query_params = urlparse.parse_qs(req.environ['QUERY_STRING'])
124 + cidrs = query_params.get('cidr')
125 + if cidrs:
126 + for cidr in cidrs:
127 + if id == 'disassociate':
128 + self.disassociate(req, cidr)
129 + elif id == 'delete':
130 + self.delete(req, cidr)
131 + return exc.HTTPAccepted()

At first this exception handling was a bit confusing to me, then I realized it was because this method is handling multiple cases:

DELETE /v1.1/os-networks/networks/{id}
DELETE /v1.1/os-networks/networks/delete?cidr={cidr}
DELETE /v1.1/os-networks/networks/disassociate?cidr={cidr}

You're using special identifiers 'delete' and 'disassociate' to allow for the deletion or disassociation of multiple networks at the same time. IMO this is something we should avoid and I'd rather have one way to do things instead of 2. Also there aren't any prior API calls that I'm aware of that do this (multiple deletions at the same time) so it would be more 'standard' to remove this feature and only allow deletions and disassociations by integer identifier. It's quite confusing to call DELETE on /v1.1/os-networks/networks/disassociate, it's double negative-ish.

447 + "NetworkAdmin",

Can you replace these tabs with spaces?