launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@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'?

I've renamed the function and updated the docstring.

> [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.

Done.

> [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.

Done.

-- 
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.