Cleanup and fix typing issues with latest mypy

Mainly facts - the changes to the `FactBase` types are breaking but not
any functionality. For typing fact classes to pass they must implement
the `command` & `requires_command` attributes as class methods, not
plain strings.

This change hugely simplifies the typing logic here (method or string?)
which the latest (1.11) mypy raised issues for. The underlying logic
remains unchanged and string attributes will continue to work for the
foreseeable future.

Author: Fizzadar

pyinfra/api/facts.py

@@ -14,18 +14,7 @@
 import re
 from inspect import getcallargs
 from socket import error as socket_error, timeout as timeout_error
-from typing import (
-    TYPE_CHECKING,
-    Any,
-    Callable,
-    Generic,
-    Iterable,
-    Optional,
-    Type,
-    TypeVar,
-    Union,
-    cast,
-)
+from typing import TYPE_CHECKING, Any, Callable, Generic, Iterable, Optional, Type, TypeVar, cast
 
 import click
 import gevent
@@ -38,7 +27,6 @@
     get_kwargs_str,
     log_error_or_warning,
     log_host_command_error,
-    make_hash,
     print_host_combined_output,
 )
 from pyinfra.connectors.util import CommandOutput
@@ -66,11 +54,12 @@ class FactBase(Generic[T]):
 
     abstract: bool = True
 
-    shell_executable: Optional[str] = None
+    shell_executable: str | None = None
 
-    requires_command: Optional[str] = None
+    command: Callable[..., str | StringCommand]
 
-    command: Union[str, Callable]
+    def requires_command(self, *args, **kwargs) -> str | None:
+        return None
 
     def __init_subclass__(cls) -> None:
         super().__init_subclass__()
@@ -113,8 +102,7 @@ def __init_subclass__(cls) -> None:
         module_name = cls.__module__.replace("pyinfra.facts.", "")
         cls.name = f"{module_name}.{cls.__name__}"
 
-    @staticmethod
-    def process_data(data):
+    def process_data(self, data):
         return data
 
 
@@ -130,30 +118,6 @@ def _make_command(command_attribute, host_args):
     return command_attribute
 
 
-def _get_executor_kwargs(
-    state: "State",
-    host: "Host",
-    override_kwargs: Optional[dict[str, Any]] = None,
-    override_kwarg_keys: Optional[list[str]] = None,
-):
-    if override_kwargs is None:
-        override_kwargs = {}
-    if override_kwarg_keys is None:
-        override_kwarg_keys = []
-
-    # Use the current operation global kwargs, or generate defaults
-    global_kwargs = host.current_op_global_arguments
-    if not global_kwargs:
-        global_kwargs, _ = pop_global_arguments({}, state, host)
-
-    # Apply any current op kwargs that *weren't* found in the overrides
-    override_kwargs.update(
-        {key: value for key, value in global_kwargs.items() if key not in override_kwarg_keys},
-    )
-
-    return {key: value for key, value in override_kwargs.items() if key in CONNECTOR_ARGUMENT_KEYS}
-
-
 def _handle_fact_kwargs(state, host, cls, args, kwargs):
     args = args or []
     kwargs = kwargs or {}
