Python code modernization, 4/n (#1162)

* Address attribute-defined-outside-init.

* Address broad-exception-raised.

* Address broad-exception-caught.

* Address consider-iterating-dictionary.

* Address consider-using-dict-comprehension.

* Address consider-using-f-string.

* Address consider-using-in.

* Address consider-using-max-builtin.

* Address some consider-using-with.

* Address invalid-name.

* Address keyword-arg-before-vararg.

* Address line-too-long.

* Address no-else-continue.

* Address no-else-raise.

* Address no-else-return.

* Remove broken dead code.

* Make consider-using-f-string changes compatible with older Python versions.

* Python 3.11 and earlier apparently do not like multi-line f-strings.

plugins/module_utils/_api/_import_helper.py

@@ -70,14 +70,16 @@ except ImportError:
                 self.connection = self
                 self.connectionpool = self
 
-                self.RecentlyUsedContainer = object()
-                self.PoolManager = object()
+                self.RecentlyUsedContainer = object()  # pylint: disable=invalid-name
+                self.PoolManager = object()  # pylint: disable=invalid-name
                 self.match_hostname = object()
-                self.HTTPConnectionPool = _HTTPConnectionPool
+                self.HTTPConnectionPool = (  # pylint: disable=invalid-name
+                    _HTTPConnectionPool
+                )
 
         class FakeURLLIB3Connection:
             def __init__(self):
-                self.HTTPConnection = _HTTPConnection
+                self.HTTPConnection = _HTTPConnection  # pylint: disable=invalid-name
 
         urllib3 = FakeURLLIB3()
         urllib3_connection = FakeURLLIB3Connection()

plugins/module_utils/_api/api/client.py

@@ -47,7 +47,7 @@ from ..transport.sshconn import PARAMIKO_IMPORT_ERROR, SSHHTTPAdapter
 from ..transport.ssladapter import SSLHTTPAdapter
 from ..transport.unixconn import UnixHTTPAdapter
 from ..utils import config, json_stream, utils
-from ..utils.decorators import check_resource, update_headers
+from ..utils.decorators import update_headers
 from ..utils.proxy import ProxyConfig
 from ..utils.socket import consume_socket_output, demux_adaptor, frames_iter
 from .daemon import DaemonApiMixin
@@ -278,8 +278,7 @@ class APIClient(_Session, DaemonApiMixin):
 
         if kwargs.get("versioned_api", True):
             return f"{self.base_url}/v{self._version}{pathfmt.format(*args)}"
-        else:
-            return f"{self.base_url}{pathfmt.format(*args)}"
+        return f"{self.base_url}{pathfmt.format(*args)}"
 
     def _raise_for_status(self, response):
         """Raises stored :class:`APIError`, if one occurred."""
@@ -427,12 +426,11 @@ class APIClient(_Session, DaemonApiMixin):
 
         if stream:
             return gen
-        else:
-            try:
-                # Wait for all the frames, concatenate them, and return the result
-                return consume_socket_output(gen, demux=demux)
-            finally:
-                response.close()
+        try:
+            # Wait for all the frames, concatenate them, and return the result
+            return consume_socket_output(gen, demux=demux)
+        finally:
+            response.close()
 
     def _disable_socket_timeout(self, socket):
         """Depending on the combination of python version and whether we are
@@ -462,14 +460,6 @@ class APIClient(_Session, DaemonApiMixin):
 
             s.settimeout(None)
 
-    @check_resource("container")
-    def _check_is_tty(self, container):
-        cont = self.inspect_container(container)
-        return cont["Config"]["Tty"]
-
-    def _get_result(self, container, stream, res):
-        return self._get_result_tty(stream, res, self._check_is_tty(container))
-
     def _get_result_tty(self, stream, res, is_tty):
         # We should also use raw streaming (without keep-alive)
         # if we are dealing with a tty-enabled container.
@@ -484,8 +474,7 @@ class APIClient(_Session, DaemonApiMixin):
         sep = b""
         if stream:
             return self._multiplexed_response_stream_helper(res)
-        else:
-            return sep.join(list(self._multiplexed_buffer_helper(res)))
+        return sep.join(list(self._multiplexed_buffer_helper(res)))
 
     def _unmount(self, *args):
         for proto in args:
@@ -497,8 +486,7 @@ class APIClient(_Session, DaemonApiMixin):
         except _InvalidSchema as e:
             if self._custom_adapter:
                 return self._custom_adapter
-            else:
-                raise e
+            raise e
 
     @property
     def api_version(self):

plugins/module_utils/_api/auth.py

@@ -368,7 +368,7 @@ def _load_legacy_config(config_file):
                 }
             }
         }
-    except Exception as e:
+    except Exception as e:  # pylint: disable=broad-exception-caught
         log.debug(e)
         pass
 

plugins/module_utils/_api/context/config.py

@@ -36,7 +36,7 @@ def get_current_context_name_with_source():
                     json.load(f).get("currentContext", "default"),
                     f"configuration file {docker_cfg_path}",
                 )
-        except Exception:
+        except Exception:  # pylint: disable=broad-exception-caught
             pass
     return "default", "fallback value"
 
@@ -54,7 +54,7 @@ def write_context_name_to_docker_config(name=None):
         try:
             with open(docker_cfg_path, "rt", encoding="utf-8") as f:
                 config = json.load(f)
-        except Exception as e:
+        except Exception as e:  # pylint: disable=broad-exception-caught
             return e
     current_context = config.get("currentContext", None)
     if current_context and not name:
@@ -68,7 +68,7 @@ def write_context_name_to_docker_config(name=None):
     try:
         with open(docker_cfg_path, "wt", encoding="utf-8") as f:
             json.dump(config, f, indent=4)
-    except Exception as e:
+    except Exception as e:  # pylint: disable=broad-exception-caught
         return e
 
 

plugins/module_utils/_api/context/context.py

@@ -42,7 +42,7 @@ class Context:
         description=None,
     ):
         if not name:
-            raise Exception("Name not provided")
+            raise ValueError("Name not provided")
         self.name = name
         self.context_type = None
         self.orchestrator = orchestrator
@@ -136,7 +136,7 @@ class Context:
                 metadata = json.load(f)
         except (OSError, KeyError, ValueError) as e:
             # unknown format
-            raise Exception(
+            raise RuntimeError(
                 f"Detected corrupted meta file for context {name} : {e}"
             ) from e
 
@@ -157,7 +157,7 @@ class Context:
     def _load_certs(self):
         certs = {}
         tls_dir = get_tls_dir(self.name)
-        for endpoint in self.endpoints.keys():
+        for endpoint in self.endpoints:
             if not os.path.isdir(os.path.join(tls_dir, endpoint)):
                 continue
             ca_cert = None
@@ -238,11 +238,11 @@ class Context:
         return self.context_type is None
 
     @property
-    def Name(self):
+    def Name(self):  # pylint: disable=invalid-name
         return self.name
 
     @property
-    def Host(self):
+    def Host(self):  # pylint: disable=invalid-name
         if not self.orchestrator or self.orchestrator == "swarm":
             endpoint = self.endpoints.get("docker", None)
             if endpoint:
@@ -252,27 +252,27 @@ class Context:
         return self.endpoints[self.orchestrator].get("Host", None)
 
     @property
-    def Orchestrator(self):
+    def Orchestrator(self):  # pylint: disable=invalid-name
         return self.orchestrator
 
     @property
-    def Metadata(self):
+    def Metadata(self):  # pylint: disable=invalid-name
         meta = {}
         if self.orchestrator:
             meta = {"StackOrchestrator": self.orchestrator}
         return {"Name": self.name, "Metadata": meta, "Endpoints": self.endpoints}
 
     @property
-    def TLSConfig(self):
+    def TLSConfig(self):  # pylint: disable=invalid-name
         key = self.orchestrator
         if not key or key == "swarm":
             key = "docker"
-        if key in self.tls_cfg.keys():
+        if key in self.tls_cfg:
             return self.tls_cfg[key]
         return None
 
     @property
-    def TLSMaterial(self):
+    def TLSMaterial(self):  # pylint: disable=invalid-name
         certs = {}
         for endpoint, tls in self.tls_cfg.items():
             cert, key = tls.cert
@@ -280,5 +280,5 @@ class Context:
         return {"TLSMaterial": certs}
 
     @property
-    def Storage(self):
+    def Storage(self):  # pylint: disable=invalid-name
         return {"Storage": {"MetadataPath": self.meta_path, "TLSPath": self.tls_path}}

plugins/module_utils/_api/credentials/store.py

@@ -91,8 +91,7 @@ class Store:
                 raise errors.StoreError(
                     f"{self.program} not installed or not available in PATH"
                 )
-            else:
-                raise errors.StoreError(
-                    f'Unexpected OS error "{e.strerror}", errno={e.errno}'
-                )
+            raise errors.StoreError(
+                f'Unexpected OS error "{e.strerror}", errno={e.errno}'
+            )
         return output

plugins/module_utils/_api/transport/npipesocket.py

@@ -28,9 +28,9 @@ except ImportError:
     PYWIN32_IMPORT_ERROR = traceback.format_exc()
 
 
-cERROR_PIPE_BUSY = 0xE7
-cSECURITY_SQOS_PRESENT = 0x100000
-cSECURITY_ANONYMOUS = 0
+ERROR_PIPE_BUSY = 0xE7
+SECURITY_SQOS_PRESENT = 0x100000
+SECURITY_ANONYMOUS = 0
 
 MAXIMUM_RETRY_COUNT = 10
 
@@ -55,7 +55,9 @@ class NpipeSocket:
     def __init__(self, handle=None):
         self._timeout = win32pipe.NMPWAIT_USE_DEFAULT_WAIT
         self._handle = handle
+        self._address = None
         self._closed = False
+        self.flags = None
 
     def accept(self):
         raise NotImplementedError()
@@ -77,8 +79,8 @@ class NpipeSocket:
                 None,
                 win32file.OPEN_EXISTING,
                 (
-                    cSECURITY_ANONYMOUS
-                    | cSECURITY_SQOS_PRESENT
+                    SECURITY_ANONYMOUS
+                    | SECURITY_SQOS_PRESENT
                     | win32file.FILE_FLAG_OVERLAPPED
                 ),
                 0,
@@ -86,7 +88,7 @@ class NpipeSocket:
         except win32pipe.error as e:
             # See Remarks:
             # https://msdn.microsoft.com/en-us/library/aa365800.aspx
-            if e.winerror == cERROR_PIPE_BUSY:
+            if e.winerror == ERROR_PIPE_BUSY:
                 # Another program or thread has grabbed our pipe instance
                 # before we got to it. Wait for availability and attempt to
                 # connect again.

plugins/module_utils/_api/transport/sshconn.py

@@ -72,7 +72,7 @@ class SSHSocket(socket.socket):
         env.pop("LD_LIBRARY_PATH", None)
         env.pop("SSL_CERT_FILE", None)
 
-        self.proc = subprocess.Popen(
+        self.proc = subprocess.Popen(  # pylint: disable=consider-using-with
             args,
             env=env,
             stdout=subprocess.PIPE,
@@ -82,7 +82,7 @@ class SSHSocket(socket.socket):
 
     def _write(self, data):
         if not self.proc or self.proc.stdin.closed:
-            raise Exception(
+            raise RuntimeError(
                 "SSH subprocess not initiated. connect() must be called first."
             )
         written = self.proc.stdin.write(data)
@@ -97,7 +97,7 @@ class SSHSocket(socket.socket):
 
     def recv(self, n):
         if not self.proc:
-            raise Exception(
+            raise RuntimeError(
                 "SSH subprocess not initiated. connect() must be called first."
             )
         return self.proc.stdout.read(n)

plugins/module_utils/_api/utils/build.py

@@ -125,19 +125,22 @@ def create_archive(root, files=None, fileobj=None, gzip=False, extra_files=None)
 
 
 def mkbuildcontext(dockerfile):
-    f = tempfile.NamedTemporaryFile()
-    t = tarfile.open(mode="w", fileobj=f)
-    if isinstance(dockerfile, io.StringIO):
-        raise TypeError("Please use io.BytesIO to create in-memory Dockerfiles")
-    elif isinstance(dockerfile, io.BytesIO):
-        dfinfo = tarfile.TarInfo("Dockerfile")
-        dfinfo.size = len(dockerfile.getvalue())
-        dockerfile.seek(0)
-    else:
-        dfinfo = t.gettarinfo(fileobj=dockerfile, arcname="Dockerfile")
-    t.addfile(dfinfo, dockerfile)
-    t.close()
-    f.seek(0)
+    f = tempfile.NamedTemporaryFile()  # pylint: disable=consider-using-with
+    try:
+        with tarfile.open(mode="w", fileobj=f) as t:
+            if isinstance(dockerfile, io.StringIO):
+                raise TypeError("Please use io.BytesIO to create in-memory Dockerfiles")
+            if isinstance(dockerfile, io.BytesIO):
+                dfinfo = tarfile.TarInfo("Dockerfile")
+                dfinfo.size = len(dockerfile.getvalue())
+                dockerfile.seek(0)
+            else:
+                dfinfo = t.gettarinfo(fileobj=dockerfile, arcname="Dockerfile")
+            t.addfile(dfinfo, dockerfile)
+        f.seek(0)
+    except Exception:  # noqa: E722
+        f.close()
+        raise
     return f
 
 

plugins/module_utils/_api/utils/config.py

@@ -68,8 +68,7 @@ def home_dir():
     """
     if IS_WINDOWS_PLATFORM:
         return os.environ.get("USERPROFILE", "")
-    else:
-        return os.path.expanduser("~")
+    return os.path.expanduser("~")
 
 
 def load_general_config(config_path=None):

plugins/module_utils/_api/utils/decorators.py

@@ -17,23 +17,6 @@ from .. import errors
 from . import utils
 
 
-def check_resource(resource_name):
-    def decorator(f):
-        @functools.wraps(f)
-        def wrapped(self, resource_id=None, *args, **kwargs):
-            if resource_id is None and kwargs.get(resource_name):
-                resource_id = kwargs.pop(resource_name)
-            if isinstance(resource_id, dict):
-                resource_id = resource_id.get("Id", resource_id.get("ID"))
-            if not resource_id:
-                raise errors.NullResource("Resource ID was not provided")
-            return f(self, resource_id, *args, **kwargs)
-
-        return wrapped
-
-    return decorator
-
-
 def minimum_version(version):
     def decorator(f):
         @functools.wraps(f)

plugins/module_utils/_api/utils/ports.py

@@ -90,9 +90,8 @@ def split_port(port):
         if external is not None and len(internal) != len(external):
             raise ValueError("Port ranges don't match in length")
         return internal, external
-    else:
-        if not external:
-            external = [None] * len(internal)
-        elif len(internal) != len(external):
-            raise ValueError("Port ranges don't match in length")
-        return internal, [(host, ext_port) for ext_port in external]
+    if not external:
+        external = [None] * len(internal)
+    elif len(internal) != len(external):
+        raise ValueError("Port ranges don't match in length")
+    return internal, [(host, ext_port) for ext_port in external]

plugins/module_utils/_api/utils/socket.py

@@ -111,8 +111,7 @@ def frames_iter(socket, tty):
     """
     if tty:
         return ((STDOUT, frame) for frame in frames_iter_tty(socket))
-    else:
-        return frames_iter_no_tty(socket)
+    return frames_iter_no_tty(socket)
 
 
 def frames_iter_no_tty(socket):
@@ -194,7 +193,6 @@ def demux_adaptor(stream_id, data):
     """
     if stream_id == STDOUT:
         return (data, None)
-    elif stream_id == STDERR:
+    if stream_id == STDERR:
         return (None, data)
-    else:
-        raise ValueError(f"{stream_id} is not a valid stream")
+    raise ValueError(f"{stream_id} is not a valid stream")

plugins/module_utils/_api/utils/utils.py

@@ -75,10 +75,9 @@ def compare_version(v1, v2):
     s2 = StrictVersion(v2)
     if s1 == s2:
         return 0
-    elif s1 > s2:
+    if s1 > s2:
         return -1
-    else:
-        return 1
+    return 1
 
 
 def version_lt(v1, v2):
@@ -250,7 +249,7 @@ def parse_host(addr, is_win32=False, tls=False):
 
     # These protos are valid aliases for our library but not for the
     # official spec
-    if proto == "http" or proto == "https":
+    if proto in ("http", "https"):
         tls = proto == "https"
         proto = "tcp"
     elif proto == "http+unix":
@@ -273,12 +272,11 @@ def parse_host(addr, is_win32=False, tls=False):
         raise errors.DockerException(
             f"Invalid bind address format: no path allowed for this protocol: {addr}"
         )
-    else:
-        path = parsed_url.path
-        if proto == "unix" and parsed_url.hostname is not None:
-            # For legacy reasons, we consider unix://path
-            # to be valid and equivalent to unix:///path
-            path = "/".join((parsed_url.hostname, path))
+    path = parsed_url.path
+    if proto == "unix" and parsed_url.hostname is not None:
+        # For legacy reasons, we consider unix://path
+        # to be valid and equivalent to unix:///path
+        path = "/".join((parsed_url.hostname, path))
 
     netloc = parsed_url.netloc
     if proto in ("tcp", "ssh"):
@@ -419,7 +417,7 @@ def parse_bytes(s):
     else:
         digits_part = s[:-1]
 
-    if suffix in units.keys() or suffix.isdigit():
+    if suffix in units or suffix.isdigit():
         try:
             digits = float(digits_part)
         except ValueError: