Some pylint fixes

Change-Id: Ic5c1b533c3479b4fdb8cbfe56d051022d1efa779

20 files changed, 69 insertions, 63 deletions

diff --git a/doc/v2/development.rst b/doc/v2/development.rst
index d44ff637c..a3a8539c0 100644
--- a/doc/v2/development.rst
+++ b/doc/v2/development.rst
@@ -883,6 +883,24 @@ the pylint output, some warnings are recommended to be disabled::
 .. note:: Docstrings should still be added wherever a docstring would
    be useful.
 
+Many developers use a ``~/.pylintrc`` file which already includes a
+sample list of warnings to disable. Other warnings frequently disabled
+in ``~/.pylintrc`` include:
+
+.. code-block:: none
+
+        too-many-locals,
+        too-many-ancestors,
+        too-many-arguments,
+        too-many-instance-attributes,
+        too-many-nested-blocks,
+        too-many-return-statements,
+        too-many-branches,
+        too-many-statements,
+        too-few-public-methods,
+        wrong-import-order,
+        ungrouped-imports,
+
 ``pylint`` also supports local disabling of warnings and there are many
 examples of:
 
@@ -892,7 +910,8 @@ examples of:
 
 There is a ``pylint-django`` plugin available in unstable and testing
 and whilst it improves the pylint output for the ``lava-server``
-codebase, it still has a high level of false indications.
+codebase, it still has a high level of false indications, particularly
+when extending an existing model.
 
 pep8
 ****
diff --git a/lava/utils/managers.py b/lava/utils/managers.py
index 4930afc0f..294152284 100644
--- a/lava/utils/managers.py
+++ b/lava/utils/managers.py
@@ -51,8 +51,8 @@ class MaterializedView(models.Model):
         abstract = True
         managed = False
 
-    def delete(self, *args, **kwargs):
+    def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
         raise NotImplementedError
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
         raise NotImplementedError
diff --git a/lava_common/timeout.py b/lava_common/timeout.py
index 8e75712d7..0417b591e 100644
--- a/lava_common/timeout.py
+++ b/lava_common/timeout.py
@@ -99,11 +99,11 @@ class Timeout(object):
             if parent is None:
                 signal.alarm(0)
             else:
-                signal.signal(signal.SIGALRM, parent.timeout._timed_out)
+                signal.signal(signal.SIGALRM, parent.timeout._timed_out)  # pylint: disable=protected-access
                 duration = round(action_max_end_time - time.time())
                 if duration <= 0:
                     signal.alarm(0)
-                    parent.timeout._timed_out(None, None)
+                    parent.timeout._timed_out(None, None)  # pylint: disable=protected-access
                 signal.alarm(duration)
         except Exception:
             # clear the timeout alarm, the action has returned an error
diff --git a/lava_dispatcher/actions/boot/docker.py b/lava_dispatcher/actions/boot/docker.py
index 2e5634f2a..1fda71ea1 100644
--- a/lava_dispatcher/actions/boot/docker.py
+++ b/lava_dispatcher/actions/boot/docker.py
@@ -89,6 +89,7 @@ class CallDockerAction(Action):
         super().__init__()
         self.cleanup_required = False
         self.extra_options = ''
+        self.container = ''
 
     def validate(self):
         super().validate()
diff --git a/lava_dispatcher/actions/boot/u_boot.py b/lava_dispatcher/actions/boot/u_boot.py
index ad6c02c0b..0c9487ec9 100644
--- a/lava_dispatcher/actions/boot/u_boot.py
+++ b/lava_dispatcher/actions/boot/u_boot.py
@@ -70,8 +70,7 @@ class UBoot(Boot):
             raise ConfigurationError("commands not specified in boot parameters")
         if 'u-boot' in device['actions']['boot']['methods']:
             return True, 'accepted'
-        else:
-            return False, '"u-boot" was not in the device configuration boot methods'
+        return False, '"u-boot" was not in the device configuration boot methods'
 
 
 class UBootAction(BootAction):
@@ -105,6 +104,11 @@ class UBootRetry(BootAction):
     description = "interactive uboot retry action"
     summary = "uboot commands with retry"
 
