jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/339833 )
Change subject: Fix additional minor issues reported by codacy
......................................................................
Fix additional minor issues reported by codacy
- add module docstrings
- define instance variables only in __init__()
- improve ignored parameters on callbacks
Bug: T158967
Change-Id: I65f85838fb2c0772006e9a0a6e16e02d55ae966c
---
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/query.py
M cumin/tests/__init__.py
M cumin/tests/unit/backends/test_direct.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/test_transport.py
M cumin/tests/unit/test_transports.py
M cumin/tests/unit/transports/test_clustershell.py
M cumin/transport.py
M cumin/transports/__init__.py
M cumin/transports/clustershell.py
19 files changed, 46 insertions(+), 9 deletions(-)
Approvals:
jenkins-bot: Verified
Volans: Looks good to me, approved
diff --git a/cumin/backends/__init__.py b/cumin/backends/__init__.py
index 158c124..ee5dc97 100644
--- a/cumin/backends/__init__.py
+++ b/cumin/backends/__init__.py
@@ -1,3 +1,5 @@
+"""Abstract backend"""
+
import logging
from abc import ABCMeta, abstractmethod
diff --git a/cumin/backends/direct.py b/cumin/backends/direct.py
index 9dfb543..62e6c59 100644
--- a/cumin/backends/direct.py
+++ b/cumin/backends/direct.py
@@ -1,3 +1,5 @@
+"""Direct backend"""
+
from ClusterShell.NodeSet import NodeSet
from cumin.backends import BaseQuery, InvalidQueryError
diff --git a/cumin/backends/puppetdb.py b/cumin/backends/puppetdb.py
index 6751c83..9a8e4d2 100644
--- a/cumin/backends/puppetdb.py
+++ b/cumin/backends/puppetdb.py
@@ -1,3 +1,5 @@
+"""PuppetDB backend"""
+
import requests
from requests.packages import urllib3
diff --git a/cumin/cli.py b/cumin/cli.py
index d8a0fca..3d7a33b 100644
--- a/cumin/cli.py
+++ b/cumin/cli.py
@@ -147,7 +147,7 @@
return config
-def sigint_handler(signum, frame):
+def sigint_handler(*args):
""" Signal handler for Ctrl+c / SIGINT, raises KeyboardInterruptError
Arguments (as defined in
https://docs.python.org/2/library/signal.html):
diff --git a/cumin/grammar.py b/cumin/grammar.py
index 2aa36f1..8cebf45 100644
--- a/cumin/grammar.py
+++ b/cumin/grammar.py
@@ -1,3 +1,5 @@
+"""Query grammar definition"""
+
import pyparsing as pp
# Available categories
diff --git a/cumin/query.py b/cumin/query.py
index 5449b30..08c0ee9 100644
--- a/cumin/query.py
+++ b/cumin/query.py
@@ -1,3 +1,5 @@
+"""Query handling: factory and builder"""
+
import importlib
import logging
diff --git a/cumin/tests/__init__.py b/cumin/tests/__init__.py
index ca045c7..93310cd 100644
--- a/cumin/tests/__init__.py
+++ b/cumin/tests/__init__.py
@@ -1,3 +1,5 @@
+"""Tests utils"""
+
import logging
import os
diff --git a/cumin/tests/unit/backends/test_direct.py
b/cumin/tests/unit/backends/test_direct.py
index ffafba2..a217569 100644
--- a/cumin/tests/unit/backends/test_direct.py
+++ b/cumin/tests/unit/backends/test_direct.py
@@ -1,3 +1,4 @@
+"""Direct backend tests"""
import unittest
from ClusterShell.NodeSet import NodeSet
diff --git a/cumin/tests/unit/backends/test_puppetdb.py
b/cumin/tests/unit/backends/test_puppetdb.py
index 3b3e595..37195dd 100644
--- a/cumin/tests/unit/backends/test_puppetdb.py
+++ b/cumin/tests/unit/backends/test_puppetdb.py
@@ -1,3 +1,4 @@
+"""PuppetDB backend tests"""
import unittest
import requests_mock
diff --git a/cumin/tests/unit/test_backends.py
b/cumin/tests/unit/test_backends.py
index f3b1294..5e050e5 100644
--- a/cumin/tests/unit/test_backends.py
+++ b/cumin/tests/unit/test_backends.py
@@ -1,3 +1,4 @@
+"""Abstract query tests"""
import unittest
from cumin.backends import BaseQuery
diff --git a/cumin/tests/unit/test_cli.py b/cumin/tests/unit/test_cli.py
index f768f4f..87c1349 100644
--- a/cumin/tests/unit/test_cli.py
+++ b/cumin/tests/unit/test_cli.py
@@ -1,3 +1,4 @@
+"""CLI tests"""
import unittest
from logging import DEBUG, INFO
diff --git a/cumin/tests/unit/test_grammar.py b/cumin/tests/unit/test_grammar.py
index 7a53782..7ddea30 100644
--- a/cumin/tests/unit/test_grammar.py
+++ b/cumin/tests/unit/test_grammar.py
@@ -1,3 +1,4 @@
+"""Grammar tests"""
import unittest
from cumin.grammar import grammar
diff --git a/cumin/tests/unit/test_query.py b/cumin/tests/unit/test_query.py
index 013da03..4667a77 100644
--- a/cumin/tests/unit/test_query.py
+++ b/cumin/tests/unit/test_query.py
@@ -1,3 +1,4 @@
+"""Query handling tests"""
import os
import pkgutil
import unittest
@@ -17,6 +18,8 @@
@staticmethod
def new(config, logger=None):
"""Return an instance of the mocked query class"""
+ 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)
diff --git a/cumin/tests/unit/test_transport.py
b/cumin/tests/unit/test_transport.py
index d1c4e25..8fae76e 100644
--- a/cumin/tests/unit/test_transport.py
+++ b/cumin/tests/unit/test_transport.py
@@ -1,3 +1,5 @@
+"""Transport class tests"""
+
import os
import pkgutil
import unittest
diff --git a/cumin/tests/unit/test_transports.py
b/cumin/tests/unit/test_transports.py
index 38eb406..1024c1e 100644
--- a/cumin/tests/unit/test_transports.py
+++ b/cumin/tests/unit/test_transports.py
@@ -1,3 +1,5 @@
+"""Abstract worker tests"""
+
import os
import unittest
diff --git a/cumin/tests/unit/transports/test_clustershell.py
b/cumin/tests/unit/transports/test_clustershell.py
index 9d1c4ea..4771d0f 100644
--- a/cumin/tests/unit/transports/test_clustershell.py
+++ b/cumin/tests/unit/transports/test_clustershell.py
@@ -1,3 +1,4 @@
+"""ClusterShell transport tests"""
import unittest
import mock
@@ -13,6 +14,7 @@
"""An instance of worker_class should be an instance of BaseWorker"""
worker = clustershell.worker_class({})
self.assertIsInstance(worker, BaseWorker)
+ task_self.assert_called_once_with()
class TestClusterShellWorker(unittest.TestCase):
@@ -91,7 +93,7 @@
def test_get_results(self):
"""Calling get_results() should call ClusterShell iter_buffers with
the right parameters"""
- self.worker.task.iter_buffers = TestClusterShellWorker._iter_buffers
+ self.worker.task.iter_buffers = TestClusterShellWorker.iter_buffers
self.worker.execute(self.nodes, self.commands, 'async')
for nodes, output in self.worker.get_results():
pass
@@ -99,7 +101,7 @@
self.assertEqual(output, 'output 9')
@staticmethod
- def _iter_buffers():
+ def iter_buffers():
"""A generator to simulate the buffer iteration of ClusterShell
objects"""
for i in xrange(10):
yield 'output {}'.format(i), ['node{}0'.format(i),
'node{}1'.format(i), 'node{}2'.format(i)]
@@ -107,13 +109,14 @@
class TestBaseEventHandler(unittest.TestCase):
"""BaseEventHandler test class"""
- def setUp(self):
+ def setUp(self, *args):
"""Initialize default properties and instances"""
self.nodes = ['node1', 'node2']
self.commands = ['command1', 'command2']
self.worker = mock.MagicMock()
self.worker.current_node = 'node1'
self.worker.command = 'command1'
+ self.handler = None
@mock.patch('cumin.transports.clustershell.colorama')
def test_close(self, colorama):
@@ -121,6 +124,7 @@
self.handler = clustershell.BaseEventHandler(self.nodes, self.commands)
with self.assertRaises(NotImplementedError):
self.handler.close(self.worker)
+ colorama.init.assert_called_once_with()
class ConcreteBaseEventHandler(clustershell.BaseEventHandler):
@@ -204,7 +208,7 @@
def test_ev_close(self):
"""Calling close should close progress bars"""
- self.worker.task.iter_buffers = TestClusterShellWorker._iter_buffers
+ self.worker.task.iter_buffers = TestClusterShellWorker.iter_buffers
self.worker.num_timeout.return_value = 0
self.handler.ev_close(self.worker)
self.assertTrue(self.handler.pbar_ko.close.called)
@@ -260,10 +264,11 @@
self.assertFalse(self.worker.task.shell.called)
self.assertTrue(self.handler.pbar_ko.update.called)
- def test_close(self):
+ @mock.patch('cumin.transports.clustershell.colorama')
+ def test_close(self, colorama):
"""Calling close with a worker should close progress bars"""
self.handler.ev_start(self.worker)
- self.worker.task.iter_buffers = TestClusterShellWorker._iter_buffers
+ self.worker.task.iter_buffers = TestClusterShellWorker.iter_buffers
self.worker.num_timeout.return_value = 0
self.handler.close(self.worker)
self.assertTrue(self.handler.pbar_ok.close.called)
diff --git a/cumin/transport.py b/cumin/transport.py
index af762dd..10e9117 100644
--- a/cumin/transport.py
+++ b/cumin/transport.py
@@ -1,3 +1,5 @@
+"""Transport factory"""
+
import importlib
diff --git a/cumin/transports/__init__.py b/cumin/transports/__init__.py
index 6abd0f7..50ffde9 100644
--- a/cumin/transports/__init__.py
+++ b/cumin/transports/__init__.py
@@ -1,3 +1,5 @@
+"""Abstract transport"""
+
import logging
import os
diff --git a/cumin/transports/clustershell.py b/cumin/transports/clustershell.py
index 433b72e..d4e6523 100644
--- a/cumin/transports/clustershell.py
+++ b/cumin/transports/clustershell.py
@@ -1,3 +1,5 @@
+"""ClusterShell transport: worker and event handlers"""
+
from collections import defaultdict
import ClusterShell
@@ -87,6 +89,8 @@
self.kwargs = kwargs
self.success_nodes = []
self.failed_commands = defaultdict(list)
+ self.pbar_ok = None
+ self.pbar_ko = None
# Initialize color and progress bar formats
# TODO: decouple the output handling from the event handling
@@ -108,7 +112,7 @@
Arguments: according to EventHandler interface
"""
self.failed_commands[worker.command].append(worker.current_node)
- if hasattr(self, 'pbar_ko'):
+ if self.pbar_ko is not None:
self.pbar_ko.update()
tqdm.write(worker.current_errmsg)
@@ -117,7 +121,7 @@
Arguments: according to EventHandler interface
"""
- if hasattr(self, 'pbar_ko'):
+ if self.pbar_ko is not None:
self.pbar_ko.update(worker.num_timeout())
def _print_report_line(self, num, tot, message, color=colorama.Fore.RED,
nodes=None):
--
To view, visit https://gerrit.wikimedia.org/r/339833
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I65f85838fb2c0772006e9a0a6e16e02d55ae966c
Gerrit-PatchSet: 2
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <rcocci...@wikimedia.org>
Gerrit-Reviewer: Volans <rcocci...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
