launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/nodegroup-name-is-natural-key into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/nodegroup-name-is-natural-key/+merge/117244

This is as discussed with Raphaël.  Well okay, he pointed the whole thing out to me when we were discussing the wider problem of making Node.nodegroup NOT NULL.  While Django wants to have an “id” column as a surrogate key for any model table in the database, it does have a facility for other keys in the data as well.  It calls this natural keys.

In this branch I make NodeGroup.name a natural key so that the dev fixture will be able to define nodes with “nodegroup: [master],” rather than having to refer to the master nodegroup by id.  We'll need some kind of reference once we slap the NOT NULL constraint onto Node.nodegroup: we wouldn't be able to load the dev sample data otherwise!

(The master nodegroup will not be in a fixture, because it requires dynamically-generated data.  But we'll make sure that it's been created by the time the fixture gets loaded, using a database migration.)


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/nodegroup-name-is-natural-key/+merge/117244
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/nodegroup-name-is-natural-key into lp:maas.

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-07-27 10:23:53 +0000
+++ src/maasserver/models/nodegroup.py	2012-07-30 11:41:19 +0000
@@ -92,6 +92,10 @@
         self.cached_master = None
         self.filter(name='master').delete()
 
+    def get_by_natural_key(self, name):
+        """For Django, a node group's name is a natural key."""
+        return self.get(name=name)
+
 
 class NodeGroup(TimestampedModel):
 

=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py	2012-07-27 10:23:53 +0000
+++ src/maasserver/tests/test_nodegroup.py	2012-07-30 11:41:19 +0000
@@ -127,6 +127,18 @@
         self.assertEqual(master, reload_object(groupless_node).nodegroup)
         self.assertNotEqual(master, reload_object(groupful_node).nodegroup)
 
+    def test_get_by_natural_key_looks_up_by_name(self):
+        nodegroup = factory.make_node_group()
+        self.assertEqual(
+            nodegroup, NodeGroup.objects.get_by_natural_key(nodegroup.name))
+
+    def test_get_by_natural_key_will_not_return_other_nodegroup(self):
+        factory.make_node_group()
+        self.assertRaises(
+            NodeGroup.DoesNotExist,
+            NodeGroup.objects.get_by_natural_key,
+            factory.make_name("nonexistent-nodegroup"))
+
 
 class TestNodeGroup(TestCase):