Antoni Segura Puimedon has posted comments on this change.
Change subject: tc: list some information about filters ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
Just for the elif/continue thing, the docstring is just that I find it helpful to know if I expect a list or a generator.
.................................................... File vdsm/tc.py Line 92: Filter = namedtuple('Filter', 'target prio') Line 93: Line 94: Line 95: def filters(bridge, parent, out=None): Line 96: "Return a (very) limitted information about tc filters on bridge dev" Maybe it could be mentioned in the docstring that what is returned is a generators: "Return a generator that provides (very) limited information for each of the tc filters on the bridge device." Line 97: Line 98: if out is None: Line 99: out = _process_request([EXT_TC, 'filter', 'show', 'dev', bridge, Line 100: 'parent', parent])
Line 102: prio = target = None Line 103: for line in out.splitlines(): Line 104: if line.startswith('filter protocol ip pref '): Line 105: prio = line.split()[4] Line 106: if line.startswith('\taction order '): In case the previous if branch proved to be true, doing this check is unnecessary, we could do an 'elif' or put a continue in the previous line. (the same applies to line 110. Line 107: elems = line.split() Line 108: if elems[3] == 'mirred' and elems[4] == '(Egress': Line 109: target = elems[-2][:-1] Line 110: if line == ' ':
-- To view, visit http://gerrit.ovirt.org/7298 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I22953e6d09f846cae18d15a54cd6e5c9e4519e20 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server