+    def __init__(self):
+        super().__init__()
+        self.method_params = None
+        self.usb_mass_device = None
+
     def populate(self, parameters):
         self.internal_pipeline = Pipeline(parent=self, job=self.job, parameters=parameters)
         self.method_params = self.job.device['actions']['boot']['methods']['u-boot']['parameters']
diff --git a/lava_dispatcher/actions/deploy/download.py b/lava_dispatcher/actions/deploy/download.py
index bc9814e2e..08906ef00 100644
--- a/lava_dispatcher/actions/deploy/download.py
+++ b/lava_dispatcher/actions/deploy/download.py
@@ -67,10 +67,6 @@ import urllib.parse as lavaurl
 
 # pylint: disable=logging-not-lazy
 
-# FIXME: separate download actions for decompressed and uncompressed downloads
-# so that the logic can be held in the Strategy class, not the Action.
-# FIXME: create a download3.py which uses urllib.urlparse
-
 
 class DownloaderAction(RetryAction):
     """
@@ -104,11 +100,11 @@ class DownloaderAction(RetryAction):
         if url.scheme == 'scp':
             action = ScpDownloadAction(self.key, self.path, url, self.uniquify)
         elif url.scheme == 'http' or url.scheme == 'https':
-            action = HttpDownloadAction(self.key, self.path, url, self.uniquify)  # pylint: disable=redefined-variable-type
+            action = HttpDownloadAction(self.key, self.path, url, self.uniquify)
         elif url.scheme == 'file':
-            action = FileDownloadAction(self.key, self.path, url, self.uniquify)  # pylint: disable=redefined-variable-type
+            action = FileDownloadAction(self.key, self.path, url, self.uniquify)
         elif url.scheme == 'lxc':
-            action = LxcDownloadAction(self.key, self.path, url)  # pylint: disable=redefined-variable-type
+            action = LxcDownloadAction(self.key, self.path, url)
         else:
             raise JobError("Unsupported url protocol scheme: %s" % url.scheme)
         self.internal_pipeline.add_action(action)
@@ -139,7 +135,7 @@ class DownloadHandler(Action):  # pylint: disable=too-many-instance-attributes
         self.size = -1
         self.decompress_command_map = {'xz': 'unxz', 'gz': 'gunzip', 'bz2': 'bunzip2'}
 
-    def reader(self):
+    def reader(self):  # pylint: disable=no-self-use
         raise LAVABug("'reader' function unimplemented")
 
     def cleanup(self, connection):
@@ -156,11 +152,8 @@ class DownloadHandler(Action):  # pylint: disable=too-many-instance-attributes
         # Also files without suffixes, e.g. kernel images
         # Don't rename files we don't support decompressing during download
         if not modify or len(parts) == 1 or (modify not in self.decompress_command_map):
-            return (os.path.join(path, filename),
-                    None)
-        else:
-            return (os.path.join(path, '.'.join(parts[:-1])),
-                    parts[-1])
+            return (os.path.join(path, filename), None)
+        return (os.path.join(path, '.'.join(parts[:-1])), parts[-1])
 
     def validate(self):
         super().validate()
diff --git a/lava_dispatcher/actions/deploy/prepare.py b/lava_dispatcher/actions/deploy/prepare.py
index 5a6175350..78f67ddb1 100644
--- a/lava_dispatcher/actions/deploy/prepare.py
+++ b/lava_dispatcher/actions/deploy/prepare.py
@@ -193,7 +193,7 @@ class PrepareFITAction(Action):
         else:
             self.device_params = device_params
 
-    def _make_mkimage_command(self, params):
+    def _make_mkimage_command(self, params):  # pylint: disable=no-self-use
         cmd = [
             'mkimage',
             '-D', '"-I dts -O dtb -p 2048"',
diff --git a/lava_dispatcher/actions/test/monitor.py b/lava_dispatcher/actions/test/monitor.py
index 6760febfd..865984398 100644
--- a/lava_dispatcher/actions/test/monitor.py
+++ b/lava_dispatcher/actions/test/monitor.py
@@ -23,10 +23,7 @@ import pexpect
 
 from collections import OrderedDict
 from lava_dispatcher.action import Pipeline
-from lava_common.exceptions import (
-    InfrastructureError,
-    LAVABug,
-)
+from lava_common.exceptions import InfrastructureError
 from lava_dispatcher.actions.test import (
     TestAction,
 )
@@ -57,8 +54,7 @@ class TestMonitor(LavaTest):
                 if all([x for x in required_parms if x in monitor]):
                     return True, 'accepted'
             return False, 'missing a required parameter from %s' % required_parms
-        else:
-            return False, '"monitors" not in parameters'
+        return False, '"monitors" not in parameters'
 
     @classmethod
     def needs_deployment_data(cls):
diff --git a/lava_dispatcher/connection.py b/lava_dispatcher/connection.py
index b17c9d76c..513d094df 100644
--- a/lava_dispatcher/connection.py
+++ b/lava_dispatcher/connection.py
@@ -88,6 +88,9 @@ class Connection(object):
     Connecting between devices is handled inside the YAML test definition, whether by
     multinode or by configured services inside the test image.
     """
