Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Saggi Mizrahi has abandoned this change. Change subject: Make libvirtvm.py PEP8 compliant .. Patch Set 2: Abandoned -- To view, visit http://gerrit.ovirt.org/7422 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Dan Kenigsberg has posted comments on this change. Change subject: Make libvirtvm.py PEP8 compliant .. Patch Set 2: Do not submit I believe this issue has been solved by http://gerrit.ovirt.org/6561 . Please abandon. -- To view, visit http://gerrit.ovirt.org/7422 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Saggi Mizrahi has posted comments on this change.
Change subject: Make libvirtvm.py PEP8 compliant
..
Patch Set 2: (2 inline comments)
File vdsm/libvirtvm.py
Line 82: config.getint('vars',
'vm_sample_net_window'))
Line 83:
Line 84: self.addStatsFunction(self.highWrite, self.updateVolumes,
Line 85: self.sampleCpu, self.sampleDisk,
self.sampleDiskLatency,
Line 86: self.sampleNet)
This is function definition and not function invocation
Line 87:
Line 88: def _highWrite(self):
Line 89: if not self._vm._volumesPrepared:
Line 90: # Avoid queries from storage during recovery process
Line 142
Line 143
Line 144
Line 145
Line 146
They are not really comments. This is just useless pasted json.
--
To view, visit http://gerrit.ovirt.org/7422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Saggi Mizrahi has posted comments on this change.
Change subject: Make libvirtvm.py PEP8 compliant
..
Patch Set 1: (3 inline comments)
File vdsm/libvirtvm.py
Line 81: config.getint('vars',
'vm_sample_net_interval'),
Line 82: config.getint('vars',
'vm_sample_net_window'))
Line 83:
Line 84: self.addStatsFunction(self.highWrite, self.updateVolumes,
Line 85: self.sampleCpu, self.sampleDisk,
self.sampleDiskLatency,
Only in function declarations. When using a function you don't need to align
the arguments.
http://www.python.org/dev/peps/pep-0008/#id12
# Extra indentation is not necessary.
foo = long_function_name(
var_one, var_two,
var_three, var_four)
Line 86: self.sampleNet)
Line 87:
Line 88: def _highWrite(self):
Line 89: if not self._vm._volumesPrepared:
Line 118: vmDrive.apparentsize =
int(volSize['apparentsize'])
Line 119:
Line 120: def _sampleCpu(self):
Line 121: state, maxMem, memory, nrVirtCpu, cpuTime =
self._vm._dom.info()
Line 122: return cpuTime / (1000 ** 3)
Spaces around operators.
http://www.python.org/dev/peps/pep-0008/#id20
Line 123:
Line 124: def _sampleDisk(self):
Line 125: if not self._vm._volumesPrepared:
Line 126: # Avoid queries from storage during recovery process
Line 269: 'writeLatency': '0',
Line 270: 'flushLatency': '0'}
Line 271: try:
Line 272: (dLatency['readLatency'], dLatency['writeLatency'],
Line 273: dLatency['flushLatency']) =
_avgLatencyCalc(sInfo[dName],
I don't think the spec specifies that.
Line 274: eInfo[dName])
Line 275: except (KeyError, TypeError):
Line 276: self._log.debug(Disk %s latency not available,
dName)
Line 277: else:
--
To view, visit http://gerrit.ovirt.org/7422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Saggi Mizrahi has posted comments on this change. Change subject: Make libvirtvm.py PEP8 compliant .. Patch Set 1: (1 inline comment) File vdsm/libvirtvm.py Line 269: 'writeLatency': '0', Line 270: 'flushLatency': '0'} Line 271: try: Line 272: (dLatency['readLatency'], dLatency['writeLatency'], Line 273: dLatency['flushLatency']) = _avgLatencyCalc(sInfo[dName], Further more it would break indentation being a multiply of four. Line 274: eInfo[dName]) Line 275: except (KeyError, TypeError): Line 276: self._log.debug(Disk %s latency not available, dName) Line 277: else: -- To view, visit http://gerrit.ovirt.org/7422 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Antoni Segura Puimedon has posted comments on this change.
Change subject: Make libvirtvm.py PEP8 compliant
..
Patch Set 1: No score
(3 inline comments)
As Zhou Zheng Sheng said, there is a patch that is doing the same task, I
reviewed both to the best of my ability and I attach now some comments that
help clarify my earlier comments. I admit that I might be mistaken in my
interpretations so I will leave the score to 0 and hope that somebody will have
a more definitive knowledge about these points than I do.
File vdsm/libvirtvm.py
Line 81: config.getint('vars',
'vm_sample_net_interval'),
Line 82: config.getint('vars',
'vm_sample_net_window'))
Line 83:
Line 84: self.addStatsFunction(self.highWrite, self.updateVolumes,
Line 85: self.sampleCpu, self.sampleDisk,
self.sampleDiskLatency,
note that in the example you pose there is no argument in the first line, thus
it is allowed as you show. I am under the understanding that it is not the case
when there are arguments in the line that opens the brackets.
Line 86: self.sampleNet)
Line 87:
Line 88: def _highWrite(self):
Line 89: if not self._vm._volumesPrepared:
Line 118: vmDrive.apparentsize =
int(volSize['apparentsize'])
Line 119:
Line 120: def _sampleCpu(self):
Line 121: state, maxMem, memory, nrVirtCpu, cpuTime =
self._vm._dom.info()
Line 122: return cpuTime / (1000 ** 3)
You are right about the spaces, I overlooked them, what caught my attentions
was the presence of unneeded parenthesis.
Line 123:
Line 124: def _sampleDisk(self):
Line 125: if not self._vm._volumesPrepared:
Line 126: # Avoid queries from storage during recovery process
Line 269: 'writeLatency': '0',
Line 270: 'flushLatency': '0'}
Line 271: try:
Line 272: (dLatency['readLatency'], dLatency['writeLatency'],
Line 273: dLatency['flushLatency']) =
_avgLatencyCalc(sInfo[dName],
It is a block, thus, I believe (although I could very well be mistaken), should
follow the rules of indentation inside parenthesis like the example in:
http://www.python.org/dev/peps/pep-0008/#maximum-line-length
Note:
- The alignment to beginning parenthesis in the if.
- The alignment in the raise ValueError statement (no multiples of four but
aligned to call beginning parenthesis).
Line 274: eInfo[dName])
Line 275: except (KeyError, TypeError):
Line 276: self._log.debug(Disk %s latency not available,
dName)
Line 277: else:
--
To view, visit http://gerrit.ovirt.org/7422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
oVirt Jenkins CI Server has posted comments on this change. Change subject: Make libvirtvm.py PEP8 compliant .. Patch Set 2: Build Successful http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/644/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/7422 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
oVirt Jenkins CI Server has posted comments on this change. Change subject: Make libvirtvm.py PEP8 compliant .. Patch Set 1: Build Successful http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/601/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/7422 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Antoni Segura Puimedon has posted comments on this change.
Change subject: Make libvirtvm.py PEP8 compliant
..
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
Just some indentations do not seem to follow completely pep8.
File vdsm/libvirtvm.py
Line 81: config.getint('vars',
'vm_sample_net_interval'),
Line 82: config.getint('vars',
'vm_sample_net_window'))
Line 83:
Line 84: self.addStatsFunction(self.highWrite, self.updateVolumes,
Line 85: self.sampleCpu, self.sampleDisk,
self.sampleDiskLatency,
If I am not mistaken, according to PEP8, if you put at least one parameter
after opening parenthesis, the indentation of the rest of the lines before the
closing parenthesis must be column of opening parenthesis + 1 (as it was
before).
Line 86: self.sampleNet)
Line 87:
Line 88: def _highWrite(self):
Line 89: if not self._vm._volumesPrepared:
Line 118: vmDrive.apparentsize =
int(volSize['apparentsize'])
Line 119:
Line 120: def _sampleCpu(self):
Line 121: state, maxMem, memory, nrVirtCpu, cpuTime =
self._vm._dom.info()
Line 122: return cpuTime / (1000 ** 3)
I think the previous version was already PEP8 compliant.
Line 123:
Line 124: def _sampleDisk(self):
Line 125: if not self._vm._volumesPrepared:
Line 126: # Avoid queries from storage during recovery process
Line 269: 'writeLatency': '0',
Line 270: 'flushLatency': '0'}
Line 271: try:
Line 272: (dLatency['readLatency'], dLatency['writeLatency'],
Line 273: dLatency['flushLatency']) =
_avgLatencyCalc(sInfo[dName],
Shouldn't dLatency here be aligned to the previous line one?
Line 274: eInfo[dName])
Line 275: except (KeyError, TypeError):
Line 276: self._log.debug(Disk %s latency not available,
dName)
Line 277: else:
Line 428: self._vm._vmStats.cont()
Line 429: raise
Line 430: else:
Line 431: hooks.before_vm_migrate_source(self._vm._dom.XMLDesc(0),
Line 432: self._vm.conf)
Alignment like in the first comment. Idem in 437, 620
Line 433: response =
self.destServer.migrationCreate(self._machineParams)
Line 434: if response['status']['code']:
Line 435: self.status = response
Line 436: raise RuntimeError('migration destination error: ' +
--
To view, visit http://gerrit.ovirt.org/7422
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Make libvirtvm.py PEP8 compliant
Zhou Zheng Sheng has posted comments on this change. Change subject: Make libvirtvm.py PEP8 compliant .. Patch Set 1: The same work has been done in http://gerrit.ovirt.org/#/c/7422/ . -- To view, visit http://gerrit.ovirt.org/7422 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32570c2d1a2efa4a5446caa6d973a10b4c84d996 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
