Bumped pylint requirement to 2.14.5 and cleaned up issues

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

changelog/136.trivial.rst

@@ -2,3 +2,4 @@ CI and internal changes:
 
 * Lock system tests to a version of nox that still works
 * Bump python version to 3.9 for lint workflow
+* Bumped pylint requirement to `2.14.5` and cleaned up issues

noxfile.py

@@ -8,10 +8,9 @@
 import tarfile
 import tempfile
 
-import nox
-from nox.command import CommandFailed
-from nox.logger import logger
-
+import nox  # pylint: disable=import-error
+from nox.command import CommandFailed  # pylint: disable=import-error
+from nox.logger import logger  # pylint: disable=import-error
 
 COVERAGE_VERSION_REQUIREMENT = "coverage==5.5"
 SALT_REQUIREMENT = os.environ.get("SALT_REQUIREMENT") or "salt>=3004"
@@ -73,7 +72,8 @@ def pytest_version(session):
             session,
             "python",
             "-c",
-            'import sys, pkg_resources; sys.stdout.write("{}".format(pkg_resources.get_distribution("pytest").version))',
+            'import sys, pkg_resources; sys.stdout.write("{}".format('
+            'pkg_resources.get_distribution("pytest").version))',
             silent=True,
             log=False,
         )
@@ -261,18 +261,29 @@ def tests(session):
             shutil.copyfile(str(COVERAGE_REPORT_DB), str(ARTIFACTS_DIR / ".coverage"))
 
 
-def _lint(session, rcfile, flags, paths):
+def _lint(session, rcfile, extra_args, paths):
     if SKIP_REQUIREMENTS_INSTALL is False:
+        salt_requirements = []
+        if "3003" in SALT_REQUIREMENT:
+            salt_requirements.append("jinja2<3.1")
+        salt_requirements.append(SALT_REQUIREMENT)
         session.install(
             "--progress-bar=off",
             "-r",
             os.path.join("requirements", "lint.txt"),
+            *salt_requirements,
+            silent=PIP_INSTALL_SILENT,
+        )
+        session.install(
+            "--progress-bar=off",
+            "-e",
+            ".",
             silent=PIP_INSTALL_SILENT,
         )
         session.run("pylint", "--version")
     pylint_report_path = os.environ.get("PYLINT_REPORT")
 
-    cmd_args = ["pylint", "--rcfile={}".format(rcfile)] + list(flags) + list(paths)
+    cmd_args = ["pylint", "--rcfile={}".format(rcfile)] + extra_args + paths
 
     stdout = tempfile.TemporaryFile(mode="w+b")
     try:
@@ -286,7 +297,7 @@ def _lint(session, rcfile, flags, paths):
             sys.stdout.flush()
             if pylint_report_path:
                 # Write report
-                with open(pylint_report_path, "w") as wfh:
+                with open(pylint_report_path, "w", encoding="utf=8") as wfh:
                     wfh.write(contents)
                 session.log("Report file written to %r", pylint_report_path)
         stdout.close()
@@ -306,25 +317,35 @@ def lint_code(session):
     """
     Run PyLint against the code. Set PYLINT_REPORT to a path to capture output.
     """
-    flags = ["--disable=I"]
+    extra_args = ["--disable=I"]
     if session.posargs:
         paths = session.posargs
     else:
         paths = ["setup.py", "noxfile.py", "src/saltfactories/"]
-    _lint(session, ".pylintrc", flags, paths)
+    _lint(session, ".pylintrc", extra_args, paths)
 
 
 @nox.session(python="3", name="lint-tests")
 def lint_tests(session):
     """
     Run PyLint against Salt and it's test suite. Set PYLINT_REPORT to a path to capture output.
     """
-    flags = ["--disable=I"]
+    flags = [
+        "I",
+        "redefined-outer-name",
+        "missing-function-docstring",
+        "missing-class-docstring",
+        "unused-argument",
+        "disallowed-name",
+    ]
+    extra_args = [
+        "--disable={}".format(",".join(flags)),
+    ]
     if session.posargs:
         paths = session.posargs
     else:
         paths = ["tests/"]
