[Kimchi-devel] [PATCH] pep8: Use blacklist instead of whitelist

Paulo Ricardo Paz Vital pvital at linux.vnet.ibm.com
Tue Sep 30 18:22:43 UTC 2014


I think the POP8 fixes should be in a different patch than your black
list proposal (I mean, two patches) that is a good idea. 

But the patch is good.

-- 
Reviewed-by: Paulo Vital <pvital at linux.vnet.ibm.com>


On Tue, 2014-09-30 at 15:03 -0300, Crístian Viana wrote:
> Currently, the command "make check-local" runs the PEP8 verification
> rules only on files listed in a configuration files. New files must
> still be added manually and not all files are being checked.
> 
> List the files which *shouldn't* be checked against the PEP8 rules instead
> of listing the files which should be checked. Also, fix the errors on
> those new files.
> 
> The blacklisted files are: src/kimchi/{i18n,websocket,websockify}.py.
> 
> Signed-off-by: Crístian Viana <vianac at linux.vnet.ibm.com>
> ---
>  Makefile.am                 | 48 ++-------------------------------------------
>  src/kimchi/__init__.py      |  2 +-
>  src/kimchi/basemodel.py     |  4 +++-
>  src/kimchi/netinfo.py       |  2 +-
>  src/kimchi/screenshot.py    | 17 +++++++++-------
>  src/kimchi/sslcert.py       |  5 +++--
>  src/kimchi/vmdisks.py       |  6 +++---
>  src/kimchi/vnc.py           |  2 +-
>  src/kimchi/xmlutils.py      |  2 +-
>  tests/test_authorization.py | 20 +++++++++++--------
>  tests/test_exception.py     | 10 +++++-----
>  tests/test_server.py        |  5 +----
>  tests/test_vmtemplate.py    |  2 +-
>  13 files changed, 44 insertions(+), 81 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3293d9e..c04ba32 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -38,51 +38,7 @@ EXTRA_DIST = \
>  	$(NULL)
>  
> 
> -# When fixing a file to conform with pep8, add it to the WL here.
> -# So it will be checked from now on.
> -PEP8_WHITELIST = \
> -	plugins/__init__.py \
> -	plugins/sample/__init__.py \
> -	plugins/sample/model.py \
> -	src/kimchid.in \
> -	src/kimchi/asynctask.py \
> -	src/kimchi/auth.py \
> -	src/kimchi/cachebust.py \
> -	src/kimchi/config.py.in \
> -	src/kimchi/control/*.py \
> -	src/kimchi/control/vm/*.py \
> -	src/kimchi/disks.py \
> -	src/kimchi/distroloader.py \
> -	src/kimchi/exception.py \
> -	src/kimchi/featuretests.py \
> -	src/kimchi/imageinfo.py \
> -	src/kimchi/iscsi.py \
> -	src/kimchi/isoinfo.py \
> -	src/kimchi/kvmusertests.py \
> -	src/kimchi/mockmodel.py \
> -	src/kimchi/model/*.py \
> -	src/kimchi/objectstore.py \
> -	src/kimchi/osinfo.py \
> -	src/kimchi/proxy.py \
> -	src/kimchi/repositories.py \
> -	src/kimchi/rollbackcontext.py \
> -	src/kimchi/root.py \
> -	src/kimchi/scan.py \
> -	src/kimchi/server.py \
> -	src/kimchi/swupdate.py \
> -	src/kimchi/template.py \
> -	src/kimchi/utils.py \
> -	src/kimchi/vmtemplate.py \
> -	tests/test_config.py.in \
> -	tests/test_mockmodel.py \
> -	tests/test_model.py \
> -	tests/test_osinfo.py \
> -	tests/test_plugin.py \
> -	tests/test_rest.py \
> -	tests/test_rollbackcontext.py \
> -	tests/test_storagepool.py \
> -	tests/utils.py \
> -	$(NULL)
> +PEP8_BLACKLIST = "src/kimchi/i18n.py,src/kimchi/websocket.py,src/kimchi/websockify.py"
>  
>  SKIP_PYFLAKES_ERR = "\./src/kimchi/websocket\.py"
>  
> @@ -98,7 +54,7 @@ check-local:
>  		while read LINE; do echo "$$LINE"; false; done
>  
>  	$(PEP8) --version
> -	$(PEP8) --filename '*.py,*.py.in' $(PEP8_WHITELIST)
> +	$(PEP8) --filename '*.py,*.py.in' --exclude="$(PEP8_BLACKLIST)" .
>  
> 
>  # Link built mo files in the source tree to enable use of translations from
> diff --git a/src/kimchi/__init__.py b/src/kimchi/__init__.py
> index 1237e99..edf8e5d 100644
> --- a/src/kimchi/__init__.py
> +++ b/src/kimchi/__init__.py
> @@ -15,4 +15,4 @@
>  #
>  # 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
> diff --git a/src/kimchi/basemodel.py b/src/kimchi/basemodel.py
> index 93aab0c..096c66b 100644
> --- a/src/kimchi/basemodel.py
> +++ b/src/kimchi/basemodel.py
> @@ -46,7 +46,9 @@ class BaseModel(object):
>  
>  class Singleton(type):
>      _instances = {}
> +
>      def __call__(cls, *args, **kwargs):
>          if cls not in cls._instances:
> -            cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
> +            inst = super(Singleton, cls).__call__(*args, **kwargs)
> +            cls._instances[cls] = inst
>          return cls._instances[cls]
> diff --git a/src/kimchi/netinfo.py b/src/kimchi/netinfo.py
> index 2eaf4ac..bb50479 100644
> --- a/src/kimchi/netinfo.py
> +++ b/src/kimchi/netinfo.py
> @@ -185,7 +185,7 @@ def get_interface_type(iface):
>  
> 
>  def get_interface_info(iface):
> -    if not iface in ethtool.get_devices():
> +    if iface not in ethtool.get_devices():
>          raise ValueError('unknown interface: %s' % iface)
>  
>      ipaddr = ''
> diff --git a/src/kimchi/screenshot.py b/src/kimchi/screenshot.py
> index 8528a39..e599d40 100644
> --- a/src/kimchi/screenshot.py
> +++ b/src/kimchi/screenshot.py
> @@ -15,7 +15,7 @@
>  #
>  # 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 glob
> @@ -39,6 +39,7 @@ from kimchi.utils import kimchi_log
>  (fd, pipe) = tempfile.mkstemp()
>  stream_test_result = None
>  
> +
>  class VMScreenshot(object):
>      OUTDATED_SECS = 5
>      THUMBNAIL_SIZE = (256, 256)
> @@ -48,8 +49,9 @@ class VMScreenshot(object):
>      def __init__(self, args):
>          self.vm_uuid = args['uuid']
>          args.setdefault('thumbnail',
> -            os.path.join(config.get_screenshot_path(),
> -                '%s-%s.png' % (self.vm_uuid, str(uuid.uuid4()))))
> +                        os.path.join(config.get_screenshot_path(),
> +                                     '%s-%s.png' %
> +                                     (self.vm_uuid, str(uuid.uuid4()))))
>          self.info = args
>  
>      @staticmethod
> @@ -66,8 +68,8 @@ class VMScreenshot(object):
>          if now - last_update > self.OUTDATED_SECS:
>              self._clean_extra(self.LIVE_WINDOW)
>              self._generate_thumbnail()
> -        return '/data/screenshots/%s' % os.path.basename(self.info['thumbnail'])
> -
> +        return '/data/screenshots/%s' %\
> +               os.path.basename(self.info['thumbnail'])
>  
>      def _clean_extra(self, window=-1):
>          """
> @@ -77,7 +79,8 @@ class VMScreenshot(object):
>          try:
>              now = time.time()
>              clear_list = glob.glob("%s/%s-*.png" %
> -                (config.get_screenshot_path(), self.vm_uuid))
> +                                   (config.get_screenshot_path(),
> +                                    self.vm_uuid))
>              for f in clear_list:
>                  if now - os.path.getmtime(f) > window:
>                      os.unlink(f)
> @@ -122,7 +125,7 @@ class VMScreenshot(object):
>          else:
>              counter = 0
>              ret = os.waitpid(pid, os.WNOHANG)
> -            while ret == (0,0) and counter < 3:
> +            while ret == (0, 0) and counter < 3:
>                  counter += 1
>                  time.sleep(1)
>                  ret = os.waitpid(pid, os.WNOHANG)
> diff --git a/src/kimchi/sslcert.py b/src/kimchi/sslcert.py
> index bf4d261..e772dc8 100644
> --- a/src/kimchi/sslcert.py
> +++ b/src/kimchi/sslcert.py
> @@ -19,7 +19,7 @@
>  #
>  # 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 time
>  
> @@ -34,6 +34,7 @@ class SSLCert(object):
>      def _gen(self):
>          def keygen_cb(*args):
>              pass
> +
>          def passphrase_cb(*args):
>              return ''
>  
> @@ -54,7 +55,6 @@ class SSLCert(object):
>          subject.CN = 'kimchi'
>          subject.O = 'kimchi-project.org'
>  
> -
>          t = long(time.time()) + time.timezone
>          now = ASN1.ASN1_UTCTIME()
>          now.set_time(t)
> @@ -80,6 +80,7 @@ class SSLCert(object):
>      def key_pem(self):
>          return self._key
>  
> +
>  def main():
>      c = SSLCert()
>      print c.cert_text()
> diff --git a/src/kimchi/vmdisks.py b/src/kimchi/vmdisks.py
> index b61c883..f1c3f02 100644
> --- a/src/kimchi/vmdisks.py
> +++ b/src/kimchi/vmdisks.py
> @@ -39,9 +39,9 @@ def get_vm_disk(dom, dev_name):
>      # Retrieve disk xml and format return dict
>      disk = get_device_xml(dom, dev_name)
>      if disk is None:
> -        raise NotFoundError(
> -                "KCHVMSTOR0007E",
> -                {'dev_name': dev_name, 'vm_name': dom.name()})
> +        raise NotFoundError("KCHVMSTOR0007E",
> +                            {'dev_name': dev_name,
> +                             'vm_name': dom.name()})
>      path = ""
>      dev_bus = disk.target.attrib['bus']
>      try:
> diff --git a/src/kimchi/vnc.py b/src/kimchi/vnc.py
> index cfa7c6d..09a4cad 100644
> --- a/src/kimchi/vnc.py
> +++ b/src/kimchi/vnc.py
> @@ -16,7 +16,7 @@
>  #
>  # 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 base64
>  import errno
> diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py
> index 8612e66..d3db32a 100644
> --- a/src/kimchi/xmlutils.py
> +++ b/src/kimchi/xmlutils.py
> @@ -26,7 +26,7 @@ from xml.etree import ElementTree
>  def xpath_get_text(xml, expr):
>      doc = libxml2.parseDoc(xml)
>      res = doc.xpathEval(expr)
> -    ret = [None if x.children == None else x.children.content for x in res]
> +    ret = [None if x.children is None else x.children.content for x in res]
>  
>      doc.freeDoc()
>      return ret
> diff --git a/tests/test_authorization.py b/tests/test_authorization.py
> index 2c342a5..71b416f 100644
> --- a/tests/test_authorization.py
> +++ b/tests/test_authorization.py
> @@ -40,7 +40,7 @@ ssl_port = None
>  def setUpModule():
>      global test_server, model, host, port, ssl_port
>  
> -    patch_auth(sudo = False)
> +    patch_auth(sudo=False)
>      model = kimchi.mockmodel.MockModel('/tmp/obj-store-test')
>      host = '127.0.0.1'
>      port = get_free_port('http')
> @@ -111,23 +111,27 @@ class AuthorizationTests(unittest.TestCase):
>          resp = self.request('/templates/test', '{}', 'DELETE')
>          self.assertEquals(403, resp.status)
>  
> -
>          # Non-root users can only get vms authorized to them
>          model.templates_create({'name': u'test', 'cdrom': '/nonexistent.iso'})
>  
>          model.vms_create({'name': u'test-me', 'template': '/templates/test'})
> -        model.vm_update(u'test-me', {'users': [ kimchi.mockmodel.fake_user.keys()[0] ], 'groups': []})
> +        model.vm_update(u'test-me',
> +                        {'users': [kimchi.mockmodel.fake_user.keys()[0]],
> +                         'groups': []})
>  
> -        model.vms_create({'name': u'test-usera', 'template': '/templates/test'})
> -        model.vm_update(u'test-usera', {'users': [ 'userA' ], 'groups': []})
> +        model.vms_create({'name': u'test-usera',
> +                          'template': '/templates/test'})
> +        model.vm_update(u'test-usera', {'users': ['userA'], 'groups': []})
>  
> -        model.vms_create({'name': u'test-groupa', 'template': '/templates/test'})
> -        model.vm_update(u'test-groupa', {'groups': [ 'groupA' ]})
> +        model.vms_create({'name': u'test-groupa',
> +                          'template': '/templates/test'})
> +        model.vm_update(u'test-groupa', {'groups': ['groupA']})
>  
>          resp = self.request('/vms', '{}', 'GET')
>          self.assertEquals(200, resp.status)
>          vms_data = json.loads(resp.read())
> -        self.assertEquals([ u'test-groupa', u'test-me' ], sorted([ v['name'] for v in vms_data ]))
> +        self.assertEquals([u'test-groupa', u'test-me'],
> +                          sorted([v['name'] for v in vms_data]))
>          resp = self.request('/vms', req, 'POST')
>          self.assertEquals(403, resp.status)
>  
> diff --git a/tests/test_exception.py b/tests/test_exception.py
> index 90c1ea1..a533015 100644
> --- a/tests/test_exception.py
> +++ b/tests/test_exception.py
> @@ -15,7 +15,7 @@
>  #
>  # 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 json
>  import os
> @@ -77,8 +77,8 @@ class ExceptionTests(unittest.TestCase):
>          req = json.dumps({})
>          resp = json.loads(request(host, ssl_port, '/vms', req, 'POST').read())
>          self.assertEquals('400 Bad Request', resp.get('code'))
> -        msg = u"KCHVM0016E: Specify a template to create a virtual machine from"
> -        self.assertEquals(msg, resp.get('reason'))
> +        m = u"KCHVM0016E: Specify a template to create a virtual machine from"
> +        self.assertEquals(m, resp.get('reason'))
>          self.assertNotIn('call_stack', resp)
>  
>      def test_development_env(self):
> @@ -106,7 +106,7 @@ class ExceptionTests(unittest.TestCase):
>          # test 400 missing required parameter
>          req = json.dumps({})
>          resp = json.loads(request(host, ssl_port, '/vms', req, 'POST').read())
> -        msg = u"KCHVM0016E: Specify a template to create a virtual machine from"
> +        m = u"KCHVM0016E: Specify a template to create a virtual machine from"
>          self.assertEquals('400 Bad Request', resp.get('code'))
> -        self.assertEquals(msg, resp.get('reason'))
> +        self.assertEquals(m, resp.get('reason'))
>          self.assertIn('call_stack', resp)
> diff --git a/tests/test_server.py b/tests/test_server.py
> index 9beba6c..bebc383 100644
> --- a/tests/test_server.py
> +++ b/tests/test_server.py
> @@ -15,7 +15,7 @@
>  #
>  # 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 os
>  import unittest
> @@ -24,7 +24,6 @@ import unittest
>  import utils
>  import kimchi.mockmodel
>  
> -#utils.silence_server()
>  
>  class ServerTests(unittest.TestCase):
>      def test_server_start(self):
> @@ -44,5 +43,3 @@ class ServerTests(unittest.TestCase):
>          finally:
>              os.unlink('/tmp/obj-store-test')
>              s.stop()
> -
> -
> diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py
> index 4ae1d36..2a6fb8e 100644
> --- a/tests/test_vmtemplate.py
> +++ b/tests/test_vmtemplate.py
> @@ -15,7 +15,7 @@
>  #
>  # 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 os
>  import unittest




More information about the Kimchi-devel mailing list