Volans has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/361040 )
Change subject: Fix Pylint and other tools reported errors
......................................................................
Fix Pylint and other tools reported errors
- In preparation for the introduction of additional testing tools in tox
such as Pylint
Bug: T154588
Change-Id: Icf2965ec31beb9325c07df8aa3e2d1d71265bc2c
---
M cumin/backends/__init__.py
M cumin/backends/direct.py
M cumin/backends/puppetdb.py
M cumin/cli.py
M cumin/grammar.py
M cumin/tests/__init__.py
M cumin/tests/integration/test_cli.py
M cumin/tests/unit/backends/test_puppetdb.py
M cumin/tests/unit/test_backends.py
M cumin/tests/unit/test_cli.py
M cumin/tests/unit/test_grammar.py
M cumin/tests/unit/test_query.py
M cumin/tests/unit/transports/test_clustershell.py
M cumin/tests/unit/transports/test_init.py
A cumin/tests/vulture_whitelist.py
M cumin/transports/__init__.py
M cumin/transports/clustershell.py
17 files changed, 119 insertions(+), 80 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/cumin
refs/changes/40/361040/1
diff --git a/cumin/backends/__init__.py b/cumin/backends/__init__.py
index 937f752..120afb9 100644
--- a/cumin/backends/__init__.py
+++ b/cumin/backends/__init__.py
@@ -31,10 +31,10 @@
"""Add a category token to the query 'F:key = value'.
Arguments:
- category -- the category of the token, one of cumin.grammar.categories
+ category -- the category of the token, one of cumin.grammar.CATEGORIES
key -- the key for this category
value -- the value to match, if not specified the key itself will
be matched [optional, default: None]
- operator -- the comparison operator to use, one of
cumin.grammar.operators [optional: default: =]
+ operator -- the comparison operator to use, one of
cumin.grammar.OPERATORS [optional: default: =]
neg -- whether the token must be negated [optional, default:
False]
"""
diff --git a/cumin/backends/direct.py b/cumin/backends/direct.py
index c315d24..23456c7 100644
--- a/cumin/backends/direct.py
+++ b/cumin/backends/direct.py
@@ -60,4 +60,5 @@
return list(self.hosts)
-query_class = DirectQuery # Required by the auto-loader in the
cumin.query.Query factory
+# Required by the auto-loader in the cumin.query.Query factory
+query_class = DirectQuery # pylint: disable=invalid-name
diff --git a/cumin/backends/puppetdb.py b/cumin/backends/puppetdb.py
index b9774c5..b49e424 100644
--- a/cumin/backends/puppetdb.py
+++ b/cumin/backends/puppetdb.py
@@ -1,3 +1,5 @@
+# pylint: skip-file
+# See https://github.com/PyCQA/astroid/issues/437
"""PuppetDB backend."""
from string import capwords
@@ -75,7 +77,7 @@
def add_hosts(self, hosts, neg=False):
"""Required by BaseQuery."""
- if len(hosts) == 0:
+ if not hosts:
return
hosts_tokens = []
@@ -122,13 +124,13 @@
return unique_hosts
- def _get_resource_query(self, key, value=None, operator='='):
+ def _get_resource_query(self, key, value=None, operator='='): # pylint:
disable=no-self-use
"""Build a resource query based on the parameters, resolving the
special cases for %params and @field.
Arguments:
key -- the key of the resource
value -- the value to match, if not specified the key itself will
be matched [optional, default: None]
- operator -- the comparison operator to use, one of
cumin.grammar.operators [optional: default: =]
+ operator -- the comparison operator to use, one of
cumin.grammar.OPERATORS [optional: default: =]
"""
if all(char in key for char in ('%', '@')):
raise InvalidQueryError(("Resource key cannot contain both '%'
(query a resource's parameter) and '@' "
@@ -210,4 +212,5 @@
return resources.json()
-query_class = PuppetDBQuery # Required by the auto-loader in the
cumin.query.Query factory
+# Required by the auto-loader in the cumin.query.Query factory
+query_class = PuppetDBQuery # pylint: disable=invalid-name
diff --git a/cumin/cli.py b/cumin/cli.py
index 187c364..8b27bf3 100644
--- a/cumin/cli.py
+++ b/cumin/cli.py
@@ -9,7 +9,7 @@
import signal
import sys
-from logging.handlers import RotatingFileHandler
+from logging.handlers import RotatingFileHandler # pylint:
disable=ungrouped-imports
import colorama
import yaml
@@ -22,7 +22,7 @@
from cumin.transport import Transport
from cumin.transports import Command
-logger = logging.getLogger(__name__)
+logger = logging.getLogger(__name__) # pylint: disable=invalid-name
OUTPUT_FORMATS = ('txt', 'json')
INTERACTIVE_BANNER = """===== Cumin Interactive REPL =====
# Press Ctrl+d or type exit() to exit the program.
@@ -192,14 +192,14 @@
return config
-def sigint_handler(*args):
+def sigint_handler(*args): # pylint: disable=unused-argument
"""Signal handler for Ctrl+c / SIGINT, raises KeyboardInterruptError.
Arguments (as defined in https://docs.python.org/2/library/signal.html):
signum -- the signal number
frame -- the current stack frame
"""
- if not sys.stdout.isatty():
+ if not sys.stdout.isatty(): # pylint: disable=no-member
logger.warning('Execution interrupted by Ctrl+c/SIGINT')
raise KeyboardInterruptError
@@ -254,7 +254,7 @@
query = QueryBuilder(args.hosts, config, logger).build()
hosts = query.execute()
- if len(hosts) == 0:
+ if not hosts:
stderr('No hosts found that matches the query')
return hosts
@@ -267,7 +267,7 @@
elif args.force:
stderr('FORCE mode enabled, continuing without confirmation')
return hosts
- elif not sys.stdout.isatty():
+ elif not sys.stdout.isatty(): # pylint: disable=no-member
message = 'Not in a TTY but neither DRY-RUN nor FORCE mode were
specified.'
stderr(message)
raise CuminError(message)
@@ -324,7 +324,7 @@
config -- a dictionary with the parsed configuration file
"""
hosts = get_hosts(args, config)
- if len(hosts) == 0:
+ if not hosts:
return 0
worker = Transport.new(config, logger)
@@ -345,7 +345,9 @@
if args.interactive:
# Define a help function h() that will be available in the interactive
shell to print the help message.
# The name is to not shadow the Python built-in help() that might be
usefult too to inspect objects.
- def h(): tqdm.write(INTERACTIVE_BANNER)
+ def h(): # pylint: disable=unused-variable,invalid-name
+ """Helper function for the interactive shell."""
+ tqdm.write(INTERACTIVE_BANNER)
code.interact(banner=INTERACTIVE_BANNER, local=locals())
elif args.output is not None:
tqdm.write('_____FORMATTED_OUTPUT_____')
@@ -372,7 +374,7 @@
except CuminError as e:
stderr(e)
return 2
- except Exception as e:
+ except Exception as e: # pylint: disable=broad-except
stderr('Caught {name} exception:
{msg}'.format(name=e.__class__.__name__, msg=e))
return 3
@@ -390,7 +392,7 @@
except KeyboardInterruptError:
stderr('Execution interrupted by Ctrl+c/SIGINT/Aborted')
exit_code = 98
- except Exception as e:
+ except Exception as e: # pylint: disable=broad-except
stderr('Caught {name} exception:
{msg}'.format(name=e.__class__.__name__, msg=e))
logger.exception('Failed to execute')
exit_code = 99
diff --git a/cumin/grammar.py b/cumin/grammar.py
index c5dbb0c..95de3ef 100644
--- a/cumin/grammar.py
+++ b/cumin/grammar.py
@@ -3,12 +3,12 @@
import pyparsing as pp
# Available categories
-categories = (
+CATEGORIES = (
'F', # Fact
'R', # Resource
)
# Available operators
-operators = ('=', '!=', '>=', '<=', '<', '>', '~')
+OPERATORS = ('=', '!=', '>=', '<=', '<', '>', '~')
def _grammar():
@@ -35,7 +35,7 @@
and_or = (pp.Keyword('and', caseless=True) | pp.Keyword('or',
caseless=True))('bool')
neg = pp.Keyword('not', caseless=True)('neg') # 'neg' is used to allow
the use of dot notation, 'not' is reserved
- operator = pp.oneOf(operators, caseless=True)('operator') # Comparison
operators
+ operator = pp.oneOf(OPERATORS, caseless=True)('operator') # Comparison
operators
quoted_string = pp.quotedString.addParseAction(pp.removeQuotes) # Both
single and double quotes are allowed
# Parentheses
@@ -48,7 +48,7 @@
# Key-value token for allowed categories using the available comparison
operators
# i.e. F:key = value
- category = pp.oneOf(categories, caseless=True)('category')
+ category = pp.oneOf(CATEGORIES, caseless=True)('category')
key = pp.Word(pp.alphanums + '-_.%@:')('key')
selector = pp.Combine(category + ':' + key) # i.e. F:key
# All printables characters except the parentheses that are part of the
grammar
@@ -58,12 +58,12 @@
# Final grammar, see the docstring for it's BNF based on the tokens
defined above
# Groups are used to split the parsed results for an easy access
- grammar = pp.Forward()
+ full_grammar = pp.Forward()
item = pp.Group(pp.Optional(neg) + (token | hosts('hosts'))) | pp.Group(
- pp.Optional(neg) + lpar + grammar + rpar)
- grammar << item + pp.ZeroOrMore(pp.Group(and_or) + grammar)
+ pp.Optional(neg) + lpar + full_grammar + rpar)
+ full_grammar << item + pp.ZeroOrMore(pp.Group(and_or) + full_grammar) #
pylint: disable=expression-not-assigned
- return grammar
+ return full_grammar
-grammar = _grammar()
+grammar = _grammar() # pylint: disable=invalid-name
diff --git a/cumin/tests/__init__.py b/cumin/tests/__init__.py
index e3d937b..742e027 100644
--- a/cumin/tests/__init__.py
+++ b/cumin/tests/__init__.py
@@ -4,7 +4,7 @@
import os
logging.basicConfig(level=logging.DEBUG, name=__name__)
-_tests_base_path = os.path.realpath(os.path.dirname(__file__))
+_TESTS_BASE_PATH = os.path.realpath(os.path.dirname(__file__))
def get_fixture(filename, as_string=False):
@@ -14,7 +14,7 @@
filename -- the file to be opened in the test's fixture directory
as_string -- return the content as a multiline string instead of a list of
lines [optional, default: False]
"""
- with open(os.path.join(_tests_base_path, 'fixtures', filename)) as f:
+ with open(os.path.join(_TESTS_BASE_PATH, 'fixtures', filename)) as f:
if as_string:
content = f.read()
else:
diff --git a/cumin/tests/integration/test_cli.py
b/cumin/tests/integration/test_cli.py
index 931e1a2..23b6e96 100644
--- a/cumin/tests/integration/test_cli.py
+++ b/cumin/tests/integration/test_cli.py
@@ -206,6 +206,8 @@
for j, commands_set in enumerate(_VARIANTS_COMMANDS):
commands_set['params'] = _VARIANTS_PARAMETERS[i] +
commands_set.get('additional_params', [])
test_input =
make_method('params{i:02d}_commands{j:02d}'.format(i=i, j=j), commands_set)
+ if test_input.__doc__ is None:
+ raise AssertionError("Missing __doc__ for test
{name}".format(name=test_input.__name__))
setattr(cls, test_input.__name__, test_input)
return cls
@@ -220,8 +222,9 @@
def setUp(self):
"""Set default properties."""
- self.identifier = os.environ.get('CUMIN_IDENTIFIER')
- self.config = os.path.join(os.environ.get('CUMIN_TMPDIR'),
'config.yaml')
+ self.identifier = os.getenv('CUMIN_IDENTIFIER')
+ self.assertIsNotNone(self.identifier, msg='Unable to find
CUMIN_IDENTIFIER environmental variable')
+ self.config = os.path.join(os.getenv('CUMIN_TMPDIR', ''),
'config.yaml')
self.default_params = ['--force', '-d', '-c', self.config]
self.nodes_prefix = '{identifier}-'.format(identifier=self.identifier)
self.all_nodes = '{prefix}[1-5]'.format(prefix=self.nodes_prefix)
diff --git a/cumin/tests/unit/backends/test_puppetdb.py
b/cumin/tests/unit/backends/test_puppetdb.py
index 39bab82..b3ee47b 100644
--- a/cumin/tests/unit/backends/test_puppetdb.py
+++ b/cumin/tests/unit/backends/test_puppetdb.py
@@ -1,5 +1,5 @@
"""PuppetDB backend tests."""
-
+# pylint: disable=invalid-name
import unittest
import requests_mock
@@ -217,7 +217,7 @@
def _register_uris(self, requests):
"""Setup the requests library mock for each test."""
# Register a requests valid response for each endpoint
- for category in self.query.endpoints.keys():
+ for category in self.query.endpoints:
endpoint = self.query.endpoints[category]
key = self.query.hosts_keys[category]
requests.register_uri('GET', self.query.url + endpoint +
'?query=', status_code=200, json=[
diff --git a/cumin/tests/unit/test_backends.py
b/cumin/tests/unit/test_backends.py
index ea6b444..ad7988d 100644
--- a/cumin/tests/unit/test_backends.py
+++ b/cumin/tests/unit/test_backends.py
@@ -11,4 +11,4 @@
def test_instantiation(self):
"""Class BaseQuery is not instantiable being an abstract class."""
with self.assertRaises(TypeError):
- BaseQuery({})
+ BaseQuery({}) # pylint: disable=abstract-class-instantiated
diff --git a/cumin/tests/unit/test_cli.py b/cumin/tests/unit/test_cli.py
index 43bc213..29e8eec 100644
--- a/cumin/tests/unit/test_cli.py
+++ b/cumin/tests/unit/test_cli.py
@@ -105,6 +105,7 @@
mocked_os.path.exists.return_value = True
cli.setup_logging('filename', debug=True)
logging.setLevel.assert_called_with(DEBUG)
+ self.assertTrue(file_handler.called)
def test_parse_config_ok(self):
"""The configuration file is properly parsed and accessible."""
@@ -133,7 +134,7 @@
@mock.patch('cumin.cli.raw_input')
@mock.patch('cumin.cli.sys.stdout.isatty')
@mock.patch('cumin.cli.logger')
- def test_sigint_handler(self, logging, isatty, mocked_raw_input, stderr):
+ def test_sigint_handler(self, logging, isatty, mocked_raw_input, stderr):
# pylint: disable=unused-argument
"""Calling the SIGINT handler should raise KeyboardInterrupt or not
based on tty and answer."""
# Signal handler called without a tty
isatty.return_value = False
@@ -198,6 +199,8 @@
with self.assertRaises(cli.KeyboardInterruptError):
cli.get_hosts(args, config)
+ self.assertTrue(stderr.called)
+
@mock.patch('cumin.cli.stderr')
@mock.patch('cumin.cli.sys.stdout.isatty')
def test_get_hosts_no_tty_ko(self, isatty, stderr):
@@ -239,3 +242,4 @@
config = {'backend': 'direct', 'transport': 'clustershell'}
cli.run(args, config)
transport.new.assert_called_once_with(config, cli.logger)
+ self.assertTrue(stderr.called)
diff --git a/cumin/tests/unit/test_grammar.py b/cumin/tests/unit/test_grammar.py
index 8d615f4..dfd77e9 100644
--- a/cumin/tests/unit/test_grammar.py
+++ b/cumin/tests/unit/test_grammar.py
@@ -6,14 +6,15 @@
from cumin.tests import get_fixture
+def _get_category_key_token(category='F', key='key1', operator='=',
value='value1'):
+ """Generate and return a category token string and it's expected
dictionary of tokens when parsed."""
+ expected = {'category': category, 'key': key, 'operator': operator,
'value': value}
+ token = '{category}:{key} {operator} {value}'.format(**expected)
+ return token, expected
+
+
class TestGrammar(unittest.TestCase):
"""Grammar class tests."""
-
- def _get_category_key_token(self, category='F', key='key1', operator='=',
value='value1'):
- """Generate and return a category token string and it's expected
dictionary of tokens when parsed."""
- expected = {'category': category, 'key': key, 'operator': operator,
'value': value}
- token = '{category}:{key} {operator} {value}'.format(**expected)
- return token, expected
def test_valid_strings(self):
"""Run quick pyparsing test over valid grammar strings."""
@@ -27,7 +28,7 @@
def test_single_category_key_token(self):
"""A valid single token with a category that has key is properly
parsed and interpreted."""
- token, expected = self._get_category_key_token()
+ token, expected = _get_category_key_token()
parsed = grammar.parseString(token, parseAll=True)
self.assertDictEqual(parsed[0].asDict(), expected)
diff --git a/cumin/tests/unit/test_query.py b/cumin/tests/unit/test_query.py
index cd583b7..7c2b88d 100644
--- a/cumin/tests/unit/test_query.py
+++ b/cumin/tests/unit/test_query.py
@@ -1,5 +1,6 @@
"""Query handling tests."""
+import logging
import os
import pkgutil
import unittest
@@ -19,6 +20,9 @@
@staticmethod
def new(config, logger=None):
"""Return an instance of the mocked query class."""
+ if logger is not None and not isinstance(logger, logging.Logger):
+ raise AssertionError('Expected logger parameter to be None or
logging instance, got: {logger}'.format(
+ logger=logger))
if not isinstance(config, dict):
raise AssertionError("Expected instance of dict, got type '{type}'
for config.".format(type=type(config)))
return mock.MagicMock(spec_set=BaseQuery)
@@ -98,4 +102,4 @@
"""QueryBuilder._parse_token() should raise RuntimeError for an
invalid token."""
query_builder = QueryBuilder(self.invalid_query_string, self.config)
with self.assertRaisesRegexp(RuntimeError, r"Invalid query string
syntax"):
- query_builder._parse_token('invalid_token')
+ query_builder._parse_token('invalid_token') # pylint:
disable=protected-access
diff --git a/cumin/tests/unit/transports/test_clustershell.py
b/cumin/tests/unit/transports/test_clustershell.py
index 187adc7..180e8ac 100644
--- a/cumin/tests/unit/transports/test_clustershell.py
+++ b/cumin/tests/unit/transports/test_clustershell.py
@@ -1,5 +1,5 @@
"""ClusterShell transport tests."""
-
+# pylint: disable=invalid-name,no-member,protected-access
import unittest
import mock
@@ -32,7 +32,7 @@
"""ClusterShell backend worker test class."""
@mock.patch('cumin.transports.clustershell.Task.task_self')
- def setUp(self, task_self):
+ def setUp(self, task_self): # pylint: disable=arguments-differ
"""Initialize default properties and instances"""
self.config = {
'clustershell': {
@@ -126,6 +126,8 @@
self.worker.task.iter_buffers = TestClusterShellWorker.iter_buffers
self.worker.handler = 'async'
self.worker.execute()
+ nodes = None
+ output = None
for nodes, output in self.worker.get_results():
pass
self.assertEqual(str(nodes), 'node[90-92]')
@@ -140,6 +142,8 @@
def test_handler_setter_invalid(self):
"""Raise WorkerError if trying to set it to an invalid class or
value"""
class InvalidClass(object):
+ """Invalid class."""
+
pass
with self.assertRaisesRegexp(WorkerError, r'handler must be one of'):
@@ -173,7 +177,7 @@
class TestBaseEventHandler(unittest.TestCase):
"""BaseEventHandler test class."""
- def setUp(self, *args):
+ def setUp(self, *args): # pylint: disable=arguments-differ
"""Initialize default properties and instances."""
self.nodes = ['node1', 'node2']
self.commands = [Command('command1', ok_codes=[0, 100]),
Command('command2', timeout=5)]
@@ -182,6 +186,7 @@
self.worker.command = 'command1'
self.worker.nodes = clustershell.NodeSet.NodeSet.fromlist(self.nodes)
self.handler = None
+ self.args = args
@mock.patch('cumin.transports.clustershell.colorama')
def test_close(self, colorama):
@@ -201,7 +206,7 @@
self.pbar_ok = mock.Mock()
self.pbar_ko = mock.Mock()
- def close(self, worker):
+ def close(self, task):
"""Required by the BaseEventHandler class."""
@@ -210,7 +215,7 @@
@mock.patch('cumin.transports.clustershell.colorama')
@mock.patch('cumin.transports.clustershell.tqdm')
- def setUp(self, tqdm, colorama):
+ def setUp(self, tqdm, colorama): # pylint: disable=arguments-differ
"""Initialize default properties and instances."""
super(TestConcreteBaseEventHandler, self).setUp()
self.handler = ConcreteBaseEventHandler(
@@ -237,6 +242,7 @@
self.handler.on_timeout(self.worker.task)
self.assertTrue(self.handler.pbar_ko.update.called)
self.assertTrue(self.handler.global_timedout)
+ self.assertTrue(tqdm.write.called)
def test_ev_pickup(self):
"""Calling ev_pickup() should set the state of the current node to
running."""
@@ -288,7 +294,7 @@
@mock.patch('cumin.transports.clustershell.logging')
@mock.patch('cumin.transports.clustershell.colorama')
@mock.patch('cumin.transports.clustershell.tqdm')
- def setUp(self, tqdm, colorama, logger):
+ def setUp(self, tqdm, colorama, logger): # pylint:
disable=arguments-differ
"""Initialize default properties and instances."""
super(TestSyncEventHandler, self).setUp()
self.handler = clustershell.SyncEventHandler(
@@ -297,6 +303,7 @@
self.worker.eh = self.handler
self.tqdm = tqdm
self.colorama = colorama
+ self.logger = logger
def test_instantiation(self):
"""An instance of SyncEventHandler should be an instance of
BaseEventHandler."""
@@ -373,7 +380,7 @@
self.assertTrue(self.handler.nodes[self.worker.current_node].state.is_failed)
@mock.patch('cumin.transports.clustershell.tqdm')
- def test_close(self, tqdm):
+ def test_close(self, tqdm): # pylint: disable=arguments-differ
"""Calling close should print the report when needed."""
self.handler.current_command_index = 2
self.handler.close(self.worker)
@@ -386,11 +393,14 @@
@mock.patch('cumin.transports.clustershell.logging')
@mock.patch('cumin.transports.clustershell.colorama')
@mock.patch('cumin.transports.clustershell.tqdm')
- def setUp(self, tqdm, colorama, logger):
+ def setUp(self, tqdm, colorama, logger): # pylint:
disable=arguments-differ
"""Initialize default properties and instances."""
super(TestAsyncEventHandler, self).setUp()
self.handler = clustershell.AsyncEventHandler(self.nodes,
self.commands)
self.worker.eh = self.handler
+ self.tqdm = tqdm
+ self.colorama = colorama
+ self.logger = logger
def test_instantiation(self):
"""An instance of AsyncEventHandler should be an instance of
BaseEventHandler and initialize progress bars."""
@@ -430,7 +440,7 @@
self.handler.ev_timer(mock.Mock())
@mock.patch('cumin.transports.clustershell.tqdm')
- def test_close(self, tqdm):
+ def test_close(self, tqdm): # pylint: disable=arguments-differ
"""Calling close with a worker should close progress bars."""
self.worker.task.iter_buffers = TestClusterShellWorker.iter_buffers
self.worker.num_timeout.return_value = 0
diff --git a/cumin/tests/unit/transports/test_init.py
b/cumin/tests/unit/transports/test_init.py
index 315416e..5911c2e 100644
--- a/cumin/tests/unit/transports/test_init.py
+++ b/cumin/tests/unit/transports/test_init.py
@@ -1,4 +1,5 @@
"""Transport tests."""
+# pylint: disable=protected-access
import unittest
import mock
@@ -19,16 +20,6 @@
yield "node", "output"
@property
- def mode(self):
- """Required by BaseWorker"""
- return self._mode
-
- @mode.setter
- def mode(self, value):
- """Required by BaseWorker"""
- self._mode = value
-
- @property
def handler(self):
"""Required by BaseWorker"""
return self._handler
@@ -43,10 +34,11 @@
"""Command class tests."""
def setUp(self):
- command_with_options = 'command --with "options" -a -n -d params with\
spaces'
+ """Initialize test commands."""
+ command_with_options = r'command --with "options" -a -n -d params
with\ spaces'
self.same_command_a = transports.Command(command_with_options)
- self.same_command_b = "command --with 'options' -a -n -d params
with\ spaces"
- command_with_nested_quotes = 'command --with \'nested "quotes"\' -a -n
-d params with\ spaces'
+ self.same_command_b = r"command --with 'options' -a -n -d params
with\ spaces"
+ command_with_nested_quotes = 'command --with \'nested "quotes"\' -a -n
-d params with\\ spaces'
self.commands = [
{'command': 'command1'},
@@ -62,7 +54,7 @@
{'command': command_with_nested_quotes, 'ok_codes': [0, 255]},
{'command': command_with_nested_quotes, 'timeout': 5, 'ok_codes':
[0, 255]},
]
- self.different_command = "command --with 'other options' -a -n -d
other_params with\ spaces"
+ self.different_command = r"command --with 'other options' -a -n
-d other_params with\ spaces"
for command in self.commands:
command['obj'] = transports.Command(
command['command'], timeout=command.get('timeout', None),
ok_codes=command.get('ok_codes', None))
@@ -78,7 +70,8 @@
def test_repr(self):
"""A repr of a Command should allow to instantiate an instance with
the same properties."""
for command in self.commands:
- command_instance = eval(repr(command['obj']))
+ # Bandit and pylint would require to use ast.literal_eval, but it
will not work with objects
+ command_instance = eval(repr(command['obj'])) # nosec pylint:
disable=eval-used
self.assertIsInstance(command_instance, transports.Command)
self.assertEqual(repr(command_instance), repr(command['obj']))
self.assertEqual(command_instance.command, command['obj'].command)
@@ -100,7 +93,7 @@
self.assertEqual(command['obj'], command['command'])
with self.assertRaisesRegexp(ValueError, 'Unable to compare
instance of'):
- command['obj'] == 1
+ command['obj'] == 1 # pylint: disable=pointless-statement
self.assertEqual(self.same_command_a, self.same_command_b)
@@ -129,7 +122,7 @@
self.assertNotEqual(command['obj'], command['command'])
with self.assertRaisesRegexp(ValueError, 'Unable to compare
instance of'):
- command['obj'] == 1
+ command['obj'] == 1 # pylint: disable=pointless-statement
def test_timeout_getter(self):
"""Should return the timeout set, None otherwise."""
@@ -226,7 +219,7 @@
"""Accessing a property with an invalid name should raise
AttributeError."""
state = transports.State(init=transports.State.failed)
with self.assertRaisesRegexp(AttributeError, 'object has no
attribute'):
- state.invalid_property
+ state.invalid_property # pylint: disable=pointless-statement
def test_repr(self):
"""A State repr should return its representation that allows to
recreate the same State instance."""
@@ -309,7 +302,7 @@
def test_instantiation(self):
"""Raise if instantiated directly, should return an instance of
BaseWorker if inherited."""
with self.assertRaises(TypeError):
- transports.BaseWorker({})
+ transports.BaseWorker({}) # pylint:
disable=abstract-class-instantiated
self.assertIsInstance(ConcreteBaseWorker({}), transports.BaseWorker)
diff --git a/cumin/tests/vulture_whitelist.py b/cumin/tests/vulture_whitelist.py
new file mode 100644
index 0000000..355fecd
--- /dev/null
+++ b/cumin/tests/vulture_whitelist.py
@@ -0,0 +1,16 @@
+"""Vulture whitelist to avoid false positives."""
+
+
+class Whitelist:
+ """Helper class that allows mocking Python objects."""
+
+ def __getattr__(self, _):
+ """Mocking magic method __getattr__."""
+ pass
+
+
+whitelist_logging = Whitelist()
+whitelist_logging.raiseExceptions
+
+whitelist_cli = Whitelist()
+whitelist_cli.run.h
diff --git a/cumin/transports/__init__.py b/cumin/transports/__init__.py
index 783edb8..6460232 100644
--- a/cumin/transports/__init__.py
+++ b/cumin/transports/__init__.py
@@ -204,7 +204,7 @@
if isinstance(other, int):
return self._state - other
elif isinstance(other, State):
- return self._state - other._state
+ return self._state - other._state # pylint:
disable=protected-access
else:
raise ValueError("Unable to compare instance of '{other}' with
State instance".format(other=type(other)))
@@ -357,7 +357,8 @@
value -- The success ratio threshold that must be reached to consider
the run successful. A float between 0
and 1 or None. The specific meaning might change based on the
chosen transport. [default: 1]
"""
- if value is not None and (not isinstance(value, float) or not (0.0 <=
value <= 1.0)):
+ if value is not None and (not isinstance(value, float) or
+ not (0.0 <= value <= 1.0)): # pylint:
disable=superfluous-parens
raise WorkerError("success_threshold must be a float beween 0 and
1, got '{value_type}': {value}".format(
value_type=type(value), value=value))
diff --git a/cumin/transports/clustershell.py b/cumin/transports/clustershell.py
index 12efac8..0c5b8fd 100644
--- a/cumin/transports/clustershell.py
+++ b/cumin/transports/clustershell.py
@@ -27,14 +27,14 @@
# Set any ClusterShell task options
for key, value in config.get('clustershell', {}).items():
- if type(value) == list:
+ if isinstance(value, list):
self.task.set_info(key, ' '.join(value))
else:
self.task.set_info(key, value)
def execute(self):
"""Required by BaseWorker."""
- if len(self.commands) == 0:
+ if not self.commands:
self.logger.warning('No commands provided')
return
@@ -45,7 +45,7 @@
first_batch = NodeSet.NodeSet.fromlist(self.hosts[:self.batch_size])
# Instantiate handler
- self._handler_instance = self.handler(
+ self._handler_instance = self.handler( # pylint: disable=not-callable
self.hosts, self.commands,
success_threshold=self.success_threshold, batch_size=self.batch_size,
batch_sleep=self.batch_sleep, logger=self.logger,
first_batch=first_batch)
@@ -86,7 +86,7 @@
The available default handlers are defined in DEFAULT_HANDLERS.
"""
- if type(value) == type and issubclass(value, BaseEventHandler):
+ if isinstance(value, type) and issubclass(value, BaseEventHandler):
self._handler = value
elif value in DEFAULT_HANDLERS.keys():
self._handler = DEFAULT_HANDLERS[value]
@@ -285,7 +285,7 @@
return (log_message, str(nodes_string))
- def _print_report_line(self, message, color=colorama.Fore.RED,
nodes_string=''):
+ def _print_report_line(self, message, color=colorama.Fore.RED,
nodes_string=''): # pylint: disable=no-self-use
"""Helper to print a tqdm-friendly colored status line with
success/failure ratio and optional list of nodes.
Arguments:
@@ -612,7 +612,7 @@
self.logger.debug("skipped timer")
return
- if len(pending) > 0:
+ if pending:
# This usually happens when executing in batches
self.logger.warning("command '{command}' was not executed on:
{nodes}".format(
command=command, nodes=NodeSet.NodeSet.fromlist(pending)))
@@ -627,7 +627,7 @@
# Only nodes in pending state will be scheduled for
the next command
node.state.update(State.pending)
finally:
- self.lock.release()
+ self.lock.release()
if restart:
self.start_command(schedule=True)
@@ -775,5 +775,6 @@
self.return_value = 1
-worker_class = ClusterShellWorker # Required by the auto-loader in the
cumin.transport.Transport factory
+# Required by the auto-loader in the cumin.transport.Transport factory
+worker_class = ClusterShellWorker # pylint: disable=invalid-name
DEFAULT_HANDLERS = {'sync': SyncEventHandler, 'async': AsyncEventHandler} #
Available default EventHandler classes
--
To view, visit https://gerrit.wikimedia.org/r/361040
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf2965ec31beb9325c07df8aa3e2d1d71265bc2c
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