-    _lint(session, ".pylintrc", flags, paths)
+    _lint(session, ".pylintrc", extra_args, paths)
 
 
 @nox.session(python="3")
@@ -346,7 +367,7 @@ def docs(session):
     session.run("make", "coverage", "SPHINXOPTS=-W", external=True)
     docs_coverage_file = os.path.join("_build", "html", "python.txt")
     if os.path.exists(docs_coverage_file):
-        with open(docs_coverage_file) as rfh:
+        with open(docs_coverage_file, encoding="utf=8") as rfh:
             contents = rfh.readlines()[2:]
             if contents:
                 session.error("\n" + "".join(contents))
@@ -550,7 +571,7 @@ def recompress(self, targz):
         with open(d_tar, "rb") as rfh:
             with gzip.GzipFile(
                 fileobj=open(d_targz, "wb"), mode="wb", filename="", mtime=self.mtime
-            ) as gz:
+            ) as gz:  # pylint: disable=invalid-name
                 while True:
                     chunk = rfh.read(1024)
                     if not chunk:

requirements/lint.txt

@@ -1,7 +1,6 @@
 -r base.txt
 -r tests.txt
-pylint==2.7.4
-saltpylint==2020.9.28
+pylint==2.14.5
 pyenchant
 black; python_version >= '3.7'
 reorder-python-imports; python_version >= '3.7'

requirements/tests.txt

@@ -1,5 +1,5 @@
 -r base.txt
-docker
+docker>=4.0.0
 pytest-subtests
 pyfakefs==4.4.0; python_version == '3.5'
 pyfakefs; python_version > '3.5'

src/saltfactories/bases.py

@@ -52,7 +52,7 @@ class SaltMixin:
         salt system paths apply.
     """
 
-    id = attr.ib(default=None, init=False)
+    id = attr.ib(default=None, init=False)  # pylint: disable=invalid-name
     config = attr.ib(repr=False)
     config_dir = attr.ib(init=False, default=None)
     config_file = attr.ib(init=False, default=None)
@@ -290,8 +290,7 @@ def cmdline(
         cmdline.extend(args)
 
         # Keyword arguments get passed as KEY=VALUE pairs to the CLI
-        for key in kwargs:
-            value = kwargs[key]
+        for key, value in kwargs.items():
             if not isinstance(value, str):
                 value = json.dumps(value)
             cmdline.append("{}={}".format(key, value))

src/saltfactories/cli/cloud.py

@@ -65,12 +65,12 @@ def default_config(root_dir, master_id, defaults=None, overrides=None):
     @classmethod
     def configure(
         cls,
-        factories_manager,
-        daemon_id,
+        factories_manager,  # pylint: disable=unused-argument
+        daemon_id,  # pylint: disable=unused-argument
         root_dir=None,
         defaults=None,
         overrides=None,
-        **configure_kwargs
+        **configure_kwargs  # pylint: disable=unused-argument
     ):
         """
         Configure the CLI.

src/saltfactories/client.py

@@ -61,7 +61,7 @@ def run(self, function, *args, minion_tgt="minion", timeout=300, **kwargs):
                 "WARNING(SHOULD NOT HAPPEN #1935): Failed to get a reply "
                 "from the minion '{}'. Command output: {}".format(minion_tgt, ret)
             )
