Change in vdsm[master]: Makefile: lvm.py into PYFLAKES_BLACKLIST

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Tue Mar 5 03:16:28 UTC 2013


Zhou Zheng Sheng has posted comments on this change.

Change subject: Makefile: lvm.py into PYFLAKES_BLACKLIST
......................................................................


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

Though the warning message from pyflakes is the same as in the bug link, it's not the same bug.

Firstly there is a python 2.7 bug. List comprehension will leak temp variable. For example.

 t = 20
 x = [t for t in [1, 2, 3]]
 print t

The result is 3. This bug is fixed in python 3. dict and set will not leak temp variable in generator, dict and set comprehension. The aforementioned pyflakes bug is reporting variable redefinition in all the cases. Now the pyflakes is fixed and only report variable redefinition for list comprehension. This is all the bug link about.

lvm.py triggers a different pyflakes bug. As mentioned by Eduardo, "lv was not instatiated yet if you reach the else. There are two different lv definition in if - else clauses which are mutually excluyent." The pyflakes is not smart enough to detect the variable "lv" in "else" block in list comprehension is a different variable from the "lv" in the "if" block.

I think put lvm.py into PYFLAKES_BLACKLIST will reduce the check coverage. If there are defects in future lvm.py patches, we will not be able to detect them. I think for now we can just downgrade the pyflakes for a while, or just make a compromise and change the variable name in the list comprehension...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8cdeb0c28c1ee5dc3a18a7688ae60fe641f9a6c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Eyal Edri <eedri at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list