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-...', + '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...', + '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-x...', - '17': 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...', - '18': 'http://fedora.mirrors.tds.net/pub/fedora/releases/18/Live/x86_64/Fedora-18-x...', + '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-x...' + 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'])