Dan Kenigsberg has posted comments on this change.
Change subject: make misc.pgrep for general usage ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
.................................................... File tests/miscTests.py Line 68: for i in range(3): Line 69: sleepProcs.append(misc.execCmd([EXT_SLEEP, "3"], sync=False, Line 70: sudo=False)) Line 71: Line 72: pids = misc.pgrep("sleep", 'comm') I hope you won't hate me for this, but using two types of quotes for the same thing on the same line throws me off balance. Line 73: for proc in sleepProcs: Line 74: self.assertTrue(proc.pid in pids, "pid %d was not located by pgrep" Line 75: % proc.pid) Line 76:
.................................................... File vdsm/storage/misc.py Line 1109: yield int(pid) Line 1110: Line 1111: Line 1112: def pgrep(statValue, statKey): Line 1113: # see man proc(5) /proc/[pid]/stat for statKey value range please use a proper docstring instead of a comment.
please rename the args to (value, field) to better match the man page language. Line 1114: res = [] Line 1115: for pid in iteratePids(): Line 1116: try: Line 1117: pid = int(pid)
.................................................... File vdsm/utils.py Line 214: res.append(statline[procNameStart + 1:procNameEnd]) Line 215: args = statline[procNameEnd + 2:].split() Line 216: res.append(args[0]) Line 217: res.extend([int(item) for item in args[1:]]) Line 218: return dict(zip(statName, tuple(res))) thanks, this change is cool.
but why did you choose not to use namedtuple? imo, namedtuple fits to this use case like a glove to a hand: stats.nice is nicer than stats['nice'].
btw, why do you convert res to a tuple before zipping it? Line 219: Line 220: Line 221: class TimedSample: Line 222: def __init__(self):
-- To view, visit http://gerrit.ovirt.org/11033 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2034b04a4787d2f222cb98dfe3ffcdbb2a94ebb8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server