On 30 January 2013 13:04, Michael Hanselmann <[email protected]> wrote:
> 2013/1/30 Bernardo Dal Seno <[email protected]>:
>> --- a/qa/qa_cluster.py
>> +++ b/qa/qa_cluster.py
>> +# "gnt-cluster info" fields
>> +_CIFIELD_RE = re.compile(r"^[-\s]*([^\s:]+):\s*(\S.*)$")
>> +
>> +
>> +def _GetBoolClusterField(field):
>> + master = qa_config.GetMasterNode()
>> + infocmd = "gnt-cluster info"
>
> Please use a list: ["gnt-cluster", "info"]
This is as in the previous patch.
>> + info_out = qa_utils.GetCommandOutput(master["primary"], infocmd)
>> + ret = None
>> + for l in info_out.splitlines():
>> + m = _CIFIELD_RE.match(l)
>> + # FIXME: There should be a way to specify a field through a hierarchy
>> + if m and m.group(1) == field:
>> + assert ret is None
>> + if m.group(2).lower() == "true":
>
> If you use named groups (“(?P<name>…)” you can use “m.group("name")”.
I'm not a fan of named groups, but I've added them.
>> + ret = True
>> + else:
>> + ret = False
>
> Why not just “ret = (m.group(2).lower() == "true")”
Ok, see interdiff.
> Shouldn't you abort the loop here using “break”? After all you assert
> for “ret” to be None just above.
No, I want to check for bogus duplicates. I've added a comment.
Interdiff:
diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
index 22b6286..9839932 100644
--- a/qa/qa_cluster.py
+++ b/qa/qa_cluster.py
@@ -61,7 +61,7 @@ def _CheckFileOnAllNodes(filename, content):
# "gnt-cluster info" fields
-_CIFIELD_RE = re.compile(r"^[-\s]*([^\s:]+):\s*(\S.*)$")
+_CIFIELD_RE = re.compile(r"^[-\s]*(?P<field>[^\s:]+):\s*(?P<value>\S.*)$")
def _GetBoolClusterField(field):
@@ -83,12 +83,10 @@ def _GetBoolClusterField(field):
for l in info_out.splitlines():
m = _CIFIELD_RE.match(l)
# FIXME: There should be a way to specify a field through a hierarchy
- if m and m.group(1) == field:
+ if m and m.group("field") == field:
+ # Make sure that ignoring the hierarchy doesn't cause a double match
assert ret is None
- if m.group(2).lower() == "true":
- ret = True
- else:
- ret = False
+ ret = (m.group("value").lower() == "true")
if ret is not None:
return ret
raise qa_error.Error("Field not found in cluster configuration: %s" % field)
Bernardo