-        elif ret[minion_tgt] is None and function not in self.__functions_known_to_return_none:
+        elif ret[minion_tgt] is None and function not in self.functions_known_to_return_none:
             pytest.fail(
                 "WARNING(SHOULD NOT HAPPEN #1935): Failed to get '{}' from "
                 "the minion '{}'. Command output: {}".format(function, minion_tgt, ret)

src/saltfactories/daemons/container.py

@@ -185,8 +185,8 @@ def __attrs_post_init__(self):
             self.check_ports = check_ports
 
         if self.container_run_kwargs and "ports" in self.container_run_kwargs:
-            ports = self.container_run_kwargs["ports"]
-            for container_binding, host_binding in ports.items():
+            _ports = self.container_run_kwargs["ports"]
+            for container_binding, host_binding in _ports.items():
                 port, proto = container_binding.split("/")
                 if proto != "tcp":
                     continue
@@ -315,8 +315,8 @@ def get_display_name(self):
         Returns a human readable name for the factory.
         """
         if self.display_name is None:
-            self.display_name = "{}(id={!r})".format(self.__class__.__name__, self.id)
-        return super().get_display_name()
+            self.display_name = "{}(id={!r})".format(self.__class__.__name__, self.name)
+        return self.display_name
 
     def start(self, *command, max_start_attempts=None, start_timeout=None):
         """
@@ -513,18 +513,18 @@ def get_check_ports(self):
         """
         Return a list of TCP ports to check against to ensure the daemon is running.
         """
-        ports = self.check_ports.copy()
+        _ports = self.check_ports.copy()
         if self.container:
             self.container.reload()
-            for container_binding, host_binding in ports.items():
+            for container_binding, host_binding in _ports.items():
                 if isinstance(host_binding, int):
                     continue
                 host_binding = self.get_host_port_binding(
                     container_binding, protocol="tcp", ipv6=False
                 )
                 if host_binding:
-                    ports[container_binding] = host_binding
-        return ports
+                    _ports[container_binding] = host_binding
+        return _ports
 
     def get_host_port_binding(self, port, protocol="tcp", ipv6=False):
         """
@@ -541,14 +541,14 @@ def get_host_port_binding(self, port, protocol="tcp", ipv6=False):
         """
         if self.container is None:
             return None
-        ports = self.container.ports
-        log.debug("Container Ports for %s: %s", self, ports)
-        if not ports:
+        _ports = self.container.ports
+        log.debug("Container Ports for %s: %s", self, _ports)
+        if not _ports:
             return None
         container_binding = "{}/{}".format(port, protocol)
-        if container_binding not in ports:
+        if container_binding not in _ports:
             return None
-        host_port_bindings = ports[container_binding]
+        host_port_bindings = ports[container_binding]  # pylint: disable=unsubscriptable-object
         if not host_port_bindings:
             # This means the host is using the same port as the container
             return int(port)
@@ -609,7 +609,11 @@ def client_connectable(docker_client):
         except (APIError, RequestsConnectionError, PyWinTypesError) as exc:
             return "The docker client failed to ping the docker server: {}".format(exc)
 
-    def run_container_start_checks(self, started_at, timeout_at):
+    def run_container_start_checks(
+        self,
+        started_at,  # pylint: disable=unused-argument
+        timeout_at,
+    ):
         """
         Run startup checks.
         """
@@ -743,11 +747,13 @@ class SaltDaemon(Container, bases.SaltDaemon):
     Salt Daemon inside a container implementation.
     """
 
+    _daemon_started = attr.ib(init=False, repr=False, default=False)
+    _daemon_starting = attr.ib(init=False, repr=False, default=False)
+
     def __attrs_post_init__(self):
         """
         Post attrs initialization routines.
         """
-        self.daemon_started = self.daemon_starting = False
         # Default to whatever is the default python in the container
         # and not the python_executable set by salt-factories
         self.python_executable = "python"
@@ -773,6 +779,12 @@ def __attrs_post_init__(self):
         self.container_run_kwargs.setdefault("auto_remove", True)
         log.debug("%s container_run_kwargs: %s", self, self.container_run_kwargs)
 
+    def get_display_name(self):
+        """
+        Returns a human readable name for the factory.
+        """
+        return bases.SaltDaemon.get_display_name(self)
+
     def run(self, *cmd, **kwargs):
         """
         Run a command inside the container.
@@ -799,21 +811,21 @@ def start(self, *extra_cli_arguments, max_start_attempts=None, start_timeout=Non
         )
         if not running:
             return running
-        self.daemon_starting = True
+        self._daemon_starting = True
         # Now that the container is up, let's start the daemon
-        self.daemon_started = bases.SaltDaemon.start(
+        self._daemon_started = bases.SaltDaemon.start(
             self,
             *extra_cli_arguments,
             max_start_attempts=max_start_attempts,
             start_timeout=start_timeout,
         )
-        return self.daemon_started
+        return self._daemon_started
 
     def terminate(self):
         """
         Terminate the container.
         """
-        self.daemon_started = self.daemon_starting = False
+        self._daemon_started = self._daemon_starting = False
         ret = bases.SaltDaemon.terminate(self)
         Container.terminate(self)
         return ret
@@ -825,17 +837,17 @@ def is_running(self):
         running = Container.is_running(self)
         if running is False:
             return running
-        if self.daemon_starting or self.daemon_started:
+        if self._daemon_starting or self._daemon_started:
             return bases.SaltDaemon.is_running(self)
         return running
 
     def get_check_ports(self):
         """
         Return a list of ports to check against to ensure the daemon is running.
         """
-        ports = {port: port for port in bases.SaltDaemon.get_check_ports(self)}
-        ports.update(Container.get_check_ports(self))
-        return ports
+        _ports = {port: port for port in bases.SaltDaemon.get_check_ports(self)}
+        _ports.update(Container.get_check_ports(self))
+        return _ports
 
     def before_start(
         self, callback, *args, on_container=False, **kwargs
@@ -956,6 +968,12 @@ class SaltMinion(SaltDaemon, minion.SaltMinion):
     Salt minion daemon implementation running in a docker container.
     """
 
+    def get_display_name(self):
+        """
+        Returns a human readable name for the factory.
+        """
+        return minion.SaltMinion.get_display_name(self)
+
     def get_check_events(self):
         """
         Return salt events to check.

src/saltfactories/daemons/master.py

@@ -51,12 +51,12 @@ def __attrs_post_init__(self):
             self.after_terminate(self.event_listener.unregister_auth_event_handler, self.id)
 
     @state_tree.default
-    def __setup_state_tree(self):
+    def __setup_state_tree(self):  # pylint: disable=unused-private-member
         if "file_roots" in self.config:
             return SaltStateTree(envs=copy.deepcopy(self.config.get("file_roots") or {}))
 
     @pillar_tree.default
-    def __setup_pillar_tree(self):
+    def __setup_pillar_tree(self):  # pylint: disable=unused-private-member
         if "pillar_roots" in self.config:
             return SaltPillarTree(envs=copy.deepcopy(self.config.get("pillar_roots") or {}))
 
@@ -229,10 +229,10 @@ def _configure(  # pylint: disable=arguments-differ
         cls,
         factories_manager,
         daemon_id,
-        master_of_masters=None,
         root_dir=None,
         defaults=None,
         overrides=None,
+        master_of_masters=None,
         order_masters=False,
     ):
         return cls.default_config(

src/saltfactories/daemons/minion.py

@@ -32,12 +32,12 @@ class SaltMinion(SaltDaemon):
     pillar_tree = attr.ib(init=False, hash=False, repr=False)
 
     @state_tree.default
-    def __setup_state_tree(self):
+    def __setup_state_tree(self):  # pylint: disable=unused-private-member
         if "file_roots" in self.config:
             return SaltStateTree(envs=copy.deepcopy(self.config.get("file_roots") or {}))
 
     @pillar_tree.default
-    def __setup_pillar_tree(self):
+    def __setup_pillar_tree(self):  # pylint: disable=unused-private-member
         if "pillar_roots" in self.config:
             return SaltPillarTree(envs=copy.deepcopy(self.config.get("pillar_roots") or {}))