10:13 p.m.
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Pyflakes analyzes programs and detects various errors. It works by
parsing the source file, not importing it, so it is safe to use on
modules with side effects. It's also much faster.
This is important to improve the code quality.
ShaoHe Feng (3):
make pyflakes happly
add template_delete to rollback after create a template
run pyflakes when make check
Makefile.am | 7 +++++++
configure.ac | 7 +++++++
contrib/DEBIAN/control.in | 1 +
contrib/kimchi.spec.fedora.in | 1 +
docs/README.md | 6 +++---
src/kimchi/control/plugins.py | 2 +-
src/kimchi/featuretests.py | 2 --
src/kimchi/mockmodel.py | 6 +++---
src/kimchi/model/storagepools.py | 2 +-
src/kimchi/screenshot.py | 1 -
src/kimchi/template.py | 1 -
tests/test_model.py | 1 +
tests/test_server.py | 1 -
ui/pages/help/gen-index.py | 1 -
14 files changed, 25 insertions(+), 14 deletions(-)
--
1.8.4.2
10:13 p.m.
New subject: [PATCH 1/3] make pyflakes happly
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
make pyflakes happly before introduce pyflakes.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
---
src/kimchi/control/plugins.py | 2 +-
src/kimchi/featuretests.py | 2 --
src/kimchi/mockmodel.py | 6 +++---
src/kimchi/model/storagepools.py | 2 +-
src/kimchi/screenshot.py | 1 -
src/kimchi/template.py | 1 -
tests/test_server.py | 1 -
ui/pages/help/gen-index.py | 1 -
8 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/kimchi/control/plugins.py b/src/kimchi/control/plugins.py
index 8dc2273..12c621e 100644
--- a/src/kimchi/control/plugins.py
+++ b/src/kimchi/control/plugins.py
@@ -18,7 +18,7 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
import kimchi.template
-from kimchi.control.base import Collection, Resource
+from kimchi.control.base import Collection
from kimchi.control.utils import get_class_name, model_fn
from kimchi.control.utils import UrlSubNode
diff --git a/src/kimchi/featuretests.py b/src/kimchi/featuretests.py
index 045f72b..a8d8867 100644
--- a/src/kimchi/featuretests.py
+++ b/src/kimchi/featuretests.py
@@ -20,7 +20,6 @@
import cherrypy
import libvirt
import lxml.etree as ET
-import os
import subprocess
import threading
@@ -28,7 +27,6 @@ import threading
from lxml.builder import E
-from kimchi import config
from kimchi.utils import kimchi_log
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index b842b6c..7c2e2f3 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -51,7 +51,7 @@ from kimchi.model.utils import get_vm_name
from kimchi.model.vms import VM_STATIC_UPDATE_PARAMS
from kimchi.objectstore import ObjectStore
from kimchi.screenshot import VMScreenshot
-from kimchi.utils import pool_name_from_uri, run_command
+from kimchi.utils import pool_name_from_uri
from kimchi.utils import template_name_from_uri
from kimchi.vmtemplate import VMTemplate
@@ -125,7 +125,7 @@ class MockModel(object):
def vm_start(self, name):
self._get_vm(name).info['state'] = 'running'
- info = self._get_vm(name).info
+ self._get_vm(name).info
def vm_stop(self, name):
self._get_vm(name).info['state'] = 'shutoff'
@@ -703,7 +703,7 @@ class MockModel(object):
def add_task(self, target_uri, fn, opaque=None):
id = self.next_taskid
self.next_taskid = self.next_taskid + 1
- task = AsyncTask(id, target_uri, fn, self.objstore, opaque)
+ AsyncTask(id, target_uri, fn, self.objstore, opaque)
return id
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
index fea19f6..92b2496 100644
--- a/src/kimchi/model/storagepools.py
+++ b/src/kimchi/model/storagepools.py
@@ -222,7 +222,7 @@ class StoragePoolModel(object):
try:
poolDef.prepare(conn)
return True
- except Exception as e:
+ except Exception:
return False
def lookup(self, name):
diff --git a/src/kimchi/screenshot.py b/src/kimchi/screenshot.py
index 6c8a599..0040d7c 100644
--- a/src/kimchi/screenshot.py
+++ b/src/kimchi/screenshot.py
@@ -20,7 +20,6 @@
import glob
import os
-import random
import signal
import tempfile
import time
diff --git a/src/kimchi/template.py b/src/kimchi/template.py
index 5707121..9bb2da5 100644
--- a/src/kimchi/template.py
+++ b/src/kimchi/template.py
@@ -20,7 +20,6 @@
import cherrypy
import errno
import json
-import os
from kimchi.config import paths
diff --git a/tests/test_server.py b/tests/test_server.py
index d7b93ad..d06e17a 100644
--- a/tests/test_server.py
+++ b/tests/test_server.py
@@ -17,7 +17,6 @@
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-import json
import os
import unittest
diff --git a/ui/pages/help/gen-index.py b/ui/pages/help/gen-index.py
index 56cbf47..cf7c5d7 100755
--- a/ui/pages/help/gen-index.py
+++ b/ui/pages/help/gen-index.py
@@ -18,7 +18,6 @@
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-import glob
import libxml2
import sys
--
1.8.4.2
8:15 a.m.
New subject: [PATCH 1/3] make pyflakes happly
This seems to be a nice tool to improve code quality! But we need to
check its changes before applying them. Some of them do not make sense.
Take a look at two examples below:
Am 13-03-2014 00:13, schrieb shaohef(a)linux.vnet.ibm.com:
def vm_start(self, name):
self._get_vm(name).info['state'] = 'running'
- info = self._get_vm(name).info
+ self._get_vm(name).info
The variable "info" was actually unused,
but removing the assignment is
not enough. Now we have a useless call to _get_vm. The solution here is
to remove that line completely.
def vm_stop(self, name):
self._get_vm(name).info['state'] = 'shutoff'
@@ -703,7 +703,7 @@ class MockModel(object):
def add_task(self, target_uri, fn, opaque=None):
id = self.next_taskid
self.next_taskid = self.next_taskid + 1
- task = AsyncTask(id, target_uri, fn, self.objstore, opaque)
+ AsyncTask(id, target_uri, fn, self.objstore, opaque)
This situation is
similar to the above one, but looking at the Async
class code, it seems its constructor actually does something important
other than creating the object - it starts a thread. So in this case, I
believe this solution is OK. But I never used the Async class, so I may
be missing something.
9:34 p.m.
New subject: [PATCH 1/3] make pyflakes happly
On 03/13/2014 09:15 PM, Crístian Viana wrote:
This seems to be a nice tool to improve code quality! But we need to
check its changes before applying them. Some of them do not make
sense. Take a look at two examples below:
Am 13-03-2014 00:13, schrieb shaohef(a)linux.vnet.ibm.com:
> def vm_start(self, name):
> self._get_vm(name).info['state'] = 'running'
> - info = self._get_vm(name).info
> + self._get_vm(name).info
The variable "info" was actually unused, but removing the assignment
is not enough. Now we have a useless call to _get_vm. The solution
here is to remove that line completely.
good point.
we can remove this whole line.
will fix in the next version.
> def vm_stop(self, name):
> self._get_vm(name).info['state'] = 'shutoff'
> @@ -703,7 +703,7 @@ class MockModel(object):
> def add_task(self, target_uri, fn, opaque=None):
> id = self.next_taskid
> self.next_taskid = self.next_taskid + 1
> - task = AsyncTask(id, target_uri, fn, self.objstore, opaque)
> + AsyncTask(id, target_uri, fn, self.objstore, opaque)
This situation is similar to the above one, but looking at the Async
class code, it seems its constructor actually does something important
other than creating the object - it starts a thread. So in this case,
I believe this solution is OK. But I never used the Async class, so I
may be missing something.
yes AsyncTask starts a thread.
Ming can you help to check it?
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center
2:34 p.m.
New subject: [PATCH 1/3] make pyflakes happly
On 03/13/2014 10:34 PM, Sheldon wrote:
On 03/13/2014 09:15 PM, Crístian Viana wrote:
> This seems to be a nice tool to improve code quality! But we need to
> check its changes before applying them. Some of them do not make
> sense. Take a look at two examples below:
>
> Am 13-03-2014 00:13, schrieb shaohef(a)linux.vnet.ibm.com:
>> def vm_start(self, name):
>> self._get_vm(name).info['state'] = 'running'
>> - info = self._get_vm(name).info
>> + self._get_vm(name).info
> The variable "info" was actually unused, but removing the assignment
> is not enough. Now we have a useless call to _get_vm. The solution
> here is to remove that line completely.
good point.
we can remove this whole line.
will fix in the next version.
>> def vm_stop(self, name):
>> self._get_vm(name).info['state'] = 'shutoff'
>> @@ -703,7 +703,7 @@ class MockModel(object):
>> def add_task(self, target_uri, fn, opaque=None):
>> id = self.next_taskid
>> self.next_taskid = self.next_taskid + 1
>> - task = AsyncTask(id, target_uri, fn, self.objstore, opaque)
>> + AsyncTask(id, target_uri, fn, self.objstore, opaque)
> This situation is similar to the above one, but looking at the Async
> class code, it seems its constructor actually does something
> important other than creating the object - it starts a thread. So in
> this case, I believe this solution is OK. But I never used the Async
> class, so I may be missing something.
>
>
yes AsyncTask starts a thread.
Ming can you help to check it?
Should AsyncTask has an explicit start method on it, separating
construction from execution? It would make the behavior explicit.
--
Adam King <rak(a)linux.vnet.ibm.com>
IBM CSI
10:13 p.m.
New subject: [PATCH 2/3] add template_delete to rollback after create a template
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We know ModelTests will tear down the template datebase.
But it is more better to delete the template explicitly.
In this way, the test case also can test the template_delete.
Also this can make pyflakes happy.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
---
tests/test_model.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/test_model.py b/tests/test_model.py
index a3d8eff..5f797dc 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -501,6 +501,7 @@ class ModelTests(unittest.TestCase):
orig_params = {'name': 'test-template', 'memory':
1024,
'cpus': 1, 'cdrom': self.kimchi_iso}
inst.templates_create(orig_params)
+ rollback.prependDefer(inst.template_delete, 'test-template')
orig_temp = inst.template_lookup(orig_params['name'])
ident = inst.template_clone('test-template')
--
1.8.4.2
10:31 p.m.
New subject: [PATCH 2/3] add template_delete to rollback after create a template
On 03/13/2014 11:13 AM, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We know ModelTests will tear down the template datebase.
s/datebas/database
But it is more better to delete the template explicitly.
In this way, the test case also can test the template_delete.
Also this can make pyflakes happy.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
---
tests/test_model.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/test_model.py b/tests/test_model.py
index a3d8eff..5f797dc 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -501,6 +501,7 @@ class ModelTests(unittest.TestCase):
orig_params = {'name': 'test-template', 'memory':
1024,
'cpus': 1, 'cdrom': self.kimchi_iso}
inst.templates_create(orig_params)
+ rollback.prependDefer(inst.template_delete, 'test-template')
orig_temp = inst.template_lookup(orig_params['name'])
ident = inst.template_clone('test-template')
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center
10:13 p.m.
New subject: [PATCH 3/3] run pyflakes when make check
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Pyflakes analyzes programs and detects various errors. It works by
parsing the source file, not importing it, so it is safe to use on
modules with side effects. It's also much faster.
This is important to improve the code quality.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
---
Makefile.am | 7 +++++++
configure.ac | 7 +++++++
contrib/DEBIAN/control.in | 1 +
contrib/kimchi.spec.fedora.in | 1 +
docs/README.md | 6 +++---
5 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 2fdafb1..c8fba26 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -71,7 +71,14 @@ PEP8_WHITELIST = \
tests/utils.py \
$(NULL)
+SKIP_PYFLAKES_ERR = src/kimchi/websocket.py
+
check-local:
+ find . -path './.git' -prune -type f -o \
+ -name '*.py' -o -name '*.py.in' | xargs $(PYFLAKES) | \
+ grep -w -v $(SKIP_PYFLAKES_ERR) | \
+ while read LINE; do echo "$$LINE"; false; done
+
$(PEP8) --version
$(PEP8) --filename '*.py,*.py.in' $(PEP8_WHITELIST)
diff --git a/configure.ac b/configure.ac
index 365348f..a625f35 100644
--- a/configure.ac
+++ b/configure.ac
@@ -42,6 +42,13 @@ AM_GNU_GETTEXT([external])
AM_GNU_GETTEXT_VERSION([0.10])
AC_PATH_PROG([CHEETAH], [cheetah], [/usr/bin/cheetah])
+# Checking for pyflakes
+AC_PATH_PROG([PYFLAKES], [pyflakes])
+if test "x$PYFLAKES" = "x"; then
+ AC_MSG_WARN([pyflakes not found])
+fi
+
+
AC_CONFIG_FILES([
po/Makefile.in
po/gen-pot
diff --git a/contrib/DEBIAN/control.in b/contrib/DEBIAN/control.in
index c2b2a40..75c42a6 100644
--- a/contrib/DEBIAN/control.in
+++ b/contrib/DEBIAN/control.in
@@ -24,6 +24,7 @@ Depends: python-cherrypy3 (>= 3.2.0),
open-iscsi,
firewalld
Build-Depends: libxslt,
+ pyflakes,
python-libxml2
Maintainer: Aline Manera <alinefm(a)br.ibm.com>
Description: Kimchi web server
diff --git a/contrib/kimchi.spec.fedora.in b/contrib/kimchi.spec.fedora.in
index bf80104..1d51c87 100644
--- a/contrib/kimchi.spec.fedora.in
+++ b/contrib/kimchi.spec.fedora.in
@@ -31,6 +31,7 @@ Requires: nfs-utils
Requires: iscsi-initiator-utils
BuildRequires: libxslt
BuildRequires: libxml2-python
+BuildRequires: pyflakes
%if 0%{?rhel} == 6
Requires: python-ordereddict
diff --git a/docs/README.md b/docs/README.md
index 4be0e53..9c82540 100644
--- a/docs/README.md
+++ b/docs/README.md
@@ -53,7 +53,7 @@ Install Dependencies
PyPAM m2crypto python-jsonschema rpm-build \
qemu-kvm python-psutil python-ethtool sos \
python-ipaddr python-lxml nfs-utils \
- iscsi-initiator-utils libxslt pyparted
+ iscsi-initiator-utils libxslt pyparted pyflakes
# If using RHEL6, install the following additional packages:
$ sudo yum install python-unittest2 python-ordereddict
# Restart libvirt to allow configuration changes to take effect
@@ -75,7 +75,7 @@ for more information on how to configure your system to access this
repository.
python-pam python-m2crypto python-jsonschema \
qemu-kvm libtool python-psutil python-ethtool \
sosreport python-ipaddr python-lxml nfs-common \
- open-iscsi lvm2 xsltproc python-parted
+ open-iscsi lvm2 xsltproc python-parted pyflakes
Packages version requirement:
python-jsonschema >= 1.3.0
@@ -89,7 +89,7 @@ for more information on how to configure your system to access this
repository.
python-pam python-M2Crypto python-jsonschema \
rpm-build kvm python-psutil python-ethtool \
python-ipaddr python-lxml nfs-client open-iscsi \
- libxslt-tools python-xml python-parted
+ libxslt-tools python-xml python-parted pyflakes
Packages version requirement:
python-psutil >= 0.6.0
--
1.8.4.2
10:13 a.m.
Really happy to see some work on it. In order to make the transition
smoothly, I think we can enable pyflakes file by file instead of in
one monolithic .
2014/3/13 11:13, shaohef(a)linux.vnet.ibm.com:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Pyflakes analyzes programs and detects various errors. It works by
parsing the source file, not importing it, so it is safe to use on
modules with side effects. It's also much faster.
This is important to improve the code quality.
ShaoHe Feng (3):
make pyflakes happly
add template_delete to rollback after create a template
run pyflakes when make check
Makefile.am | 7 +++++++
configure.ac | 7 +++++++
contrib/DEBIAN/control.in | 1 +
contrib/kimchi.spec.fedora.in | 1 +
docs/README.md | 6 +++---
src/kimchi/control/plugins.py | 2 +-
src/kimchi/featuretests.py | 2 --
src/kimchi/mockmodel.py | 6 +++---
src/kimchi/model/storagepools.py | 2 +-
src/kimchi/screenshot.py | 1 -
src/kimchi/template.py | 1 -
tests/test_model.py | 1 +
tests/test_server.py | 1 -
ui/pages/help/gen-index.py | 1 -
14 files changed, 25 insertions(+), 14 deletions(-)
4:06 a.m.
On 03/13/2014 11:13 PM, Shu Ming wrote:
Really happy to see some work on it. In order to make the transition
smoothly, I think we can enable pyflakes file by file instead of in
one monolithic .
Ack.
2014/3/13 11:13, shaohef(a)linux.vnet.ibm.com:
> From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
>
> Pyflakes analyzes programs and detects various errors. It works by
> parsing the source file, not importing it, so it is safe to use on
> modules with side effects. It's also much faster.
>
> This is important to improve the code quality.
>
> ShaoHe Feng (3):
> make pyflakes happly
> add template_delete to rollback after create a template
> run pyflakes when make check
>
> Makefile.am | 7 +++++++
> configure.ac | 7 +++++++
> contrib/DEBIAN/control.in | 1 +
> contrib/kimchi.spec.fedora.in | 1 +
> docs/README.md | 6 +++---
> src/kimchi/control/plugins.py | 2 +-
> src/kimchi/featuretests.py | 2 --
> src/kimchi/mockmodel.py | 6 +++---
> src/kimchi/model/storagepools.py | 2 +-
> src/kimchi/screenshot.py | 1 -
> src/kimchi/template.py | 1 -
> tests/test_model.py | 1 +
> tests/test_server.py | 1 -
> ui/pages/help/gen-index.py | 1 -
> 14 files changed, 25 insertions(+), 14 deletions(-)
>
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center
3715
days inactive
3719
days old
9 comments
5 participants
participants (5)
-
Adam King -
Crístian Viana -
shaohef＠linux.vnet.ibm.com -
Sheldon
-
Shu Ming