launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/utility-cleanup into lp:maas.

Commit message:
Refactor is_local_cluster_UUID into get_local_cluster_UUID.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/utility-cleanup/+merge/148411

I'm doing this for two reasons:
- this utility is more generic refactored that way
- this utility will be used in the test integration code instead of a hacking yet another method to parse that file
-- 
https://code.launchpad.net/~rvb/maas/utility-cleanup/+merge/148411
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/utility-cleanup into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-01-30 15:32:09 +0000
+++ src/maasserver/api.py	2013-02-14 10:34:20 +0000
@@ -178,7 +178,7 @@
     absolute_reverse,
     build_absolute_uri,
     find_nodegroup,
-    is_local_cluster_UUID,
+    get_local_cluster_UUID,
     map_enum,
     strip_domain,
     )
@@ -913,13 +913,16 @@
             "eth0"}]'
         """
         uuid = get_mandatory_param(request.data, 'uuid')
-        is_local_cluster = is_local_cluster_UUID(uuid)
         existing_nodegroup = get_one(NodeGroup.objects.filter(uuid=uuid))
         if existing_nodegroup is None:
             master = NodeGroup.objects.ensure_master()
             # Does master.uuid look like it's a proper uuid?
             if master.uuid in ('master', ''):
                 # Master nodegroup not yet configured, configure it.
+                local_cluster_UUID = get_local_cluster_UUID()
+                is_local_cluster = (
+                    local_cluster_UUID is not None and
+                    local_cluster_UUID == uuid)
                 if is_local_cluster:
                     # Connecting from localhost, accept the cluster
                     # controller.

=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py	2013-01-30 15:32:09 +0000
+++ src/maasserver/utils/__init__.py	2013-02-14 10:34:20 +0000
@@ -15,8 +15,8 @@
     'build_absolute_uri',
     'find_nodegroup',
     'get_db_state',
+    'get_local_cluster_UUID',
     'ignore_unused',
-    'is_local_cluster_UUID',
     'map_enum',
     'strip_domain',
     ]
@@ -113,21 +113,21 @@
     return hostname.split('.', 1)[0]
 
 
-def is_local_cluster_UUID(uuid):
-    """Return whether the given UUID is the UUID of the local cluster."""
+def get_local_cluster_UUID():
+    """Return the UUID of the local cluster (or None if it cannot be found)."""
     try:
         cluster_config = open(settings.LOCAL_CLUSTER_CONFIG).read()
         match = re.search(
             "CLUSTER_UUID=(?P<quote>[\"']?)([^\"']+)(?P=quote)",
             cluster_config)
         if match is not None:
-            return uuid == match.groups()[1]
+            return match.groups()[1]
         else:
-            return False
+            return None
     except IOError as error:
         if error.errno == errno.ENOENT:
             # Cluster config file is not present.
-            return False
+            return None
         else:
             # Anything else is an error.
             raise

=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py	2013-01-30 12:43:55 +0000
+++ src/maasserver/utils/tests/test_utils.py	2013-02-14 10:34:20 +0000
@@ -34,7 +34,7 @@
     build_absolute_uri,
     find_nodegroup,
     get_db_state,
-    is_local_cluster_UUID,
+    get_local_cluster_UUID,
     map_enum,
     strip_domain,
     )
@@ -195,30 +195,23 @@
         self.assertEqual(results, map(strip_domain, inputs))
 
 
-class TestIsLocalClusterUUID(TestCase):
+class TestGetLocalClusterUUID(TestCase):
 
-    def test_is_local_cluster_UUID_returns_false_if_no_config_file(self):
+    def test_get_local_cluster_UUID_returns_None_if_no_config_file(self):
         bogus_file_name = '/tmp/bogus/%s' % factory.make_name('name')
         self.patch(settings, 'LOCAL_CLUSTER_CONFIG', bogus_file_name)
-        self.assertFalse(is_local_cluster_UUID(factory.getRandomUUID()))
+        self.assertIsNone(get_local_cluster_UUID())
 
-    def test_is_local_cluster_UUID_returns_false_if_parsing_fails(self):
+    def test_get_local_cluster_UUID_returns_None_if_parsing_fails(self):
         file_name = self.make_file(contents="wrong content")
         self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
-        self.assertFalse(is_local_cluster_UUID(factory.getRandomUUID()))
-
-    def test_is_local_cluster_UUID_returns_false_if_wrong_UUID(self):
-        uuid = factory.getRandomUUID()
-        other_uuid = factory.getRandomUUID()
-        file_name = self.make_file(contents='CLUSTER_UUID="%s"' % other_uuid)
-        self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
-        self.assertFalse(is_local_cluster_UUID(uuid))
-
-    def test_is_local_cluster_UUID_returns_true_if_local_UUID(self):
+        self.assertIsNone(get_local_cluster_UUID())
+
+    def test_get_local_cluster_UUID_returns_cluster_UUID(self):
         uuid = factory.getRandomUUID()
         file_name = self.make_file(contents='CLUSTER_UUID="%s"' % uuid)
         self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
-        self.assertTrue(is_local_cluster_UUID(uuid))
+        self.assertEqual(uuid, get_local_cluster_UUID())
 
 
 def get_request(origin_ip):