Hi Crístian,

I like the idea of cleanup,  but for python projects,  we should comply with PEP8[1], which limit all lines to a maximum of 79 characters.   So after we can all files pep clean,  it will resolve the line length problem too.

If you have interest in the pep8 clean,  you could use the command line tool pep8 and or vim plugin[2] to check if a python file is pep8 clean.  After the cleanup,  we need put it to the list of PEP8_WHITELIST in Makefile.am.  It can guarantee the files are checked when 'make check-local'  is executed.

[1] http://www.python.org/dev/peps/pep-0008/#maximum-line-length
[2] https://github.com/kimchi-project/kimchi/wiki/PEP8-Checking-Using-Syntastic


On 01/22/2014 04:04 AM, Crístian Viana wrote:
In order to make the code more consistent, set a maximum of 80
characters per line in Python files.

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com>
---
 src/kimchi/mockmodel.py   | 24 ++++++++-----
 src/kimchi/model.py       | 88 ++++++++++++++++++++++++++++-------------------
 src/kimchi/objectstore.py |  3 +-
 src/kimchi/osinfo.py      | 18 ++++++----
 src/kimchi/scan.py        |  7 ++--
 src/kimchi/template.py    |  3 +-
 src/kimchi/vmtemplate.py  |  7 ++--
 tests/test_osinfo.py      |  3 +-
 8 files changed, 96 insertions(+), 57 deletions(-)

diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index 4ef3fa6..1b345ae 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -61,7 +61,8 @@ class MockModel(object):
         self.distros = self._get_distros()
 
     def get_capabilities(self):
