From 5e7be479eacd4d1085cab152c35dcb6433a146ed Mon Sep 17 00:00:00 2001 From: Vratko Polak Date: Thu, 26 Apr 2018 15:15:51 +0200 Subject: [PATCH] Fix various pylint 1.5.4 warnings + DUTSetup.py:424 Else clause on loop without a break statement + InterfaceUtil.py:400 Else clause on loop without a break statement + QemuUtils.py:564 Wrong continued indentation + SetupDPDKTest.py: Locally enabling broad-except + VatExecutor.py: Catching too general exception Exception + ssh.py:95 No exception type(s) specified. + HTTPRequest.py: Tolerate HTTPCodes.OK + multiple: Drop ":returns: None" from docstrings. There are still several warnings present: - R0902(too-many-instance-attributes) - R0912(too-many-branches) - R0913(too-many-arguments) - R0914(too-many-locals) - R0915(too-many-statements) - R0401(cyclic-import) And there are multiple blocks of similar lines, mainly across various Setup*Test.py files: - R0801(duplicate-code) Change-Id: I582575cb52b85d69d268e6374852f6e74bb71052 Signed-off-by: Vratko Polak --- resources/libraries/python/DPDK/DPDKTools.py | 2 -- resources/libraries/python/DPDK/L2fwdTest.py | 1 - resources/libraries/python/DPDK/L3fwdTest.py | 1 - resources/libraries/python/DPDK/SetupDPDKTest.py | 19 +++++++----- resources/libraries/python/DUTSetup.py | 3 +- resources/libraries/python/HTTPRequest.py | 2 +- resources/libraries/python/InterfaceUtil.py | 6 ++-- resources/libraries/python/L2Util.py | 1 - resources/libraries/python/PacketVerifier.py | 1 - resources/libraries/python/QemuUtils.py | 4 +-- resources/libraries/python/SFC/SFCTest.py | 1 - resources/libraries/python/SFC/VerifyPacket.py | 3 -- resources/libraries/python/TLDK/UdpTest.py | 1 - resources/libraries/python/VatExecutor.py | 9 +++--- resources/libraries/python/ssh.py | 34 +++++++++++++--------- resources/tools/topology/update_topology.py | 2 -- .../traffic_scripts/honeycomb/read_vpp_version.py | 1 - 17 files changed, 43 insertions(+), 48 deletions(-) diff --git a/resources/libraries/python/DPDK/DPDKTools.py b/resources/libraries/python/DPDK/DPDKTools.py index a256ae5ddc..b0e67b7ab8 100644 --- a/resources/libraries/python/DPDK/DPDKTools.py +++ b/resources/libraries/python/DPDK/DPDKTools.py @@ -37,7 +37,6 @@ class DPDKTools(object): :type dut_node: dict :type dut_if1: str :type dut_if2: str - :returns: none :raises RuntimeError: If it fails to bind the interfaces to igb_uio. """ if dut_node['type'] == NodeType.DUT: @@ -72,7 +71,6 @@ class DPDKTools(object): :type dut_node: dict :type dut_if1: str :type dut_if2: str - :returns: none :raises RuntimeError: If it fails to cleanup the dpdk. """ if dut_node['type'] == NodeType.DUT: diff --git a/resources/libraries/python/DPDK/L2fwdTest.py b/resources/libraries/python/DPDK/L2fwdTest.py index cfaa39991d..65ad6a53a4 100644 --- a/resources/libraries/python/DPDK/L2fwdTest.py +++ b/resources/libraries/python/DPDK/L2fwdTest.py @@ -39,7 +39,6 @@ class L2fwdTest(object): :type nb_cores: str :type queue_nums: str :type jumbo_frames: str - :returns: none :raises RuntimeError: If the script "run_l2fwd.sh" fails. """ if dut_node['type'] == NodeType.DUT: diff --git a/resources/libraries/python/DPDK/L3fwdTest.py b/resources/libraries/python/DPDK/L3fwdTest.py index d7e9305391..15e656af07 100644 --- a/resources/libraries/python/DPDK/L3fwdTest.py +++ b/resources/libraries/python/DPDK/L3fwdTest.py @@ -44,7 +44,6 @@ class L3fwdTest(object): :type lcores_list: str :type queue_nums: str :type jumbo_frames: str - :returns: none """ if dut_node['type'] == NodeType.DUT: adj_mac0, adj_mac1 = L3fwdTest.get_adj_mac(nodes_info, dut_node, diff --git a/resources/libraries/python/DPDK/SetupDPDKTest.py b/resources/libraries/python/DPDK/SetupDPDKTest.py index d94a383786..1e88f8d8c1 100644 --- a/resources/libraries/python/DPDK/SetupDPDKTest.py +++ b/resources/libraries/python/DPDK/SetupDPDKTest.py @@ -34,7 +34,9 @@ __all__ = ["SetupDPDKTest"] def pack_framework_dir(): - """Pack the testing WS into temp file, return its name.""" + """Pack the testing WS into temp file, return its name. + + :raise RuntimeError: If command returns nonzero return code.""" tmpfile = NamedTemporaryFile(suffix=".tgz", prefix="DPDK-testing-") file_name = tmpfile.name @@ -50,7 +52,7 @@ def pack_framework_dir(): return_code = proc.wait() if return_code != 0: - raise Exception("Could not pack testing framework.") + raise RuntimeError("Could not pack testing framework.") return file_name @@ -81,6 +83,7 @@ def extract_tarball_at_node(tarball, node): :type tarball: str :type node: dict :returns: nothing + :raise RuntimeError: If command returns nonzero return code. """ logger.console('Extracting tarball to {0} on {1}'.format( con.REMOTE_FW_DIR, node['host'])) @@ -92,7 +95,7 @@ def extract_tarball_at_node(tarball, node): (ret_code, _, stderr) = ssh.exec_command(cmd, timeout=30) if ret_code != 0: logger.error('Unpack error: {0}'.format(stderr)) - raise Exception('Failed to unpack {0} at node {1}'.format( + raise RuntimeError('Failed to unpack {0} at node {1}'.format( tarball, node['host'])) @@ -103,6 +106,7 @@ def create_env_directory_at_node(node): :param node: Dictionary created from topology, will only install in the TG :type node: dict :returns: nothing + :raise RuntimeError: If command returns nonzero return code. """ logger.console('Extracting virtualenv, installing requirements.txt ' 'on {0}'.format(node['host'])) @@ -114,7 +118,7 @@ def create_env_directory_at_node(node): .format(con.REMOTE_FW_DIR), timeout=100) if ret_code != 0: logger.error('Virtualenv creation error: {0}'.format(stdout + stderr)) - raise Exception('Virtualenv setup failed') + raise RuntimeError('Virtualenv setup failed') else: logger.console('Virtualenv created on {0}'.format(node['host'])) @@ -125,6 +129,7 @@ def install_dpdk_test(node): :param node: Dictionary created from topology :type node: dict :returns: nothing + :raise RuntimeError: If command returns nonzero return code. """ arch = Topology.get_node_arch(node) logger.console('Install the DPDK on {0} ({1})'.format(node['host'], @@ -139,11 +144,10 @@ def install_dpdk_test(node): if ret_code != 0: logger.error('Install the DPDK error: {0}'.format(stderr)) - raise Exception('Install the DPDK failed') + raise RuntimeError('Install the DPDK failed') else: logger.console('Install the DPDK on {0} success!'.format(node['host'])) -#pylint: disable=broad-except def setup_node(args): """Run all set-up methods for a node. @@ -168,13 +172,12 @@ def setup_node(args): install_dpdk_test(node) if node['type'] == NodeType.TG: create_env_directory_at_node(node) - except Exception as exc: + except RuntimeError as exc: logger.error("Node setup failed, error:'{0}'".format(exc.message)) return False else: logger.console('Setup of node {0} done'.format(node['host'])) return True -#pylint: enable=broad-except def delete_local_tarball(tarball): """Delete local tarball to prevent disk pollution. diff --git a/resources/libraries/python/DUTSetup.py b/resources/libraries/python/DUTSetup.py index 9c78cfe6aa..d08704bba1 100644 --- a/resources/libraries/python/DUTSetup.py +++ b/resources/libraries/python/DUTSetup.py @@ -421,8 +421,7 @@ class DUTSetup(object): return None if name == 'Driver:': return value - else: - return None + return None @staticmethod def kernel_module_verify(node, module, force_load=False): diff --git a/resources/libraries/python/HTTPRequest.py b/resources/libraries/python/HTTPRequest.py index a64513b9ba..0f650a89a1 100644 --- a/resources/libraries/python/HTTPRequest.py +++ b/resources/libraries/python/HTTPRequest.py @@ -33,7 +33,7 @@ from requests.auth import HTTPBasicAuth @unique class HTTPCodes(IntEnum): """HTTP status codes""" - OK = 200 + OK = 200 # HTTP standard code name. # pylint: disable=invalid-name ACCEPTED = 201 UNAUTHORIZED = 401 FORBIDDEN = 403 diff --git a/resources/libraries/python/InterfaceUtil.py b/resources/libraries/python/InterfaceUtil.py index c0a8957935..aac78c9e44 100644 --- a/resources/libraries/python/InterfaceUtil.py +++ b/resources/libraries/python/InterfaceUtil.py @@ -316,7 +316,6 @@ class InterfaceUtil(object): :type node: dict :type pci_addr: str :type driver: str - :returns: None. :raises RuntimeError: If unbinding from the current driver fails. :raises RuntimeError: If binding to the new driver fails. """ @@ -397,9 +396,8 @@ class InterfaceUtil(object): return None if name == 'Driver:': return value if value else None - else: - raise RuntimeError('Get interface driver for: {0}' - .format(pci_addr)) + raise RuntimeError('Get interface driver for: {0}' + .format(pci_addr)) @staticmethod def tg_set_interfaces_udev_rules(node): diff --git a/resources/libraries/python/L2Util.py b/resources/libraries/python/L2Util.py index 7954193b60..53c3b99c83 100644 --- a/resources/libraries/python/L2Util.py +++ b/resources/libraries/python/L2Util.py @@ -132,7 +132,6 @@ class L2Util(object): :type sw_if_index: int :type bd_id: int :type shg: int - :returns: None """ VatExecutor.cmd_from_template(node, "l2_bd_add_sw_if_index.vat", bd_id=bd_id, sw_if_index=sw_if_index, diff --git a/resources/libraries/python/PacketVerifier.py b/resources/libraries/python/PacketVerifier.py index d1e7bbd229..978babf7ce 100644 --- a/resources/libraries/python/PacketVerifier.py +++ b/resources/libraries/python/PacketVerifier.py @@ -180,7 +180,6 @@ def packet_reader(interface_name, queue): :param queue: Queue in which this function will push incoming packets. :type interface_name: str :type queue: multiprocessing.Queue - :returns: None """ sock = conf.L2listen(iface=interface_name, type=ETH_P_ALL) diff --git a/resources/libraries/python/QemuUtils.py b/resources/libraries/python/QemuUtils.py index 99fb7f4b8d..5821455cc3 100644 --- a/resources/libraries/python/QemuUtils.py +++ b/resources/libraries/python/QemuUtils.py @@ -560,8 +560,8 @@ class QemuUtils(object): pid = '-pidfile {}'.format(self._pid_file) # Run QEMU - cmd = '{0} {1} {2} {3} {4} {5} {6} {7} {8} {9} {10}'.format(bin_path, - self._qemu_opt.get('smp'), mem, ssh_fwd, + cmd = '{0} {1} {2} {3} {4} {5} {6} {7} {8} {9} {10}'.format( + bin_path, self._qemu_opt.get('smp'), mem, ssh_fwd, self._qemu_opt.get('options'), drive, qmp, serial, qga, graphic, pid) try: diff --git a/resources/libraries/python/SFC/SFCTest.py b/resources/libraries/python/SFC/SFCTest.py index 2584632af9..d4f77baa52 100644 --- a/resources/libraries/python/SFC/SFCTest.py +++ b/resources/libraries/python/SFC/SFCTest.py @@ -43,7 +43,6 @@ class SFCTest(object): :type if1_adj_mac: str :type if2_adj_mac: str :type testtype: str - :returns: none :raises RuntimeError: If the script execute fails. """ diff --git a/resources/libraries/python/SFC/VerifyPacket.py b/resources/libraries/python/SFC/VerifyPacket.py index 90dfd375ca..a13c7601d5 100644 --- a/resources/libraries/python/SFC/VerifyPacket.py +++ b/resources/libraries/python/SFC/VerifyPacket.py @@ -66,7 +66,6 @@ class VerifyPacket(object): :param payload_data: the payload data in the packet. :type payload_data: str - :returns: none :raises RuntimeError: If the vxlan protocol field verify fails. """ # get the vxlan packet and check it @@ -87,7 +86,6 @@ class VerifyPacket(object): :param test_type: the functional test type. :type payload_data: str :type test_type: str - :returns: none :raises RuntimeError: If the vxlangpe and nsh protocol field verify fails. """ @@ -171,7 +169,6 @@ class VerifyPacket(object): :type ether: scapy.Ether :type frame_size: Integer :type test_type: str - :returns: none :raises RuntimeError: If the packet field verify fails. """ diff --git a/resources/libraries/python/TLDK/UdpTest.py b/resources/libraries/python/TLDK/UdpTest.py index 0571323dcf..b3f1b221e6 100644 --- a/resources/libraries/python/TLDK/UdpTest.py +++ b/resources/libraries/python/TLDK/UdpTest.py @@ -73,7 +73,6 @@ class UdpTest(object): :type file_prefix: str :type dest_ip: str :type is_ipv4: bool - :returns: none. :raises RuntimeError: If failed to execute udpfwd test on the dut node. """ pci_address = Topology.get_interface_pci_addr(dut_node, dut_if) diff --git a/resources/libraries/python/VatExecutor.py b/resources/libraries/python/VatExecutor.py index 41e97e3901..d27e0677d3 100644 --- a/resources/libraries/python/VatExecutor.py +++ b/resources/libraries/python/VatExecutor.py @@ -204,6 +204,7 @@ class VatTerminal(object): __LINUX_PROMPT = (":~$ ", "~]$ ", "~]# ") def __init__(self, node, json_param=True): + """TODO: Should we document this constructor can raise RuntimeError?""" json_text = ' json' if json_param else '' self.json = json_param self._node = node @@ -211,7 +212,7 @@ class VatTerminal(object): self._ssh.connect(self._node) try: self._tty = self._ssh.interactive_terminal_open() - except Exception: + except IOError: raise RuntimeError("Cannot open interactive terminal on node {0}". format(self._node)) @@ -221,7 +222,7 @@ class VatTerminal(object): self._tty, 'sudo -S {0}{1}'.format(Constants.VAT_BIN_NAME, json_text), self.__VAT_PROMPT) - except Exception: + except IOError: continue else: break @@ -253,9 +254,9 @@ class VatTerminal(object): """Execute command on the opened VAT terminal. :param cmd: Command to be executed. - :returns: Command output in python representation of JSON format or None if not in JSON mode. + :raise RuntimeError: If VAT command execution fails. """ VatHistory.add_to_vat_history(self._node, cmd) logger.debug("Executing command in VAT terminal: {0}".format(cmd)) @@ -263,7 +264,7 @@ class VatTerminal(object): out = self._ssh.interactive_terminal_exec_command(self._tty, cmd, self.__VAT_PROMPT) self.vat_stdout = out - except Exception: + except IOError: self._exec_failure = True vpp_pid = get_vpp_pid(self._node) if vpp_pid: diff --git a/resources/libraries/python/ssh.py b/resources/libraries/python/ssh.py index 8c064e2729..fe4404b053 100644 --- a/resources/libraries/python/ssh.py +++ b/resources/libraries/python/ssh.py @@ -92,11 +92,11 @@ class SSH(object): format(self._ssh.get_transport().getpeername())) logger.debug('Connections: {0}'. format(str(SSH.__existing_connections))) - except: + except RuntimeError as exc: if attempts > 0: self._reconnect(attempts-1) else: - raise + raise exc def disconnect(self, node): """Close SSH connection to the node. @@ -236,8 +236,8 @@ class SSH(object): def interactive_terminal_open(self, time_out=30): """Open interactive terminal on a new channel on the connected Node. - :param time_out: Timeout in seconds. - :returns: SSH channel with opened terminal. + FIXME: Convert or document other possible exceptions, such as + socket.error or SSHException. .. warning:: Interruptingcow is used here, and it uses signal(SIGALRM) to let the operating system interrupt program @@ -245,6 +245,10 @@ class SSH(object): handlers only apply to the main thread, so you cannot use this from other threads. You must not use this in a program that uses SIGALRM itself (this includes certain profilers) + + :param time_out: Timeout in seconds. + :returns: SSH channel with opened terminal. + :raise IOError: If receive attempt results in socket.timeout. """ chan = self._ssh.get_transport().open_session() chan.get_pty() @@ -264,7 +268,8 @@ class SSH(object): break except socket.timeout: logger.error('Socket timeout: {0}'.format(buf)) - raise Exception('Socket timeout: {0}'.format(buf)) + # TODO: Find out which exception would callers appreciate here. + raise IOError('Socket timeout: {0}'.format(buf)) return chan def interactive_terminal_exec_command(self, chan, cmd, prompt): @@ -272,18 +277,19 @@ class SSH(object): interactive_terminal_open() method has to be called first! - :param chan: SSH channel with opened terminal. - :param cmd: Command to be executed. - :param prompt: Command prompt, sequence of characters used to - indicate readiness to accept commands. - :returns: Command output. - .. warning:: Interruptingcow is used here, and it uses signal(SIGALRM) to let the operating system interrupt program execution. This has the following limitations: Python signal handlers only apply to the main thread, so you cannot use this from other threads. You must not use this in a program that uses SIGALRM itself (this includes certain profilers) + + :param chan: SSH channel with opened terminal. + :param cmd: Command to be executed. + :param prompt: Command prompt, sequence of characters used to + indicate readiness to accept commands. + :returns: Command output. + :raise IOError: If receive attempt results in socket.timeout. """ chan.sendall('{c}\n'.format(c=cmd)) buf = '' @@ -299,8 +305,9 @@ class SSH(object): except socket.timeout: logger.error('Socket timeout during execution of command: ' '{0}\nBuffer content:\n{1}'.format(cmd, buf)) - raise Exception('Socket timeout during execution of command: ' - '{0}\nBuffer content:\n{1}'.format(cmd, buf)) + # TODO: Find out which exception would callers appreciate here. + raise IOError('Socket timeout during execution of command: ' + '{0}\nBuffer content:\n{1}'.format(cmd, buf)) tmp = buf.replace(cmd.replace('\n', ''), '') for item in prompt: tmp.replace(item, '') @@ -353,6 +360,7 @@ class SSH(object): def exec_cmd(node, cmd, timeout=600, sudo=False): """Convenience function to ssh/exec/return rc, out & err. + FIXME: Document :param, :type, :raise and similar. Returns (rc, stdout, stderr). """ if node is None: diff --git a/resources/tools/topology/update_topology.py b/resources/tools/topology/update_topology.py index f60fdf1653..2e267639ef 100755 --- a/resources/tools/topology/update_topology.py +++ b/resources/tools/topology/update_topology.py @@ -79,7 +79,6 @@ def update_mac_addresses_for_node(node): :param node: Node from topology. :type node: dict - :return: None """ for port_name, port in node['interfaces'].items(): if 'driver' not in port: @@ -128,7 +127,6 @@ def update_nodes_mac_addresses(topology): :param topology: Topology information with nodes. :type topology: dict - :return: None """ for node in topology['nodes'].values(): update_mac_addresses_for_node(node) diff --git a/resources/traffic_scripts/honeycomb/read_vpp_version.py b/resources/traffic_scripts/honeycomb/read_vpp_version.py index 8a861801d6..70ba733ca1 100755 --- a/resources/traffic_scripts/honeycomb/read_vpp_version.py +++ b/resources/traffic_scripts/honeycomb/read_vpp_version.py @@ -252,7 +252,6 @@ class ConfigBlaster(object): :param function: Function to be executed in each thread. :type function: function - :return: None """ self.total_ok_rqsts = 0 -- 2.16.6