Re: [Kimchi-devel] [PATCH] pep8 cleanup for model.py

Hi Sheldon, thanks for sending this patch, but I think Aline is refactoring model.py, splitting such as what was done for 'control'.

Rodrigo Trujillo On 01/13/2014 09:12 AM, shaohef(a)linux.vnet.ibm.com wrote: > From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; > > This patch cleans up pep8 style issue in model.py > > Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; > --- > src/kimchi/model.py | 127 > +++++++++++++++++++++++++++++----------------------- > 1 file changed, 72 insertions(+), 55 deletions(-) > > diff --git a/src/kimchi/model.py b/src/kimchi/model.py > index 4b7df7e..9d0dad3 100644 > --- a/src/kimchi/model.py > +++ b/src/kimchi/model.py > @@ -18,16 +18,14 @@ > # > # You should have received a copy of the GNU Lesser General Public > # License along with this library; if not, write to the Free Software > -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > > import cherrypy > import copy > import disks > import fnmatch > -import functools > import glob > import ipaddr > -import json > import libvirt > import logging > import os > @@ -43,17 +41,10 @@ import uuid > > > from cherrypy.process.plugins import BackgroundTask > -from cherrypy.process.plugins import SimplePlugin > from collections import defaultdict > from xml.etree import ElementTree > > > -try: > - from collections import OrderedDict > -except ImportError: > - from ordereddict import OrderedDict > - > - > from kimchi import config > from kimchi import netinfo > from kimchi import network as knetwork > @@ -70,7 +61,7 @@ from kimchi.isoinfo import IsoImage > from kimchi.objectstore import ObjectStore > from kimchi.scan import Scanner > from kimchi.screenshot import VMScreenshot > -from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log > +from kimchi.utils import get_enabled_plugins, kimchi_log > from kimchi.vmtemplate import VMTemplate > > > @@ -90,12 +81,15 @@ def _uri_to_name(collection, uri): > raise InvalidParameter(uri) > return m.group(1) > > + > def template_name_from_uri(uri): > return _uri_to_name('templates', uri) > > + > def pool_name_from_uri(uri): > return _uri_to_name('storagepools', uri) > > + > def get_vm_name(vm_name, t_name, name_list): > if vm_name: > return vm_name > @@ -105,6 +99,7 @@ def get_vm_name(vm_name, t_name, name_list): > return vm_name > raise OperationFailed("Unable to choose a VM name") > > + > class Model(object): > dom_state_map = {0: 'nostate', > 1: 'running', > @@ -152,9 +147,10 @@ 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}, > - {'cmd': 'supportconfig -h', 'fn':None}, > - {'cmd': 'linuxexplorers --help', 'fn':None}) > + self.report_tools = ({'cmd': 'sosreport --help', > + 'fn': self._sosreport_generate}, > + {'cmd': 'supportconfig -h', 'fn': None}, > + {'cmd': 'linuxexplorers --help', 'fn': None}) > > self.distros = self._get_distros() > if 'qemu:///' in self.libvirt_uri: > @@ -183,7 +179,8 @@ class Model(object): > net = conn.networkDefineXML(xml) > except libvirt.libvirtError, e: > cherrypy.log.error( > - "Fatal: Cannot create default network because of %s, exit kimchid" > % e.message, > + "Fatal: Cannot create default network because of %s, " > + "exit kimchid" % e.message, > severity=logging.ERROR) > sys.exit(1) > > @@ -192,7 +189,8 @@ class Model(object): > net.create() > except libvirt.libvirtError, e: > cherrypy.log.error( > - "Fatal: Cannot activate default network because of %s, exit > kimchid" % e.message, > + "Fatal: Cannot activate default network because of %s, " > + "exit kimchid" % e.message, > severity=logging.ERROR) > sys.exit(1) > > @@ -208,7 +206,8 @@ class Model(object): > 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, > + "Fatal: Cannot create default pool because of %s, " > + "exit kimchid" % e.message, > severity=logging.ERROR) > sys.exit(1) > > @@ -280,7 +279,7 @@ class Model(object): > distro, version, codename = platform.linux_distribution() > res['os_distro'] = distro > res['os_version'] = version > - res['os_codename'] = unicode(codename,"utf-8") > + res['os_codename'] = unicode(codename, "utf-8") > > return res > > @@ -320,7 +319,7 @@ class Model(object): > max_net_io = round(max(currentMaxNetRate, int(rate)), 1) > > self.stats[vm_uuid].update({'net_io': rate, 'max_net_io': max_net_io, > - 'netRxKB': netRxKB, 'netTxKB': netTxKB}) > + 'netRxKB': netRxKB, 'netTxKB': netTxKB}) > > def _get_disk_io_rate(self, vm_uuid, dom, seconds): > prevDiskRdKB = self.stats[vm_uuid].get('diskRdKB', 0) > @@ -346,8 +345,10 @@ class Model(object): > rate = rd_stats + wr_stats > max_disk_io = round(max(currentMaxDiskRate, int(rate)), 1) > > - self.stats[vm_uuid].update({'disk_io': rate, 'max_disk_io': > max_disk_io, > - 'diskRdKB': diskRdKB, 'diskWrKB': diskWrKB}) > + self.stats[vm_uuid].update({'disk_io': rate, > + 'max_disk_io': max_disk_io, > + 'diskRdKB': diskRdKB, > + 'diskWrKB': diskWrKB}) > > def debugreport_lookup(self, name): > path = config.get_debugreports_path() > @@ -392,7 +393,7 @@ class Model(object): > > return name_lists > > - def _update_host_stats(self): > + def _update_host_stats(self): > preTimeStamp = self.host_stats['timestamp'] > timestamp = time.time() > # FIXME when we upgrade psutil, we can get uptime by psutil.uptime > @@ -474,12 +475,14 @@ 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() > @@ -598,7 +601,7 @@ class Model(object): > > def vm_connect(self, name): > graphics, port = self._vm_get_graphics(name) > - if graphics == "vnc" and port != None: > + if graphics == "vnc" and port is not None: > vnc.add_proxy_token(name, port) > else: > raise OperationFailed("Only able to connect to running vm's vnc " > @@ -621,7 +624,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) > @@ -632,11 +636,12 @@ 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 = False if len(self.libvirt_stream_protocols) == 0 \ > + else True > > xml = t.to_vm_xml(name, vm_uuid, libvirt_stream, self.qemu_stream_dns) > try: > - dom = conn.defineXML(xml.encode('utf-8')) > + conn.defineXML(xml.encode('utf-8')) > except libvirt.libvirtError as e: > for v in vol_list: > vol = conn.storageVolLookupByPath(v['path']) > @@ -704,7 +709,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: > @@ -925,7 +931,7 @@ class Model(object): > return '-'.join(('kimchi', interface, vlan_id)) > > def _is_vlan_tagged_bridge(self, bridge): > - return bridge.startswith('kimchi-') > + return bridge.startswith('kimchi-') > > def _create_vlan_tagged_bridge(self, interface, vlan_id): > br_name = self._get_vlan_tagged_bridge_name(interface, vlan_id) > @@ -960,7 +966,7 @@ class Model(object): > 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 > > @@ -981,7 +987,6 @@ class Model(object): > except: > raise > > - > def _get_vm(self, name): > conn = self.conn.get() > try: > @@ -1027,7 +1032,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=[]) > @@ -1040,10 +1046,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']) > + params['path'] = scan_params['pool_path'] = \ > + self.scanner.scan_dir_prepare(params['name']) > task_id = self.add_task('', self.scanner.start_scan, scan_params) > # Record scanning-task/storagepool mapping for future querying > with self.objstore as session: > @@ -1068,7 +1075,7 @@ class Model(object): > > if name in self.storagepools_get_list(): > raise InvalidOperation( > - "The name %s has been used by a pool" % name) > + "The name %s has been used by a pool" % name) > > try: > if task_id: > @@ -1178,7 +1185,7 @@ class Model(object): > pool = self._get_storagepool(name) > if pool.isActive(): > raise InvalidOperation( > - "Unable to delete the active storagepool %s" % name) > + "Unable to delete the active storagepool %s" % name) > try: > pool.undefine() > except libvirt.libvirtError as e: > @@ -1204,7 +1211,7 @@ class Model(object): > raise > > def storagevolumes_create(self, pool, params): > - info = self.storagepool_lookup(pool) > + self.storagepool_lookup(pool) > try: > name = params['name'] > xml = _get_volume_xml(**params) > @@ -1239,7 +1246,8 @@ class Model(object): > except IsoFormatError: > bootable = False > res.update( > - dict(os_distro=os_distro, os_version=os_version, path=path, > bootable=bootable)) > + dict(os_distro=os_distro, os_version=os_version, > + path=path, bootable=bootable)) > > return res > > @@ -1269,7 +1277,8 @@ class Model(object): > pool = self._get_storagepool(pool) > if not pool.isActive(): > raise InvalidOperation( > - "Unable to list volumes in inactive storagepool %s" % pool.name()) > + "Unable to list volumes in inactive storagepool " > + "%s" % pool.name()) > try: > self._pool_refresh(pool) > return pool.listVolumes() > @@ -1280,7 +1289,8 @@ class Model(object): > pool = self._get_storagepool(pool) > if not pool.isActive(): > raise InvalidOperation( > - "Unable to list volumes in inactive storagepool %s" % pool.name()) > + "Unable to list volumes in inactive storagepool " > + "%s" % pool.name()) > try: > return pool.storageVolLookupByName(name) > except libvirt.libvirtError as e: > @@ -1301,7 +1311,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: > @@ -1346,7 +1357,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: > @@ -1364,7 +1376,7 @@ class Model(object): > > def _get_distros(self): > distroloader = DistroLoader() > - return distroloader.get() > + return distroloader.get() > > def distros_get_list(self): > return self.distros.keys() > @@ -1436,9 +1448,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 > > @@ -1449,9 +1463,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() > @@ -1485,7 +1501,7 @@ class LibvirtVMScreenshot(VMScreenshot): > dom = conn.lookupByUUIDString(self.vm_uuid) > vm_name = dom.name() > stream = conn.newStream(0) > - mimetype = dom.screenshot(stream, 0, 0) > + dom.screenshot(stream, 0, 0) > stream.recvAll(handler, fd) > except libvirt.libvirtError: > try: > @@ -1734,8 +1750,8 @@ class LibvirtConnection(object): > def get_wrappable_objects(self): > """ > When a wrapped function returns an instance of another libvirt object, > - we also want to wrap that object so we can catch errors that happen > when > - calling its methods. > + we also want to wrap that object so we can catch errors that happen > + when calling its methods. > """ > objs = [] > for name in ('virDomain', 'virDomainSnapshot', 'virInterface', > @@ -1791,7 +1807,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) > @@ -1808,8 +1825,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