+
+    name = "Connection"
+
     def __init__(self, job, raw_connection):
         self.device = job.device
         self.job = job
diff --git a/lava_dispatcher/deployment_data.py b/lava_dispatcher/deployment_data.py
index 4595b0e72..a982d634e 100644
--- a/lava_dispatcher/deployment_data.py
+++ b/lava_dispatcher/deployment_data.py
@@ -55,11 +55,9 @@ class deployment_data_dict(object):  # pylint: disable=invalid-name, too-few-pub
     def get(self, *args):
         if len(args) == 1:
             return self.__data__.get(args[0])
-        else:
-            if args[0] in self.__data__.keys():
-                return self.__data__.get(args[0])
-            else:
-                return args[1]
+        if args[0] in self.__data__.keys():
+            return self.__data__.get(args[0])
+        return args[1]
 
     def keys(self):
         """
diff --git a/lava_dispatcher/parser.py b/lava_dispatcher/parser.py
index 207847e06..bba06b41b 100644
--- a/lava_dispatcher/parser.py
+++ b/lava_dispatcher/parser.py
@@ -109,7 +109,7 @@ class JobParser(object):
         mapping['yaml_line'] = node.__line__
         return mapping
 
-    def _timeouts(self, data, job):
+    def _timeouts(self, data, job):  # pylint: disable=no-self-use
         if 'job' in data.get('timeouts', {}):
             duration = Timeout.parse(data['timeouts']['job'])
             job.timeout = Timeout('job', duration)
diff --git a/lava_dispatcher/shell.py b/lava_dispatcher/shell.py
index 850e7cf65..6a3039914 100644
--- a/lava_dispatcher/shell.py
+++ b/lava_dispatcher/shell.py
@@ -157,7 +157,7 @@ class ShellCommand(pexpect.spawn):  # pylint: disable=too-many-public-methods
             sent = super().send(string)
         return sent
 
-    def expect(self, *args, **kw):
+    def expect(self, *args, **kw):  # pylint: disable=arguments-differ
         """
         No point doing explicit logging here, the SignalDirector can help
         the TestShellAction make much more useful reports of what was matched
diff --git a/lava_dispatcher/test/test_defs.py b/lava_dispatcher/test/test_defs.py
index fde5afbb2..f6d16773e 100644
--- a/lava_dispatcher/test/test_defs.py
+++ b/lava_dispatcher/test/test_defs.py
@@ -20,7 +20,6 @@
 
 import re
 import os
-import sys
 import glob
 import stat
 import yaml
@@ -31,7 +30,6 @@ import unittest
 import subprocess
 from nose.tools import nottest
 from lava_dispatcher.power import FinalizeAction
-from lava_dispatcher.device import NewDevice
 from lava_dispatcher.parser import JobParser
 from lava_common.exceptions import InfrastructureError
 from lava_dispatcher.actions.test.shell import TestShellRetry, PatternFixup
diff --git a/lava_dispatcher/test/test_depthcharge.py b/lava_dispatcher/test/test_depthcharge.py
index 3214f43e5..9cfb7bef4 100644
--- a/lava_dispatcher/test/test_depthcharge.py
+++ b/lava_dispatcher/test/test_depthcharge.py
@@ -22,16 +22,9 @@
 
 import os
 import unittest
