launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/maas/manual_tags into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~gz/maas/manual_tags/+merge/131232

Support "manual" tags. Basically, an empty definition is treated as unmanaged, and the auto updating skips over any such tags. This means the existing admin apis for working with tags can be used to add or remove these tags from any nodes without those entries being changed later.

Of note is that a tag that has a definition, and then is updated to not have one, any matching nodes will be preserved. If this isn't the desired behaviour, the tag can be deleted, then readded with no definition instead.
-- 
https://code.launchpad.net/~gz/maas/manual_tags/+merge/131232
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gz/maas/manual_tags into lp:maas.

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-10-23 09:50:42 +0000
+++ src/maasserver/forms.py	2012-10-24 16:11:28 +0000
@@ -806,6 +806,8 @@
 
     def clean_definition(self):
         definition = self.cleaned_data['definition']
+        if not definition:
+            return ""
         try:
             etree.XPath(definition)
         except etree.XPathSyntaxError as e:

=== modified file 'src/maasserver/models/tag.py'
--- src/maasserver/models/tag.py	2012-10-10 10:25:28 +0000
+++ src/maasserver/models/tag.py	2012-10-24 16:11:28 +0000
@@ -99,7 +99,7 @@
 
     name = CharField(max_length=256, unique=True, editable=True,
                      validators=[RegexValidator(_tag_name_regex)])
-    definition = TextField()
+    definition = TextField(blank=True)
     comment = TextField(blank=True)
 
     objects = TagManager()
@@ -121,6 +121,8 @@
     def populate_nodes(self):
         """Find all nodes that match this tag, and update them."""
         from maasserver.populate_tags import populate_tags
+        if not self.definition:
+            return
         # before we pass off any work, ensure the definition is valid XPATH
         try:
             etree.XPath(self.definition)

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-10-19 13:33:31 +0000
+++ src/maasserver/tests/test_api.py	2012-10-24 16:11:28 +0000
@@ -2884,6 +2884,18 @@
         self.assertEqual({'rebuilding': tag.name}, parsed_result)
         self.assertItemsEqual([node_matching], tag.node_set.all())
 
+    def test_POST_rebuild_leaves_manual_tags(self):
+        tag = factory.make_tag(definition='')
+        node = factory.make_node()
+        node.tags.add(tag)
+        self.assertItemsEqual([node], tag.node_set.all())
+        self.become_admin()
+        response = self.client.post(self.get_tag_uri(tag), {'op': 'rebuild'})
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        self.assertEqual({'rebuilding': tag.name}, parsed_result)
+        self.assertItemsEqual([node], tag.node_set.all())
+
     def test_POST_rebuild_unknown_404(self):
         self.become_admin()
         response = self.client.post(
@@ -2936,6 +2948,24 @@
         self.assertEqual(definition, parsed_result['definition'])
         self.assertTrue(Tag.objects.filter(name=name).exists())
 
+    def test_POST_new_without_definition_creates_tag(self):
+        self.become_admin()
+        name = factory.getRandomString()
+        comment = factory.getRandomString()
+        response = self.client.post(
+            self.get_uri('tags/'),
+            {
+                'op': 'new',
+                'name': name,
+                'comment': comment,
+            })
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        self.assertEqual(name, parsed_result['name'])
+        self.assertEqual(comment, parsed_result['comment'])
+        self.assertEqual("", parsed_result['definition'])
+        self.assertTrue(Tag.objects.filter(name=name).exists())
+
     def test_POST_new_invalid_tag_name(self):
         self.become_admin()
         # We do not check the full possible set of invalid names here, a more