Thanks for the fixes! A couple nitpick comments inline. v2 and your
responses in v1 resolved all of my previous concerns.
On Tue, Jul 10, 2012 at 11:40 PM, Arun Sharma <arun.sha...@calsoftinc.com>wrote:
> + # Filter out ovs relevant information if --ovs option passed
> + # else collect all information
> + if only_ovs_info:
> + ovs_info_caps = [CAP_NETWORK_STATUS, CAP_SYSTEM_LOGS,
> + CAP_NETWORK_CONFIG]
> + ovs_info_list = ['process-tree']
> + # We cannot use iteritems, since we modify 'data' as we pass
> through
> + for (k, v) in data.items():
> + cap = v['cap']
> + if 'filename' in v:
> + info = k[0]
> + else:
> + info = k
> + if info not in ovs_info_list and cap not in ovs_info_caps:
> + del data[k]
> +
> + try:
> + load_plugins(filter='ovs')
> + except:
> + pass
> + else:
> + try:
> + load_plugins()
> + except:
> + pass
> +
>
This probably will be easier to maintain by having the
try/except outside of the if/else.
filters = set()
if only_ovs_info:
filters.add('ovs')
...
filter = ",".join(filters)
try:
load_plugins(filter=filter)
except:
pass
> for el in xmldoc.documentElement.getElementsByTagName("*"):
> - if el.tagName == "files":
> - newest_first = getBoolAttr(el, 'newest_first')
> - file_output(dir, getText(el.childNodes).split(),
> - newest_first=newest_first)
> - elif el.tagName == "directory":
> - pattern = el.getAttribute("pattern")
> - if pattern == '': pattern = None
> - negate = getBoolAttr(el, 'negate')
> - newest_first = getBoolAttr(el, 'newest_first')
> - tree_output(dir, getText(el.childNodes), pattern and
> re.compile(pattern) or None,
> - negate=negate, newest_first=newest_first)
> - elif el.tagName == "command":
> - label = el.getAttribute("label")
> - if label == '': label = None
> - cmd_output(dir, getText(el.childNodes), label)
> + filters_tmp = el.getAttribute("filters")
> + if filters_tmp == '':
> + filters = []
> + else:
> + filters = filters_tmp.split(',')
> + if filter is None or filter in filters:
>
Nit, you can save this whole level of indentation by doing
if not(filter is None or filter in filters): continue
> + if el.tagName == "files":
> + newest_first = getBoolAttr(el, 'newest_first')
> + file_output(dir, getText(el.childNodes).split(),
> + newest_first=newest_first)
> + elif el.tagName == "directory":
> + pattern = el.getAttribute("pattern")
> + if pattern == '': pattern = None
> + negate = getBoolAttr(el, 'negate')
> + newest_first = getBoolAttr(el, 'newest_first')
> + tree_output(dir, getText(el.childNodes),
> + pattern and re.compile(pattern) or
> None,
> + negate=negate,
> newest_first=newest_first)
> + elif el.tagName == "command":
> + label = el.getAttribute("label")
> + if label == '': label = None
> + cmd_output(dir, getText(el.childNodes), label)
>
Looks good to me otherwise.
-Reid
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
