launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Hi Diogo!  Notes about this tests:

70	+    def createadmin(self):
71	+        """Run sudo maas createsuperuser."""
72	+        cmd_output = check_output(
73	+            ["sudo", "maas", "createsuperuser", "--username=%s" % ADMIN_USER,
74	+            "--email=example@xxxxxxxxxxxxx", "--noinput"])
75	+        ## Set password for admin user.
76	+        try:
77	+            admin = User.objects.get(username=ADMIN_USER)
78	+        except User.DoesNotExist:
79	+            admin = User(username=ADMIN_USER)
80	+        admin.set_password(PASSWORD)
81	+        admin.save()
82	+        return cmd_output

First, I'd add an underscore to the name: def create_admin(self).  I see that the maas command it invokes doesn't have it either, but it's an old command and we've tended more towards separating words since then.

Also, Django has a shortcut for "get an object for me, or create it if it did not exist yet":

    admin, created = User.objects.get_or_create(
        username=ADMIN_USER, defaults={'password': PASSWORD})
    if not created:
        admin.set_password(PASSWORD)
        admin.save()

(The "created" indicates whether the object was created.  It's False if the object already existed.  The "defaults" argument provides additional settings to use when creating the object.  In this case another way to do it would be to leave out the "defaults," ignore the value of "created," and just set the password unconditionally.)

Documentation for get_or_create: https://docs.djangoproject.com/en/1.4/ref/models/querysets/#get-or-create


Jeroen
-- 
https://code.launchpad.net/~matsubara/maas/packaging-tests/+merge/123630
Your team MAAS Maintainers is requested to review the proposed merge of lp:~matsubara/maas/packaging-tests into lp:~maas-maintainers/maas/packaging.