Code review comment for lp:~rvb/maas/bug-1104215

Revision history for this message
Gavin Panella (allenap) wrote :

Looks good.

[1]

+                    if is_local_cluster:
+                        return get_celery_credentials()
+                    else:
+                        return HttpResponse(
+                            "Cluster registered.  Awaiting admin approval.",
+                            status=httplib.ACCEPTED)

The condition - if is_local_cluster - is a secondhand indicator for
what you should be testing here: that the nodegroup status is
ACCEPTED.

[2]

+    try:
+        cluster_config = file(settings.LOCAL_CLUSTER_CONFIG).read()

`file` is gone in Python 3.x. Use `open`.

[3]

+    except IOError:
+        # Cluster config file is not present.
+        return False

Be more specific here:

    except IOError as error:
        if error.errno == errno.ENOENT:
            # Cluster config file is not present.
            return False
        else:
            # Anything else is an error.
            raise

[4]

+        match = re.search('CLUSTER_UUID="([^"]*)"', cluster_config)
+        if match is not None:
+            return uuid == match.groups()[0]

I know you're probably well aware of this, but: euargh.

[5]

+        match = re.search('CLUSTER_UUID="([^"]*)"', cluster_config)

Just in case someone comes along and removes the quotes, or changes
them to single quotes, or does either of those things in another part
of MAAS's code base, consider the following expression:

    re.search(
        "CLUSTER_UUID=(?P<quote>[\"']?)([^\"']+)(?P=quote)",
        cluster_config)

review: Approve

« Back to merge proposal