Code review comment for lp:~tpatil/nova/os-security-groups

Revision history for this message
Tushar Patil (tpatil) wrote :

> I see that this convention from the ec2 code made it to this branch.
>
> if context.is_admin:
> groups = db.security_group_get_all(context)
>
> My preference would be to not list everything for admin users by default, as
> that makes it hard to use admin accounts as users. I'd rather use a separate
> parameter to specify that you in fact want a complete list, or have a separate
> admin api call. That said, I think that servers api behaves like this right
> now (I don't like that either :)

You are right, I have copied this code from Openstack EC2 API and I also agree with you that there should be separate admin API to fetch all security groups.

For now, I will simply remove this code and will add admin API to fetch all security groups sometime later after diablo timeframe.

« Back to merge proposal