launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

[4]

149	+def give_credentials_to_user(user_from, user_dest):
150	+    """Gives one user's credentials to another.

'credentials' is a bit vague don't you think?  How about 'API credentials'?

[5]

57	+from maasserver.models import (
58	+    FileStorage,
59	+    Node,
60	+    SSHKey,
61	+    User,
62	+    )

There is no 'User' class in maasserver.models, it's simply an alias for django.contrib.auth.models.User.  I think it's clearer to import the class from Django.

[6]

106	+def get_owned_nodes_owners():
107	+    """Returns a `QuerySet` of the owners of nodes owned by real users."""
108	+    owner_ids = get_owned_nodes().values_list("owner", flat=True)
109	+    return User.objects.filter(id__in=owner_ids)

Adding '.distinct()' to the query on line 108 might speed things up a bit if there is lots of nodes belonging to just a few users.

-- 
https://code.launchpad.net/~allenap/maas/shared-to-per-tenant-storage/+merge/151858
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/shared-to-per-tenant-storage into lp:maas.