-from lava_dispatcher.actions.deploy.tftp import TftpAction
-from lava_dispatcher.actions.boot.depthcharge import (
-    DepthchargeAction,
-    DepthchargeStart,
-    DepthchargeCommandOverlay,
-    DepthchargeRetry,
-)
 from lava_dispatcher.device import NewDevice
 from lava_dispatcher.parser import JobParser
-from lava_dispatcher.test.test_basic import Factory, StdoutTestCase
+from lava_dispatcher.test.test_basic import StdoutTestCase
 from lava_dispatcher.test.utils import DummyLogger, infrastructure_error
 
 
@@ -41,7 +34,7 @@ class DepthchargeFactory(object):
     Factory objects are dispatcher based classes, independent
     of any database objects.
     """
-    def create_jaq_job(self, filename):
+    def create_jaq_job(self, filename):  # pylint: disable=no-self-use
         device = NewDevice(os.path.join(
             os.path.dirname(__file__), '../devices/jaq-01.yaml'))
         yaml = os.path.join(os.path.dirname(__file__), filename)
@@ -107,7 +100,7 @@ class TestDepthchargeAction(StdoutTestCase):
 -b {dtb} \
 -i {ramdisk} \
 {fit_path}'.format(**params)
-        cmd = prep_fit._make_mkimage_command(params)
+        cmd = prep_fit._make_mkimage_command(params)  # pylint: disable=protected-access
         self.assertEqual(cmd_ref, ' '.join(cmd))
 
         depthcharge = [action for action in job.pipeline.actions
diff --git a/lava_dispatcher/test/test_utils.py b/lava_dispatcher/test/test_utils.py
index 71266db2b..d97fc8d21 100644
--- a/lava_dispatcher/test/test_utils.py
+++ b/lava_dispatcher/test/test_utils.py
@@ -173,12 +173,12 @@ class TestBzr(StdoutTestCase):  # pylint: disable=too-many-public-methods
 
     def test_clone_at_2(self):
         bzr = vcs.BzrHelper('repo')
-        self.assertEqual(bzr.clone('bzr.clone1', '2'), '2')
+        self.assertEqual(bzr.clone('bzr.clone1', revision='2'), '2')
 
     def test_clone_at_1(self):
         bzr = vcs.BzrHelper('repo')
-        self.assertEqual(bzr.clone('bzr.clone1', '1'), '1')
-        self.assertEqual(bzr.clone('bzr.clone2', '1'), '1')
+        self.assertEqual(bzr.clone('bzr.clone1', revision='1'), '1')
+        self.assertEqual(bzr.clone('bzr.clone2', revision='1'), '1')
 
     def test_non_existing_bzr(self):
         bzr = vcs.BzrHelper('does_not_exists')
@@ -192,8 +192,8 @@ class TestBzr(StdoutTestCase):  # pylint: disable=too-many-public-methods
 
     def test_invalid_commit(self):
         bzr = vcs.BzrHelper('repo')
-        self.assertRaises(InfrastructureError, bzr.clone, 'foo.bar', '3')
-        self.assertRaises(InfrastructureError, bzr.clone, 'foo.bar', 'badrev')
+        self.assertRaises(InfrastructureError, bzr.clone, 'foo.bar', revision='3')
+        self.assertRaises(InfrastructureError, bzr.clone, 'foo.bar', revision='badrev')
 
 
 class TestConstants(StdoutTestCase):  # pylint: disable=too-many-public-methods
diff --git a/lava_dispatcher/utils/compression.py b/lava_dispatcher/utils/compression.py
index 276fff97b..dfa6b7f3c 100644
--- a/lava_dispatcher/utils/compression.py
+++ b/lava_dispatcher/utils/compression.py
@@ -38,10 +38,10 @@ from lava_dispatcher.utils.shell import which
 
 
 # https://www.kernel.org/doc/Documentation/xz.txt
-compress_command_map = {'xz': ['xz', '--check=crc32'],
+compress_command_map = {'xz': ['xz', '--check=crc32'],  # pylint: disable=invalid-name
                         'gz': ['gzip'],
                         'bz2': ['bzip2']}
-decompress_command_map = {'xz': ['unxz'],
+decompress_command_map = {'xz': ['unxz'],  # pylint: disable=invalid-name
                           'gz': ['gunzip'],
                           'bz2': ['bunzip2'],
                           'zip': ['unzip']}
diff --git a/lava_dispatcher/utils/filesystem.py b/lava_dispatcher/utils/filesystem.py
index 5de9639b3..f9d8dd915 100644
--- a/lava_dispatcher/utils/filesystem.py
+++ b/lava_dispatcher/utils/filesystem.py
@@ -375,8 +375,7 @@ def copy_overlay_to_sparse_fs(image, overlay):
     # Check if we have space left on the mounted image.
     output = guest.df()
     logger.debug(output)
-    device, size, used, available, percent, mountpoint = output.split(
-        "\n")[1].split()
+    _, _, _, available, percent, _ = output.split("\n")[1].split()
     guest.umount(devices[0])
     if int(available) is 0 or percent == '100%':
         raise JobError("No space in image after applying overlay: %s" % image)
@@ -418,9 +417,6 @@ def is_sparse_image(image):
     """
     Returns True if the image is an 'Android sparse image' else False.
     """
-    image_magic = magic.open(magic.MAGIC_NONE)
+    image_magic = magic.open(magic.MAGIC_NONE)  # pylint: disable=no-member
     image_magic.load()
-    if image_magic.file(image).split(',')[0] == 'Android sparse image':
-        return True
-    else:
-        return False
+    return bool(image_magic.file(image).split(',')[0] == 'Android sparse image')
diff --git a/lava_dispatcher/utils/storage.py b/lava_dispatcher/utils/storage.py
index 14c92a6fa..9868d28f3 100644
--- a/lava_dispatcher/utils/storage.py
+++ b/lava_dispatcher/utils/storage.py
@@ -37,6 +37,7 @@ class FlashUBootUMSAction(Action):
         super().__init__()
         self.params = None
         self.usb_mass_device = usb_mass_device
+        self.ums_device = None
 
     def validate(self):
         super().validate()
diff --git a/lava_dispatcher/utils/udev.py b/lava_dispatcher/utils/udev.py
index 53cfc0aa1..fe09ce5ec 100644
--- a/lava_dispatcher/utils/udev.py
+++ b/lava_dispatcher/utils/udev.py
@@ -154,6 +154,7 @@ class WaitDeviceBoardID(Action):
 
     def __init__(self, board_id=None):
         super().__init__()
+        self.udev_device = None
         if not board_id:
             self.board_id = self.job.device.get('board_id', None)
         else:
diff --git a/lava_dispatcher/utils/vcs.py b/lava_dispatcher/utils/vcs.py
index 78bb502b5..62f0f7042 100644
--- a/lava_dispatcher/utils/vcs.py
+++ b/lava_dispatcher/utils/vcs.py
@@ -34,7 +34,7 @@ class VCSHelper(object):
     def __init__(self, url):
         self.url = url
 
-    def clone(self, dest_path, revision=None, branch=None):
+    def clone(self, dest_path, shallow=None, revision=None, branch=None, history=None):
         raise NotImplementedError
 
 
@@ -44,7 +44,7 @@ class BzrHelper(VCSHelper):
         super().__init__(url)
         self.binary = '/usr/bin/bzr'
 
-    def clone(self, dest_path, revision=None, branch=None):
+    def clone(self, dest_path, shallow=None, revision=None, branch=None, history=None):
         cwd = os.getcwd()
         logger = logging.getLogger('dispatcher')
         env = dict(os.environ)
@@ -69,7 +69,7 @@ class BzrHelper(VCSHelper):
 
         except subprocess.CalledProcessError as exc:
             exc_command = [i.strip() for i in exc.cmd]
-            exc_message = str(exc)  # pylint: disable=redefined-variable-type
+            exc_message = str(exc)
             exc_output = str(exc).split('\n')
             logger.exception(yaml.dump({
                 'command': exc_command,
@@ -151,3 +151,6 @@ class URLHelper(VCSHelper):
     def __init__(self, url):
         super().__init__(url)
         self.binary = None
+
+    def clone(self, dest_path, shallow=None, revision=None, branch=None, history=None):
+        raise NotImplementedError