aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVratko Polak <vrpolak@cisco.com>2019-12-04 13:24:07 +0100
committerPeter Mikus <pmikus@cisco.com>2019-12-05 07:03:57 +0000
commit063abf35e81deaf749ebbcfee339fbd1d9e89412 (patch)
treec932eac04f162c46b0f3c38db10105b945a617cb
parent6221f1b96d2a167c6db74ff26cd7ec7906ae9486 (diff)
Deal with some "pylint: disable=" comments
+ When possible, fix the violation. + Else, add a comment: + An explanation (if not already present) and keep disable. + A TODO (if not already present) and remove the disable. - This makes tox job report more pylint violations, but any such violation is fixable and should be fixed. - Although some need to be fixed in VPP, such as enum item long names. Change-Id: I48604b5eda070083d79dff1439620dbd9e798e1f Signed-off-by: Vratko Polak <vrpolak@cisco.com>
-rw-r--r--pylint.cfg3
-rw-r--r--resources/libraries/python/ContainerUtils.py21
-rw-r--r--resources/libraries/python/GBP.py5
-rw-r--r--resources/libraries/python/IPUtil.py3
-rw-r--r--resources/libraries/python/OptionString.py21
-rw-r--r--resources/libraries/python/PacketVerifier.py7
-rw-r--r--resources/libraries/python/PapiExecutor.py5
-rw-r--r--resources/libraries/python/Policer.py6
-rw-r--r--resources/libraries/python/TrafficGenerator.py6
-rw-r--r--resources/libraries/python/VatExecutor.py4
-rw-r--r--resources/libraries/python/topology.py6
-rwxr-xr-xresources/traffic_scripts/ipsec.py6
-rwxr-xr-xresources/traffic_scripts/policer.py7
13 files changed, 62 insertions, 38 deletions
diff --git a/pylint.cfg b/pylint.cfg
index 5ae4ed551b..82b3a06215 100644
--- a/pylint.cfg
+++ b/pylint.cfg
@@ -42,8 +42,9 @@ extension-pkg-whitelist=numpy, scipy
# --enable=similarities". If you want to run only the classes checker, but have
# no Warning level messages displayed, use"--disable=all --enable=classes
# --disable=W"
-disable=redefined-variable-type, locally-disabled, locally-enabled
+#disable=redefined-variable-type, locally-disabled, locally-enabled
+# TODO: Add explanation when disabling an id, either locally or globally.
[REPORTS]
diff --git a/resources/libraries/python/ContainerUtils.py b/resources/libraries/python/ContainerUtils.py
index fc61eea3bd..74add98359 100644
--- a/resources/libraries/python/ContainerUtils.py
+++ b/resources/libraries/python/ContainerUtils.py
@@ -11,15 +11,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-# Bug workaround in pylint for abstract classes.
-# pylint: disable=W0223
-
"""Library to manipulate Containers."""
from collections import OrderedDict, Counter
from io import open
from string import Template
+from robot.libraries.BuiltIn import BuiltIn
+
from resources.libraries.python.Constants import Constants
from resources.libraries.python.ssh import SSH
from resources.libraries.python.topology import Topology, SocketType
@@ -442,8 +441,6 @@ class ContainerEngine:
)
self.execute(u"supervisorctl start vpp")
- # pylint: disable=import-outside-toplevel
- from robot.libraries.BuiltIn import BuiltIn
topo_instance = BuiltIn().get_library_instance(
u"resources.libraries.python.topology.Topology"
)
@@ -652,6 +649,13 @@ class LXC(ContainerEngine):
self._configure_cgroup(u"lxc")
+ def build(self):
+ """Build container (compile).
+
+ TODO: Remove from parent class if no sibling implements this.
+ """
+ raise NotImplementedError
+
def create(self):
"""Create/deploy an application inside a container on system.
@@ -870,6 +874,13 @@ class Docker(ContainerEngine):
if self.container.cpuset_cpus:
self._configure_cgroup(u"docker")
+ def build(self):
+ """Build container (compile).
+
+ TODO: Remove from parent class if no sibling implements this.
+ """
+ raise NotImplementedError
+
def create(self):
"""Create/deploy container.
diff --git a/resources/libraries/python/GBP.py b/resources/libraries/python/GBP.py
index 3d4d249b10..59dd269d95 100644
--- a/resources/libraries/python/GBP.py
+++ b/resources/libraries/python/GBP.py
@@ -43,8 +43,9 @@ class GBPBridgeDomainFlags(IntEnum):
class GBPSubnetType(IntEnum):
"""GBP Subnet Type."""
GBP_API_SUBNET_TRANSPORT = 1
- GBP_API_SUBNET_STITCHED_INTERNAL = 2 # pylint: disable=invalid-name
- GBP_API_SUBNET_STITCHED_EXTERNAL = 3 # pylint: disable=invalid-name
+ # TODO: Names too long for pylint, fix in VPP.
+ GBP_API_SUBNET_STITCHED_INTERNAL = 2
+ GBP_API_SUBNET_STITCHED_EXTERNAL = 3
GBP_API_SUBNET_L3_OUT = 4
GBP_API_SUBNET_ANON_L3_OUT = 5
diff --git a/resources/libraries/python/IPUtil.py b/resources/libraries/python/IPUtil.py
index f99deb1e08..04961fa45a 100644
--- a/resources/libraries/python/IPUtil.py
+++ b/resources/libraries/python/IPUtil.py
@@ -56,7 +56,8 @@ class FibPathType(IntEnum):
class FibPathFlags(IntEnum):
"""FIB path flags."""
FIB_PATH_FLAG_NONE = 0
- FIB_PATH_FLAG_RESOLVE_VIA_ATTACHED = 1 # pylint: disable=invalid-name
+ # TODO: Name too long for pylint, fix in VPP.
+ FIB_PATH_FLAG_RESOLVE_VIA_ATTACHED = 1
FIB_PATH_FLAG_RESOLVE_VIA_HOST = 2
diff --git a/resources/libraries/python/OptionString.py b/resources/libraries/python/OptionString.py
index 9a30d37b9a..bdb5ee2b4c 100644
--- a/resources/libraries/python/OptionString.py
+++ b/resources/libraries/python/OptionString.py
@@ -92,12 +92,17 @@ class OptionString:
self.parts.extend(other.parts)
return self
- def _check_and_add(self, part, prefixed):
+ def check_and_add(self, part, prefixed):
"""Convert to string, strip, conditionally add prefixed if non-empty.
Value of None is converted to empty string.
Emptiness is tested before adding prefix.
+ This could be a protected method (name starting with underscore),
+ but then pylint does not understand add_equals and add_with_value
+ are allowed to call this on the temp instance.
+ TODO: Is there a way to make pylint understand?
+
:param part: Unchecked part to add to list of parts.
:param prefixed: Whether to add prefix when adding.
:type part: object
@@ -123,7 +128,7 @@ class OptionString:
:returns: Self, to enable method chaining.
:rtype: OptionString
"""
- self._check_and_add(parameter, prefixed=True)
+ self.check_and_add(parameter, prefixed=True)
return self
def add_if(self, parameter, condition):
@@ -160,11 +165,8 @@ class OptionString:
:rtype: OptionString
"""
temp = OptionString(prefix=self.prefix)
- # TODO: Is pylint really that ignorant?
- # How could it not understand temp is of type of this class?
- # pylint: disable=protected-access
- if temp._check_and_add(parameter, prefixed=True):
- if temp._check_and_add(value, prefixed=False):
+ if temp.check_and_add(parameter, prefixed=True):
+ if temp.check_and_add(value, prefixed=False):
self.extend(temp)
return self
@@ -183,9 +185,8 @@ class OptionString:
:rtype: OptionString
"""
temp = OptionString(prefix=self.prefix)
- # pylint: disable=protected-access
- if temp._check_and_add(parameter, prefixed=True):
- if temp._check_and_add(value, prefixed=False):
+ if temp.check_and_add(parameter, prefixed=True):
+ if temp.check_and_add(value, prefixed=False):
self.parts.append(u"=".join(temp.parts))
return self
diff --git a/resources/libraries/python/PacketVerifier.py b/resources/libraries/python/PacketVerifier.py
index 397ce76f49..fb2337e49d 100644
--- a/resources/libraries/python/PacketVerifier.py
+++ b/resources/libraries/python/PacketVerifier.py
@@ -75,7 +75,6 @@ from scapy.packet import Raw
# Enable libpcap's L2listen
conf.use_pcap = True
-import scapy.arch.pcapdnet # pylint: disable=C0413, unused-import
__all__ = [
u"RxQueue", u"TxQueue", u"Interface", u"create_gratuitous_arp_request",
@@ -235,7 +234,11 @@ class RxQueue(PacketVerifier):
pkt_pad = str(auto_pad(pkt))
print(f"Received packet on {self._ifname} of len {len(pkt)}")
if verbose:
- pkt.show2() # pylint: disable=no-member
+ if hasattr(pkt, u"show2"):
+ pkt.show2()
+ else:
+ # Never happens in practice, but Pylint does not know that.
+ print(f"Unexpected instance: {pkt!r}")
print()
if pkt_pad in ignore_list:
ignore_list.remove(pkt_pad)
diff --git a/resources/libraries/python/PapiExecutor.py b/resources/libraries/python/PapiExecutor.py
index 7f226f4ff3..5a79a60903 100644
--- a/resources/libraries/python/PapiExecutor.py
+++ b/resources/libraries/python/PapiExecutor.py
@@ -196,6 +196,11 @@ class PapiSocketExecutor:
# Package path has to be one level above the vpp_papi directory.
package_path = package_path.rsplit(u"/", 1)[0]
sys.path.append(package_path)
+ # Only now, interpreter has a chance to locate the code to import.
+ # That means the import statement here is in the correct place.
+ # No refactor allows the import to be moved to where pylint wants,
+ # and pylint does not execute the logic for locating the code,
+ # so, dear pylint, please ignore these offences.
# pylint: disable=import-outside-toplevel, import-error
from vpp_papi.vpp_papi import VPPApiClient as vpp_class
vpp_class.apidir = api_json_directory
diff --git a/resources/libraries/python/Policer.py b/resources/libraries/python/Policer.py
index f6a852e417..d5500e1d89 100644
--- a/resources/libraries/python/Policer.py
+++ b/resources/libraries/python/Policer.py
@@ -84,7 +84,11 @@ class DSCP(IntEnum):
class Policer:
"""Policer utilities."""
- # pylint: disable=too-many-arguments, too-many-locals
+ # TODO: Pylint says too-many-arguments and too-many-locals.
+ # It is right, we should refactor the code
+ # and group similar arguments together (into documented classes).
+ # Note that even the call from Robot Framework
+ # is not very readable with this many arguments.
@staticmethod
def policer_set_configuration(
node, policer_name, cir, eir, cbs, ebs, rate_type, round_type,
diff --git a/resources/libraries/python/TrafficGenerator.py b/resources/libraries/python/TrafficGenerator.py
index 14d2dc8d1c..007079f254 100644
--- a/resources/libraries/python/TrafficGenerator.py
+++ b/resources/libraries/python/TrafficGenerator.py
@@ -125,7 +125,8 @@ class TGDropRateSearchImpl(DropRateSearch):
return tg_instance.get_latency_int()
-# pylint: disable=too-many-instance-attributes
+# TODO: Pylint says too-many-instance-attributes.
+# A fix is developed in https://gerrit.fd.io/r/c/csit/+/22221
class TrafficGenerator(AbstractMeasurer):
"""Traffic Generator.
@@ -201,7 +202,8 @@ class TrafficGenerator(AbstractMeasurer):
"""
return self._latency
- # pylint: disable=too-many-locals
+ # TODO: pylint says disable=too-many-locals.
+ # A fix is developed in https://gerrit.fd.io/r/c/csit/+/22221
def initialize_traffic_generator(
self, tg_node, tg_if1, tg_if2, tg_if1_adj_node, tg_if1_adj_if,
tg_if2_adj_node, tg_if2_adj_if, osi_layer, tg_if1_dst_mac=None,
diff --git a/resources/libraries/python/VatExecutor.py b/resources/libraries/python/VatExecutor.py
index be8dbbe9dc..19b92a6a1a 100644
--- a/resources/libraries/python/VatExecutor.py
+++ b/resources/libraries/python/VatExecutor.py
@@ -20,6 +20,8 @@ from os import remove
from paramiko.ssh_exception import SSHException
from robot.api import logger
+import resources.libraries.python.DUTSetup as PidLib
+
from resources.libraries.python.Constants import Constants
from resources.libraries.python.PapiHistory import PapiHistory
from resources.libraries.python.ssh import SSH, SSHTimeout
@@ -60,8 +62,6 @@ def get_vpp_pid(node):
running on the DUT node.
:rtype: int or list
"""
- # pylint: disable=import-outside-toplevel
- import resources.libraries.python.DUTSetup as PidLib
pid = PidLib.DUTSetup.get_vpp_pid(node)
return pid
diff --git a/resources/libraries/python/topology.py b/resources/libraries/python/topology.py
index 46a6628a0a..92ade4a7a3 100644
--- a/resources/libraries/python/topology.py
+++ b/resources/libraries/python/topology.py
@@ -41,15 +41,19 @@ def load_topo_from_yaml():
return safe_load(work_file.read())[u"nodes"]
-# pylint: disable=invalid-name
class NodeType:
"""Defines node types used in topology dictionaries."""
+ # TODO: Two letter initialisms are well-known, but too short for pylint.
+ # Candidates: TG -> TGN, VM -> VNF.
+
# Device Under Test (this node has VPP running on it)
DUT = u"DUT"
# Traffic Generator (this node has traffic generator on it)
+ # pylint: disable=invalid-name
TG = u"TG"
# Virtual Machine (this node running on DUT node)
+ # pylint: disable=invalid-name
VM = u"VM"
diff --git a/resources/traffic_scripts/ipsec.py b/resources/traffic_scripts/ipsec.py
index 320303d1fb..9992a7c8cd 100755
--- a/resources/traffic_scripts/ipsec.py
+++ b/resources/traffic_scripts/ipsec.py
@@ -19,9 +19,6 @@ import sys
import logging
from ipaddress import ip_address
-# pylint: disable=no-name-in-module
-# pylint: disable=import-error
-logging.getLogger(u"scapy.runtime").setLevel(logging.ERROR)
from scapy.layers.inet import IP
from scapy.layers.inet6 import IPv6, ICMPv6ND_NS
from scapy.layers.ipsec import SecurityAssociation, ESP
@@ -122,8 +119,7 @@ def check_ip(pkt_recv, ip_layer, src_ip, dst_ip):
)
-# pylint: disable=too-many-locals
-# pylint: disable=too-many-statements
+# TODO: Pylint says too-many-locals and too-many-statements. Refactor!
def main():
"""Send and receive IPsec packet."""
diff --git a/resources/traffic_scripts/policer.py b/resources/traffic_scripts/policer.py
index c222c76152..97cad612ac 100755
--- a/resources/traffic_scripts/policer.py
+++ b/resources/traffic_scripts/policer.py
@@ -19,10 +19,6 @@ import sys
import logging
from ipaddress import ip_address
-# pylint: disable=no-name-in-module
-# pylint: disable=import-error
-logging.getLogger("scapy.runtime").setLevel(logging.ERROR)
-
from scapy.layers.l2 import Ether
from scapy.layers.inet import IP, TCP
from scapy.layers.inet6 import IPv6, ICMPv6ND_NS
@@ -72,8 +68,7 @@ def check_ipv6(pkt_recv, dscp):
raise RuntimeError(f"Not a TCP packet received: {pkt_recv!r}")
-# pylint: disable=too-many-locals
-# pylint: disable=too-many-statements
+# TODO: Pylint says too-many-locals and too-many-statements. Refactor!
def main():
"""Send and receive TCP packet."""
args = TrafficScriptArg(