Change in vdsm[master]: pep8: clean few small files

asegurap at redhat.com asegurap at redhat.com
Mon Oct 22 07:48:09 UTC 2012


Antoni Segura Puimedon has posted comments on this change.

Change subject: pep8: clean few small files
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(7 inline comments)

Great to see all the code going into pep8 compliance ;-)

....................................................
File vdsm/ksm.py
Line 49:         self.cpuUsage = 0
Line 50: 
Line 51:     def _getKsmdJiffies(self):
Line 52:         return sum(map(int, file('/proc/%s/stat' % self._pid) \
Line 53:                                     .read().split()[13:15]))
'map' is deprecated in favor of list comprehensions/generators. 'file' is deprecated in favor of 'open'.

return sum(int(x) for x in open('/proc/%s/stat' % self._pid).read().split()[13:15])
Line 54: 
Line 55:     def run(self):
Line 56:         start()
Line 57:         try:


Line 72: 
Line 73:     def adjust(self):
Line 74:         """Adjust ksm's vigor
Line 75: 
Line 76:         according to configuration and current memory stress.
I can't make much sense of this docstring's change. Why is a phrase suddenly split with a blank line?
Line 77:         Return whether ksm is running"""
Line 78: 
Line 79:         self._lock.acquire()
Line 80:         try:


Line 86:         return self.state
Line 87: 
Line 88:     def memsharing(self):
Line 89:         try:
Line 90:             return (int(file('/sys/kernel/mm/ksm/pages_sharing').read()))
'file' deprecated in favor of 'open'. Can we not drop the outermost parenthesis?
Line 91:         except:
Line 92:             return 0
Line 93: 
Line 94: 


Line 93: 
Line 94: 
Line 95: def running():
Line 96:     try:
Line 97:         state = int(file('/sys/kernel/mm/ksm/run').read()) & 1 == 1
'file' deprecated in favor of 'open'.
Line 98:         return state
Line 99:     except:
Line 100:         return False
Line 101: 


Line 101: 
Line 102: 
Line 103: def npages():
Line 104:     try:
Line 105:         npages = int(file('/sys/kernel/mm/ksm/pages_to_scan').read())
'file' deprecated in favor of 'open'.
Line 106:         return npages
Line 107:     except:
Line 108:         return 0
Line 109: 


Line 118:     KSM_PARAMS = {'run': 3, 'sleep_millisecs': 0x100000000,
Line 119:                   'pages_to_scan': 0x100000000}
Line 120:     for (k, v) in params.iteritems():
Line 121:         if k not in KSM_PARAMS.iterkeys():
Line 122:             raise Exception('Invalid key in KSM parameter: %s=%s' % (k, v))
Not strictly a pep8 thing. But I was wondering if we shouldn't be raising a more specific KeyError here.
Line 123:         if int(v) < 0 or int(v) >= KSM_PARAMS[k]:
Line 124:             raise Exception('Invalid value in KSM parameter: %s=%s' % (k, v))
Line 125:         with open('/sys/kernel/mm/ksm/%s' % k, 'w') as f:


Line 120:     for (k, v) in params.iteritems():
Line 121:         if k not in KSM_PARAMS.iterkeys():
Line 122:             raise Exception('Invalid key in KSM parameter: %s=%s' % (k, v))
Line 123:         if int(v) < 0 or int(v) >= KSM_PARAMS[k]:
Line 124:             raise Exception('Invalid value in KSM parameter: %s=%s' % (k, v))
Not strictly a pep8 thing. But I was wondering if we shouldn't be raising a more specific ValueError here.
Line 125:         with open('/sys/kernel/mm/ksm/%s' % k, 'w') as f:


--
To view, visit http://gerrit.ovirt.org/8711
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5326e6fb6a22979ab7d6cd6d4e61af746e9a6dd8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list