Alon Bar-Lev has posted comments on this change.
Change subject: Adding getBiosInfo API to vdsm cli and merge bios information
with vdsGetCaps
......................................................................
Patch Set 12: (7 inline comments)
....................................................
File vdsm/dmidecodeUtil.py
Line 23:
Line 24:
Line 25: def __leafDict(d, n):
Line 26: for k in d:
Line 27: if type(d[k]) == dict:
I think this == should be is
Line 28: __leafDict(d[k], n)
Line 29: else:
Line 30: n[k] = d[k]
Line 31:
Line 26: for k in d:
Line 27: if type(d[k]) == dict:
Line 28: __leafDict(d[k], n)
Line 29: else:
Line 30: n[k] = d[k]
What about something like:
def __leafDict(d):
ret = {}
for k, v in dict.iteritems():
if type(v) is dict:
ret.update(__leafDict(v))
else:
ret[k] = v
return ret
Line 31:
Line 32:
Line 33: @utils.memoized
Line 34: def getAllDmidecodeInfo():
Line 35: myLeafDict = {}
Line 36: for k in ('system', 'bios', 'cache', 'processor', 'chassis',
'memory'):
Line 37: t = {}
Line 38: __leafDict(dmidecode.__dict__[k](), t)
Line 39: myLeafDict[k] = t
again, why not return dictionary from _leafDict? so that:
myLeafDict[k] = __leafDict(dmidecode.__dict__[k]())
Line 40: return myLeafDict
Line 41:
Line 42:
Line 43: @utils.memoized
Line 43: @utils.memoized
Line 44: def getSystemStructure():
Line 45: dmiInfo = getAllDmidecodeInfo()
Line 46: sysStruct = {}
Line 47: for k1, k2 in [('system', 'Manufacturer'),
Use tuple instead of list, BTW: pep8 passes at this format?
Line 48: ('system', 'Product Name'),
Line 49: ('system', 'Version'),
Line 50: ('system', 'Serial Number'),
Line 51: ('system', 'UUID'),
Line 51: ('system', 'UUID'),
Line 52: ('system', 'Family')]:
Line 53: k = k1 + k2
Line 54: k = k.replace(' ', '')
Line 55: sysStruct[k] = dmiInfo[k1][k2]
hmmm...
sysStruct[(k1+k2).replace(' ', '')] = dmiInfo[k1][k2]
or:
k = (k1+k2).replace(' ', '')
Line 56:
Line 57: return sysStruct
Line 58:
Line 59:
Line 61:
Line 62: def formatData(data):
Line 63: ret = ""
Line 64: for i in data:
Line 65: ret = ret + "\n" + str(i) + " - " + str(data[i])
If this is only for debug I would just print the default python output.
Anyway...
for k, v in data.iteritems():
ret += '\n%s - %s' % (k, v)
Or better:
'\n'.join(['%s - %s' % (k, v) for k, v in data.iteritems()])
Line 66: return ret
Line 67:
Line 68: print(
Line 69: """
Line 95: cache=formatData(d['cache']),
Line 96: processor=formatData(d['processor']),
Line 97: chassis=formatData(d['chassis']),
Line 98: memory=formatData(d['memory'])
Line 99: )
pep8 passes?
I prefer:
print(
"ba bla".format(
bla bla,
bla bla,
bla bla,
)
)
This way you always use one indent, not drawing functions... closure is at the
same indent as statement start, but minor issue...
--
To view, visit http://gerrit.ovirt.org/9258
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic429ef101fcf9047c4b552405314dc7ba9ba07a0
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Andrew Cathrow <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