@@ -296,7 +260,7 @@ def _get_fact(
         log_host_command_error(
             host,
             e,
-            timeout=global_kwargs["_timeout"],
+            timeout=global_kwargs.get("_timeout"),
         )
 
     stdout_lines, stderr_lines = output.stdout_lines, output.stderr_lines
@@ -346,13 +310,6 @@ def _get_fact(
     return data
 
 
-def _get_fact_hash(state: "State", host: "Host", cls, args, kwargs):
-    if issubclass(cls, ShortFactBase):
-        cls = cls.fact
-    fact_kwargs, executor_kwargs = _handle_fact_kwargs(state, host, cls, args, kwargs)
-    return make_hash((cls, fact_kwargs, executor_kwargs))
-
-
 def get_host_fact(
     state: "State",
     host: "Host",

pyinfra/api/util.py

@@ -241,7 +241,7 @@ def log_error_or_warning(
     )
 
 
-def log_host_command_error(host: "Host", e: Exception, timeout: int = 0) -> None:
+def log_host_command_error(host: "Host", e: Exception, timeout: int | None = 0) -> None:
     if isinstance(e, timeout_error):
         logger.error(
             "{0}{1}".format(

pyinfra/connectors/sshuserclient/client.py

@@ -124,7 +124,7 @@ class SSHClient(ParamikoClient):
     original idea at http://bitprophet.org/blog/2012/11/05/gateway-solutions/.
     """
 
-    def connect(
+    def connect(  # type: ignore[override]
         self,
         hostname,
         _pyinfra_ssh_forward_agent=None,

pyinfra/facts/apk.py

@@ -18,8 +18,11 @@ class ApkPackages(FactBase):
         }
     """
 
-    command = "apk list --installed"
-    requires_command = "apk"
+    def command(self) -> str:
+        return "apk list --installed"
+
+    def requires_command(self) -> str:
+        return "apk"
 
     default = dict
 

pyinfra/facts/apt.py

@@ -52,11 +52,14 @@ class AptSources(FactBase):
         ]
     """
 
-    command = make_cat_files_command(
-        "/etc/apt/sources.list",
-        "/etc/apt/sources.list.d/*.list",
-    )
-    requires_command = "apt"  # if apt installed, above should exist
+    def command(self) -> str:
+        return make_cat_files_command(
+            "/etc/apt/sources.list",
+            "/etc/apt/sources.list.d/*.list",
+        )
+
+    def requires_command(self) -> str:
+        return "apt"  # if apt installed, above should exist
 
     default = list
 
@@ -86,5 +89,8 @@ class AptKeys(GpgFactBase):
     """
 
     # This requires both apt-key *and* apt-key itself requires gpg
-    command = "! command -v gpg || apt-key list --with-colons"
-    requires_command = "apt-key"
+    def command(self) -> str:
+        return "! command -v gpg || apt-key list --with-colons"
+
+    def requires_command(self) -> str:
+        return "apt-key"

pyinfra/facts/brew.py

@@ -37,18 +37,22 @@ class BrewVersion(FactBase):
 
     """
 
-    command = "brew --version"
-    requires_command = "brew"
+    def command(self) -> str:
+        return "brew --version"
+
+    def requires_command(self) -> str:
+        return "brew"
 
     @staticmethod
     def default():
         return [0, 0, 0]
 
     def process(self, output):
-        m = VERSION_MATCHER.match(output[0])
+        out = list(output)[0]
+        m = VERSION_MATCHER.match(out)
         if m is not None:
             return [int(m.group(key)) for key in ["major", "minor", "patch"]]
-        logger.warning("could not parse version string from brew: %s", output[0])
+        logger.warning("could not parse version string from brew: %s", out)
         return self.default()
 
 
@@ -63,8 +67,11 @@ class BrewPackages(FactBase):
         }
     """
 
-    command = "brew list --versions"
-    requires_command = "brew"
+    def command(self) -> str:
+        return "brew list --versions"
+
+    def requires_command(self) -> str:
+        return "brew"
 
     default = dict
 
@@ -83,20 +90,26 @@ class BrewCasks(BrewPackages):
         }
     """
 
-    command = (
-        r'if brew --version | grep -q -e "Homebrew\ +(1\.|2\.[0-5]).*" 1>/dev/null;'
-        r"then brew cask list --versions; else brew list --cask --versions; fi"
-    )
-    requires_command = "brew"
+    def command(self) -> str:
+        return (
+            r'if brew --version | grep -q -e "Homebrew\ +(1\.|2\.[0-5]).*" 1>/dev/null;'
+            r"then brew cask list --versions; else brew list --cask --versions; fi"
+        )
+
+    def requires_command(self) -> str:
+        return "brew"
 
 
 class BrewTaps(FactBase):
     """
     Returns a list of brew taps.
     """
 
-    command = "brew tap"
-    requires_command = "brew"
+    def command(self) -> str:
+        return "brew tap"
+
+    def requires_command(self) -> str:
+        return "brew"
 
     default = list
 

pyinfra/facts/bsdinit.py

@@ -9,11 +9,12 @@ class RcdStatus(InitdStatus):
     BSD init scripts are well behaved and as such their output can be trusted.
     """
 
-    command = """
-        for SERVICE in `find /etc/rc.d /usr/local/etc/rc.d -type f`; do
-            $SERVICE status 2> /dev/null || $SERVICE check 2> /dev/null
-            echo "`basename $SERVICE`=$?"
-        done
-    """
+    def command(self) -> str:
+        return """
+            for SERVICE in `find /etc/rc.d /usr/local/etc/rc.d -type f`; do
+                $SERVICE status 2> /dev/null || $SERVICE check 2> /dev/null
+                echo "`basename $SERVICE`=$?"
+            done
+        """
 
     default = dict

pyinfra/facts/cargo.py

@@ -22,10 +22,11 @@ class CargoPackages(FactBase):
 
     default = dict
 
-    requires_command = "cargo"
-
-    def command(self):
+    def command(self) -> str:
         return "cargo install --list"
 
+    def requires_command(self) -> str:
+        return "cargo"
+
     def process(self, output):
         return parse_packages(CARGO_REGEX, output)

pyinfra/facts/choco.py

@@ -18,7 +18,9 @@ class ChocoPackages(FactBase):
         }
     """
 
-    command = "choco list"
+    def command(self) -> str:
+        return "choco list"
+
     shell_executable = "ps"
 
     default = dict
@@ -32,8 +34,8 @@ class ChocoVersion(FactBase):
     Returns the choco (Chocolatey) version.
     """
 
-    command = "choco --version"
+    def command(self) -> str:
+        return "choco --version"
 
-    @staticmethod
-    def process(output):
+    def process(self, output):
         return "".join(output).replace("\n", "")

pyinfra/facts/deb.py

@@ -16,8 +16,11 @@ class DebArch(FactBase):
     Returns the architecture string used in apt repository sources, eg ``amd64``.
     """
 
-    command = "dpkg --print-architecture"
-    requires_command = "dpkg"
+    def command(self) -> str:
+        return "dpkg --print-architecture"
+
+    def requires_command(self) -> str:
+        return "dpkg"
 
 
 class DebPackages(FactBase):
@@ -31,8 +34,11 @@ class DebPackages(FactBase):
         }
     """
 
-    command = "dpkg -l"
-    requires_command = "dpkg"
+    def command(self) -> str:
+        return "dpkg -l"
+
+    def requires_command(self) -> str:
+        return "dpkg"
 
     default = dict
 
@@ -55,7 +61,8 @@ class DebPackage(FactBase):
         "version": r"^Version:\s+({0})$".format(DEB_PACKAGE_VERSION_REGEX),
     }
 
-    requires_command = "dpkg"
+    def requires_command(self, package) -> str:
+        return "dpkg"
 
     def command(self, package):
         return "! test -e {0} && (dpkg -s {0} 2>/dev/null || true) || dpkg -I {0}".format(