launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1027278 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1027278/+merge/116583

Using the initial-data fixture to create the worker user turned out to be a mistake.  The required hard-coded surrogate database key made it overwrite existing users in existing installations!

An alternative suggested by Raphaël and discussed with Julian was to create the user in a South migration.  Using South for this does not seem entirely appropriate though: it operates at the database level, and we rely extensively on signals being passed at the model level when objects are saved to the database.  Even if it would probably work now, creating a user directly in the database seems like asking for trouble in the long run.

Hence the solution chosen here: the user is cached and encapsulated in a dedicated function anyway.  The only change is that on a miss, the code needs to deal with the possibility that the worker user may not exist yet.  It's exactly the kind of thing we have encapsulation for.  The user is now created fully through the model.  All desired properties of the encapsulating function are already tested and nothing changes there.

The solution may be a reusable pattern for other objects we need to create.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1027278/+merge/116583
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1027278 into lp:maas.

=== removed file 'src/maasserver/fixtures/initial_data.yaml'
--- src/maasserver/fixtures/initial_data.yaml	2012-07-17 10:31:54 +0000
+++ src/maasserver/fixtures/initial_data.yaml	1970-01-01 00:00:00 +0000
@@ -1,13 +0,0 @@
-- model: auth.user
-  pk: 2
-  fields:
-    username: 'maas-nodegroup-worker'
-    first_name: 'Node-group worker'
-    last_name: 'Special user'
-    email: 'maas-nodegroup-worker@localhost'
-    password: '!'
-    is_staff: false
-    is_active: false
-    is_superuser: false
-    last_login: 2012-07-17
-    date_joined: 2012-07-17

=== modified file 'src/maasserver/worker_user.py'
--- src/maasserver/worker_user.py	2012-07-17 08:19:33 +0000
+++ src/maasserver/worker_user.py	2012-07-25 05:50:28 +0000
@@ -29,9 +29,22 @@
 worker_user = None
 
 
+def create_worker_user():
+    """Create and return the worker user."""
+    worker_user = User(
+        username=user_name, first_name="Node-group worker",
+        last_name="Special user", email="maas-nodegroup-worker@localhost",
+        is_staff=False, is_active=False, is_superuser=False)
+    worker_user.save()
+    return worker_user
+
+
 def get_worker_user():
     """Get the system user representing the node-group workers."""
     global worker_user
     if worker_user is None:
-        worker_user = User.objects.get(username=user_name)
+        try:
+            worker_user = User.objects.get(username=user_name)
+        except User.DoesNotExist:
+            worker_user = create_worker_user()
     return worker_user