-        return {'libvirt_stream_protocols': ['http', 'https', 'ftp', 'ftps', 'tftp'],
+        return {'libvirt_stream_protocols': ['http', 'https', 'ftp', 'ftps',
+                                             'tftp'],
                 'qemu_stream': True,
                 'screenshot': True,
                 'system_report_tool': True}
@@ -208,10 +209,12 @@ class MockModel(object):
         ident = name
 
         new_storagepool = new_t.get(u'storagepool', '')
+        new_name = kimchi.model.pool_name_from_uri(new_storagepool)
         try:
-            self._get_storagepool(kimchi.model.pool_name_from_uri(new_storagepool))
+            self._get_storagepool(new_name)
         except Exception as e:
-            raise InvalidParameter("Storagepool specified is not valid: %s." % e.message)
+            raise InvalidParameter("Storagepool specified is not valid: %s." %
+                                   e.message)
 
         for net_name in params.get(u'networks', []):
             try:
@@ -303,7 +306,8 @@ class MockModel(object):
                 pool.info['autostart'] = False
         except KeyError, item:
             raise MissingParameter(item)
-        if name in self._mock_storagepools or name in (kimchi.model.ISO_POOL_NAME,):
+        if (name in self._mock_storagepools or
+            name in (kimchi.model.ISO_POOL_NAME,)):
             raise InvalidOperation("StoragePool already exists")
         self._mock_storagepools[name] = pool
         return name
@@ -605,9 +609,11 @@ class MockVMTemplate(VMTemplate):
         try:
             pool = self.model._get_storagepool(pool_name)
         except NotFoundError:
-            raise InvalidParameter('Storage specified by template does not exist')
+            raise InvalidParameter('Storage specified by template does '
+                                   'not exist')
         if pool.info['state'] != 'active':
-            raise InvalidParameter('Storage specified by template is not active')
+            raise InvalidParameter('Storage specified by template is '
+                                   'not active')
 
         return pool
 
@@ -634,13 +640,15 @@ class MockVM(object):
         self.networks = template_info['networks']
         self.info = {'state': 'shutoff',
                      'stats': "{'cpu_utilization': 20, 'net_throughput' : 35, \
-                                'net_throughput_peak': 100, 'io_throughput': 45, \
+                                'net_throughput_peak': 100, \
+                                'io_throughput': 45, \
                                 'io_throughput_peak': 100}",
                      'uuid': self.uuid,
                      'memory': template_info['memory'],
                      'cpus': template_info['cpus'],
                      'icon': None,
-                     'graphics': {'type': 'vnc', 'listen': '0.0.0.0', 'port': None}
+                     'graphics': {'type': 'vnc', 'listen': '0.0.0.0',
+                                  'port': None}
                      }
         self.info['graphics'].update(template_info['graphics'])
 
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index ea528c6..37d98dc 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -64,7 +64,8 @@ from kimchi import xmlutils
 from kimchi.asynctask import AsyncTask
 from kimchi.distroloader import DistroLoader
 from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError
-from kimchi.exception import MissingParameter, NotFoundError, OperationFailed, TimeoutExpired
+from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
+from kimchi.exception import TimeoutExpired
 from kimchi.featuretests import FeatureTests
 from kimchi.iscsi import TargetClient
 from kimchi.isoinfo import IsoImage
@@ -155,7 +156,8 @@ class Model(object):
         # Please add new possible debug report command here
         # and implement the report generating function
         # based on the new report command
-        self.report_tools = ({'cmd': 'sosreport --help', 'fn': self._sosreport_generate},
+        self.report_tools = ({'cmd': 'sosreport --help',
+                              'fn': self._sosreport_generate},
                              {'cmd': 'supportconfig -h', 'fn':None},
                              {'cmd': 'linuxexplorers --help', 'fn':None})
 
@@ -185,18 +187,18 @@ class Model(object):
             try:
                 net = conn.networkDefineXML(xml)
             except libvirt.libvirtError, e:
-                cherrypy.log.error(
-                    "Fatal: Cannot create default network because of %s, exit kimchid" % e.message,
-                    severity=logging.ERROR)
+                cherrypy.log.error("Fatal: Cannot create default network "
+                                   "because of %s, exit kimchid" %
+                                   e.message, severity=logging.ERROR)
                 sys.exit(1)
 
         if net.isActive() == 0:
             try:
                 net.create()
             except libvirt.libvirtError, e:
-                cherrypy.log.error(
-                    "Fatal: Cannot activate default network because of %s, exit kimchid" % e.message,
-                    severity=logging.ERROR)
+                cherrypy.log.error("Fatal: Cannot activate default network "
+                                   "because of %s, exit kimchid" %
+                                   e.message, severity=logging.ERROR)
                 sys.exit(1)
 
     def _default_pool_check(self):
@@ -210,9 +212,9 @@ class Model(object):
             pass
         except OperationFailed as e:
             # path used by other pool or other reasons of failure, exit
-            cherrypy.log.error(
-                "Fatal: Cannot create default pool because of %s, exit kimchid" % e.message,
-                severity=logging.ERROR)
+            cherrypy.log.error("Fatal: Cannot create default pool because of "
+                               "%s, exit kimchid" %
+                               e.message, severity=logging.ERROR)
             sys.exit(1)
 
         if self.storagepool_lookup('default')['state'] == 'inactive':
@@ -477,12 +479,15 @@ class Model(object):
 
         for key, val in params.items():
             if key in VM_STATIC_UPDATE_PARAMS:
-                new_xml = xmlutils.xml_item_update(new_xml, VM_STATIC_UPDATE_PARAMS[key], val)
+                new_xml = xmlutils.xml_item_update(new_xml,
+                                                   VM_STATIC_UPDATE_PARAMS[key],
+                                                   val)
 
         try:
             if 'name' in params:
                 if state == 'running':
-                    raise InvalidParameter("vm name can just updated when vm shutoff")
+                    raise InvalidParameter("vm name can just updated when "
+                                           "vm shutoff")
                 else:
                     dom.undefine()
             conn = self.conn.get()
@@ -506,7 +511,8 @@ class Model(object):
         info = dom.info()
         state = Model.dom_state_map[info[0]]
         screenshot = None
-        graphics_type, graphics_listen, graphics_port = self._vm_get_graphics(name)
+        graphics = self._vm_get_graphics(name)
+        graphics_type, graphics_listen, graphics_port = graphics
         graphics_port = graphics_port if state == 'running' else None
         try:
             if state == 'running':
@@ -628,7 +634,8 @@ class Model(object):
         t = self._get_template(t_name, vm_overrides)
 
         if not self.qemu_stream and t.info.get('iso_stream', False):
-            raise InvalidOperation("Remote ISO image is not supported by this server.")
+            raise InvalidOperation("Remote ISO image is not supported by "
+                                   "this server.")
 
         t.validate()
         vol_list = t.fork_vm_storage(vm_uuid)
@@ -639,7 +646,7 @@ class Model(object):
             with self.objstore as session:
                 session.store('vm', vm_uuid, {'icon': icon})
 
-        libvirt_stream = False if len(self.libvirt_stream_protocols) == 0 else True
+        libvirt_stream = len(self.libvirt_stream_protocols) != 0
         graphics = params.get('graphics')
 
         xml = t.to_vm_xml(name, vm_uuid,
@@ -718,7 +725,8 @@ class Model(object):
         try:
             self._get_storagepool(pool_name_from_uri(new_storagepool))
         except Exception as e:
-            raise InvalidParameter("Storagepool specified is not valid: %s." % e.message)
+            raise InvalidParameter("Storagepool specified is not valid: %s." %
+                                   e.message)
 
         for net_name in params.get(u'networks', []):
             try:
@@ -1041,7 +1049,8 @@ class Model(object):
             with self.objstore as session:
                 session.delete('scanning', pool_name)
         except Exception, e:
-            kimchi_log.debug("Exception %s occured when cleaning scan result" % e.message)
+            kimchi_log.debug("Exception %s occured when cleaning scan result" %
+                             e.message)
 
     def _do_deep_scan(self, params):
         scan_params = dict(ignore_list=[])
@@ -1054,10 +1063,11 @@ class Model(object):
                 if res['state'] == 'active':
                     scan_params['ignore_list'].append(res['path'])
             except Exception, e:
-                kimchi_log.debug("Exception %s occured when get ignore path" % e.message)
+                kimchi_log.debug("Exception %s occured when get ignore path" %
+                                 e.message)
 
-        params['path'] = scan_params['pool_path'] = self.scanner.scan_dir_prepare(
-            params['name'])
+        path = self.scanner.scan_dir_prepare(params['name'])
+        params['path'] = scan_params['pool_path'] = path
         task_id = self.add_task('', self.scanner.start_scan, scan_params)
         # Record scanning-task/storagepool mapping for future querying
         with self.objstore as session:
@@ -1252,8 +1262,8 @@ class Model(object):
                 bootable = True
             except IsoFormatError:
                 bootable = False
-            res.update(
-                dict(os_distro=os_distro, os_version=os_version, path=path, bootable=bootable))
+            res.update(dict(os_distro=os_distro, os_version=os_version,
+                            path=path, bootable=bootable))
 
         return res
 
@@ -1315,7 +1325,8 @@ class Model(object):
     def _sosreport_generate(self, cb, name):
         command = 'sosreport --batch --name "%s"' % name
         try:
-            retcode = subprocess.call(command, shell=True, stdout=subprocess.PIPE)
+            retcode = subprocess.call(command, shell=True,
+                                      stdout=subprocess.PIPE)
             if retcode < 0:
                 raise OperationFailed('Command terminated with signal')
             elif retcode > 0:
@@ -1360,7 +1371,8 @@ class Model(object):
         for helper_tool in self.report_tools:
             try:
                 retcode = subprocess.call(helper_tool['cmd'], shell=True,
-                                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                                          stdout=subprocess.PIPE,
+                                          stderr=subprocess.PIPE)
                 if retcode == 0:
                     return helper_tool['fn']
             except Exception, e:
@@ -1450,9 +1462,11 @@ class LibvirtVMTemplate(VMTemplate):
             conn = self.conn.get()
             pool = conn.storagePoolLookupByName(pool_name)
         except libvirt.libvirtError:
-            raise InvalidParameter('Storage specified by template does not exist')
+            raise InvalidParameter('Storage specified by template does '
+                                   'not exist')
         if not pool.isActive():
-            raise InvalidParameter('Storage specified by template is not active')
+            raise InvalidParameter('Storage specified by template is '
+                                   'not active')
 
         return pool
 
@@ -1463,9 +1477,11 @@ class LibvirtVMTemplate(VMTemplate):
                 conn = self.conn.get()
                 network = conn.networkLookupByName(name)
             except libvirt.libvirtError:
-                raise InvalidParameter('Network specified by template does not exist')
+                raise InvalidParameter('Network specified by template does '
+                                       'not exist')
             if not network.isActive():
-                raise InvalidParameter('Network specified by template is not active')
+                raise InvalidParameter('Network specified by template is '
+                                       'not active')
 
     def _get_storage_path(self):
         pool = self._storage_validate()
@@ -1582,7 +1598,8 @@ class NetfsPoolDef(StoragePoolDef):
                 run_command(mount_cmd, 30)
                 rollback.prependDefer(run_command, umount_cmd)
             except TimeoutExpired:
-                raise InvalidParameter("Export path %s may block during nfs mount" % export_path)
+                raise InvalidParameter("Export path %s may block during "
+                                       "nfs mount" % export_path)
 
             with open("/proc/mounts" , "rb") as f:
                 rawMounts = f.read()
@@ -1593,8 +1610,8 @@ class NetfsPoolDef(StoragePoolDef):
                     mounted =  True
 
             if not mounted:
-                raise InvalidParameter(
-                    "Export path %s mount failed during nfs mount" % export_path)
+                raise InvalidParameter("Export path %s mount failed during "
+                                       "nfs mount" % export_path)
 
     @property
     def xml(self):
@@ -1830,7 +1847,8 @@ class LibvirtConnection(object):
                     except libvirt.libvirtError:
                         kimchi_log.error('Unable to connect to libvirt.')
                         if not retries:
-                            kimchi_log.error('Libvirt is not available, exiting.')
+                            kimchi_log.error('Libvirt is not available, '
+                                             'exiting.')
                             cherrypy.engine.stop()
                             raise
                     time.sleep(2)
@@ -1847,8 +1865,8 @@ class LibvirtConnection(object):
                             setattr(cls, name, wrapMethod(method))
 
                 self._connections[conn_id] = conn
-                # In case we're running into troubles with keeping the connections
-                # alive we should place here:
+                # In case we're running into troubles with keeping the
+                # connections alive we should place here:
                 # conn.setKeepAlive(interval=5, count=3)
                 # However the values need to be considered wisely to not affect
                 # hosts which are hosting a lot of virtual machines
diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py
index 7b567f3..5096ea8 100644
--- a/src/kimchi/objectstore.py
+++ b/src/kimchi/objectstore.py
@@ -107,7 +107,8 @@ class ObjectStore(object):
         try:
             return self._connections[ident]
         except KeyError:
-            self._connections[ident] = sqlite3.connect(self.location, timeout=10)
+            self._connections[ident] = sqlite3.connect(self.location,
+                                                       timeout=10)
             if len(self._connections.keys()) > 10:
                 id, conn = self._connections.popitem(last=False)
                 conn.interrupt()
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py
index f92db3d..14df9a6 100644
--- a/src/kimchi/osinfo.py
+++ b/src/kimchi/osinfo.py
@@ -56,18 +56,24 @@ modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3',
 
 isolinks = {
     'debian': {
-        'squeeze': 'http://cdimage.debian.org/debian-cd/6.0.7-live/amd64/iso-hybrid/debian-live-6.0.7-amd64-gnome-desktop.iso',
+        'squeeze': 'http://cdimage.debian.org/debian-cd/6.0.7-live/amd64/'
+                   'iso-hybrid/debian-live-6.0.7-amd64-gnome-desktop.iso',
     },
     'ubuntu': {
-        'raring': 'http://ubuntu-releases.cs.umn.edu/13.04/ubuntu-13.04-desktop-amd64.iso',
+        'raring': 'http://ubuntu-releases.cs.umn.edu/13.04/'
+                  'ubuntu-13.04-desktop-amd64.iso',
     },
     'opensuse': {
-        '12.3': 'http://suse.mirrors.tds.net/pub/opensuse/distribution/12.3/iso/openSUSE-12.3-DVD-x86_64.iso',
+        '12.3': 'http://suse.mirrors.tds.net/pub/opensuse/distribution/12.3/'
+                'iso/openSUSE-12.3-DVD-x86_64.iso',
     },
     'fedora': {
-        '16': 'http://fedora.mirrors.tds.net/pub/fedora/releases/16/Live/x86_64/Fedora-16-x86_64-Live-Desktop.iso',
-        '17': 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x86_64-Live-Desktop.iso',
-        '18': 'http://fedora.mirrors.tds.net/pub/fedora/releases/18/Live/x86_64/Fedora-18-x86_64-Live-Desktop.iso',
+        '16': 'http://fedora.mirrors.tds.net/pub/fedora/releases/16/Live/'
+              'x86_64/Fedora-16-x86_64-Live-Desktop.iso',
+        '17': 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/'
+              'x86_64/Fedora-17-x86_64-Live-Desktop.iso',
+        '18': 'http://fedora.mirrors.tds.net/pub/fedora/releases/18/Live/'
+              'x86_64/Fedora-18-x86_64-Live-Desktop.iso',
     },
 }
 
diff --git a/src/kimchi/scan.py b/src/kimchi/scan.py
index e192f01..ca460a0 100644
--- a/src/kimchi/scan.py
+++ b/src/kimchi/scan.py
@@ -59,8 +59,8 @@ class Scanner(object):
                     shutil.rmtree(d)
                     self.clean_cb(transient_pool)
         except OSError as e:
-            kimchi_log.debug(
-                    "Exception %s occured when cleaning stale pool, ignore" % e.message)
+            kimchi_log.debug("Exception %s occured when cleaning stale pool, "
+                             "ignore" % e.message)
 
     def scan_dir_prepare(self, name):
         # clean stale scan storage pools
@@ -77,7 +77,8 @@ class Scanner(object):
                 if (iso_info['distro'], iso_info['version']) == iso_img.probe():
                     return
 
-            iso_path = iso_name + hashlib.md5(iso_info['path']).hexdigest() + '.iso'
+            iso_path = (iso_name + hashlib.md5(iso_info['path']).hexdigest() +
+                       '.iso')
             link_name = os.path.join(params['pool_path'],
                                      os.path.basename(iso_path))
             os.symlink(iso_info['path'], link_name)
diff --git a/src/kimchi/template.py b/src/kimchi/template.py
index 1f19c4a..faa3c72 100644
--- a/src/kimchi/template.py
+++ b/src/kimchi/template.py
@@ -83,7 +83,8 @@ def can_accept_html():
 
 def render(resource, data):
     if can_accept('application/json'):
-        cherrypy.response.headers['Content-Type'] = 'application/json;charset=utf-8'
+        cherrypy.response.headers['Content-Type'] = ('application/json;'
+                                                    'charset=utf-8')
         return json.dumps(data, indent=2,
                           separators=(',', ':'),
                           encoding='iso-8859-1')
diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index 58147e3..6a3def3 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -111,9 +111,12 @@ class VMTemplate(object):
         qemu_stream_cmdline = """
             <qemu:commandline>
               <qemu:arg value='-drive'/>
-              <qemu:arg value='file=%(url)s,if=none,id=drive-%(bus)s0-1-0,readonly=on,format=raw'/>
+              <qemu:arg value='file=%(url)s,
+                               if=none,id=drive-%(bus)s0-1-0,readonly=on,
+                               format=raw'/>
               <qemu:arg value='-device'/>
-              <qemu:arg value='%(bus)s-cd,bus=%(bus)s.1,unit=0,drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
+              <qemu:arg value='%(bus)s-cd,bus=%(bus)s.1,unit=0,
+                               drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
             </qemu:commandline>
         """
 
diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py
index 0f88a35..28ac89b 100644
--- a/tests/test_osinfo.py
+++ b/tests/test_osinfo.py
@@ -34,7 +34,8 @@ class OSInfoTests(unittest.TestCase):
         self.assertEquals(['default'], entry['networks'])
 
     def test_fedora_lookup(self):
-        cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x86_64-Live-Desktop.iso'
+        cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/'
+             'Fedora-17-x86_64-Live-Desktop.iso'
         entry = lookup('fedora', '17')
         self.assertEquals(10, entry['disks'][0]['size'])
         self.assertEquals(cd, entry['cdrom'])