From 2b2aadb174cf2bde5b3fcf02e9beb59e9979b079 Mon Sep 17 00:00:00 2001 From: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com> Date: Fri, 17 Jan 2025 16:06:21 +0100 Subject: [PATCH] lint: Add Ruff rules (#283) --- pyproject.toml | 18 +++++++++++++++++- tests/test_prompts.py | 2 +- zabbix_cli/auth.py | 7 ++++--- zabbix_cli/cache.py | 2 +- zabbix_cli/commands/proxy.py | 4 ++-- zabbix_cli/commands/usergroup.py | 2 +- zabbix_cli/config/model.py | 4 +++- zabbix_cli/output/formatting/dates.py | 4 ++-- zabbix_cli/pyzabbix/client.py | 2 +- zabbix_cli/pyzabbix/enums.py | 2 +- zabbix_cli/pyzabbix/utils.py | 4 ++-- zabbix_cli/repl/completer.py | 2 +- zabbix_cli/repl/repl.py | 2 +- zabbix_cli/update.py | 12 +++++++----- zabbix_cli/utils/args.py | 2 +- zabbix_cli/utils/fs.py | 12 ++++++------ zabbix_cli/utils/utils.py | 2 +- 17 files changed, 52 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1943edae..9205ef8a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -154,11 +154,27 @@ extend-include = [ "tests/**/*.py", ] + [tool.ruff.lint.pydocstyle] convention = "google" [tool.ruff.lint] -extend-select = ["I"] +extend-select = [ + "E", # pydecodestyle (errors) + "W", # pydecodestyle (warnings) + "G", # flake8-logging-format + "I", # isort + "LOG", # flake8-logging + "PLE1205", # pylint (too many logging args) + "PLE1206", # pylint (too few logging args) + "TID252", # flake8-tidy-imports (prefer absolute imports) + "C4", # flake8-comprehensions + "B", # flake8-bugbear +] +ignore = [ + "E501", # Avoid enforcing line-length violations + "B008", # Argument default function call (incompatible with typer) +] [tool.ruff.lint.isort] diff --git a/tests/test_prompts.py b/tests/test_prompts.py index 659315d0..e345decc 100644 --- a/tests/test_prompts.py +++ b/tests/test_prompts.py @@ -60,5 +60,5 @@ def _do_test_is_headless(envvar: str, value: str | None, expected: bool): assert is_headless() == expected finally: # IMPORTANT: Remove envvar and clear cache after each test - os.environ = _orig_environ # type: ignore + os.environ = _orig_environ # type: ignore # noqa: B003 # I _think_ this is fine? is_headless.cache_clear() diff --git a/zabbix_cli/auth.py b/zabbix_cli/auth.py index 6fcedb33..a4a61d63 100644 --- a/zabbix_cli/auth.py +++ b/zabbix_cli/auth.py @@ -604,7 +604,8 @@ def load_auth_token_file(self) -> Union[tuple[Path, str], tuple[None, None]]: if contents: return path, contents logger.info( - f"No auth token file found. Searched in {', '.join(str(p) for p in paths)}" + "No auth token file found. Searched in %s", + {", ".join(str(p) for p in paths)}, ) return None, None @@ -616,7 +617,7 @@ def load_auth_file(self) -> tuple[Optional[Path], Optional[str]]: if contents: return path, contents logger.info( - f"No auth file found. Searched in {', '.join(str(p) for p in paths)}" + "No auth file found. Searched in %s", {", ".join(str(p) for p in paths)} ) return None, None @@ -709,7 +710,7 @@ def write_auth_token_file( try: file.write_text(f"{username}::{auth_token}") - logger.info(f"Wrote auth token file {file}") + logger.info("Wrote auth token file %s", file) except OSError as e: raise AuthTokenFileError(f"Unable to write auth token file {file}: {e}") from e diff --git a/zabbix_cli/cache.py b/zabbix_cli/cache.py index 0793e23e..8a370568 100644 --- a/zabbix_cli/cache.py +++ b/zabbix_cli/cache.py @@ -35,7 +35,7 @@ def populate(self) -> None: self._populate_hostgroup_cache() self._populate_templategroup_cache() except Exception as e: - raise ZabbixCLIError(f"Failed to populate Zabbix cache: {e}") + raise ZabbixCLIError(f"Failed to populate Zabbix cache: {e}") from e def _populate_hostgroup_cache(self) -> None: """Populates the hostgroup caches with data from the Zabbix API.""" diff --git a/zabbix_cli/commands/proxy.py b/zabbix_cli/commands/proxy.py index c5a86ea2..2c29efd8 100644 --- a/zabbix_cli/commands/proxy.py +++ b/zabbix_cli/commands/proxy.py @@ -203,7 +203,7 @@ def load_balance_proxy_hosts( all_hosts = list(itertools.chain.from_iterable(p.hosts for p in proxies)) if not all_hosts: exit_err("Proxies have no hosts to load balance.") - logging.debug(f"Found {len(all_hosts)} hosts to load balance.") + logging.debug("Found %d hosts to load balance.", len(all_hosts)) lb_proxies = { p.proxyid: LBProxy(proxy=p, weight=w) for p, w in zip(proxies, weights) @@ -226,7 +226,7 @@ def load_balance_proxy_hosts( "Proxy '%s' has no hosts after balancing.", lb_proxy.proxy.name ) continue - logging.debug(f"Moving {n_hosts} hosts to proxy {lb_proxy.proxy.name!r}") + logging.debug("Moving %d hosts to proxy %r", n_hosts, lb_proxy.proxy.name) app.state.client.move_hosts_to_proxy( hosts=lb_proxy.hosts, diff --git a/zabbix_cli/commands/usergroup.py b/zabbix_cli/commands/usergroup.py index 4ab0edf7..4101df12 100644 --- a/zabbix_cli/commands/usergroup.py +++ b/zabbix_cli/commands/usergroup.py @@ -58,7 +58,7 @@ def sort_ugroups( try: return sorted(ugroups, key=lambda ug: int(ug.usrgrpid)) except Exception as e: - logging.error(f"Failed to sort user groups by ID: {e}") + logging.error("Failed to sort user groups by ID: %s", e) # Fall back on unsorted (likely sorted by ID anyway) return ugroups diff --git a/zabbix_cli/config/model.py b/zabbix_cli/config/model.py index cc2ba5f7..a783afdc 100644 --- a/zabbix_cli/config/model.py +++ b/zabbix_cli/config/model.py @@ -547,7 +547,9 @@ def get( adapter = _get_type_adapter(type) return adapter.validate_python(attr) except AttributeError: - raise ConfigOptionNotFound(f"Plugin configuration key '{key}' not found") + raise ConfigOptionNotFound( + f"Plugin configuration key '{key}' not found" + ) from None except ValidationError as e: raise PluginConfigTypeError( f"Plugin config key '{key}' failed to validate as type {type}: {e}" diff --git a/zabbix_cli/output/formatting/dates.py b/zabbix_cli/output/formatting/dates.py index dc7d45e8..5b0f5bd5 100644 --- a/zabbix_cli/output/formatting/dates.py +++ b/zabbix_cli/output/formatting/dates.py @@ -2,8 +2,8 @@ from datetime import datetime -from ...logs import logger -from .constants import NONE_STR +from zabbix_cli.logs import logger +from zabbix_cli.output.formatting.constants import NONE_STR def datetime_str( diff --git a/zabbix_cli/pyzabbix/client.py b/zabbix_cli/pyzabbix/client.py index 16315c2f..4bf8f8f5 100644 --- a/zabbix_cli/pyzabbix/client.py +++ b/zabbix_cli/pyzabbix/client.py @@ -167,7 +167,7 @@ def parse_name_or_id_arg( # If we have a wildcard, we can omit names or IDs entirely if "*" in names_or_ids: - names_or_ids = tuple() + names_or_ids = () if len(names_or_ids) > 1: logger.debug( diff --git a/zabbix_cli/pyzabbix/enums.py b/zabbix_cli/pyzabbix/enums.py index 588e927c..e468b229 100644 --- a/zabbix_cli/pyzabbix/enums.py +++ b/zabbix_cli/pyzabbix/enums.py @@ -315,7 +315,7 @@ def get_port(self: InterfaceType) -> str: try: return self.value.metadata["port"] except KeyError: - raise ZabbixCLIError(f"Unknown interface type: {self}") + raise ZabbixCLIError(f"Unknown interface type: {self}") from None class InventoryMode(APIStrEnum): diff --git a/zabbix_cli/pyzabbix/utils.py b/zabbix_cli/pyzabbix/utils.py index 380969c8..052e4e60 100644 --- a/zabbix_cli/pyzabbix/utils.py +++ b/zabbix_cli/pyzabbix/utils.py @@ -21,8 +21,8 @@ def get_random_proxy(client: ZabbixAPI, pattern: Optional[str] = None) -> Proxy: if pattern: try: re_pattern = re.compile(pattern) - except re.error: - raise ZabbixAPICallError(f"Invalid proxy regex pattern: {pattern!r}") + except re.error as e: + raise ZabbixAPICallError(f"Invalid proxy regex pattern: {pattern!r}") from e proxies = [proxy for proxy in proxies if re_pattern.match(proxy.name)] if not proxies: raise ZabbixNotFoundError(f"No proxies matching pattern {pattern!r}") diff --git a/zabbix_cli/repl/completer.py b/zabbix_cli/repl/completer.py index 305aeb7a..49977353 100644 --- a/zabbix_cli/repl/completer.py +++ b/zabbix_cli/repl/completer.py @@ -237,7 +237,7 @@ def _get_completion_for_cmd_args( current_args = args[param.nargs * -1 :] # Show only unused opts - already_present = any([opt in previous_args for opt in opts]) + already_present = any(opt in previous_args for opt in opts) hide = self.show_only_unused and already_present and not param.multiple # Show only shortest opt diff --git a/zabbix_cli/repl/repl.py b/zabbix_cli/repl/repl.py index 8ae7ecef..9a98fc11 100644 --- a/zabbix_cli/repl/repl.py +++ b/zabbix_cli/repl/repl.py @@ -49,7 +49,7 @@ class InternalCommand(NamedTuple): description: str -_internal_commands: dict[str, InternalCommand] = dict() +_internal_commands: dict[str, InternalCommand] = {} def _register_internal_command( diff --git a/zabbix_cli/update.py b/zabbix_cli/update.py index 6ff61536..3ab04082 100644 --- a/zabbix_cli/update.py +++ b/zabbix_cli/update.py @@ -358,8 +358,10 @@ def get_release_version(self) -> str: raise UpdateError(f"Failed to get latest release: {resp.text}") try: release = GitHubRelease.model_validate_json(resp.text) - except ValidationError: - raise UpdateError(f"Failed to parse GitHub release info: {resp.text}") + except ValidationError as e: + raise UpdateError( + f"Failed to parse GitHub release info: {resp.text}" + ) from e if not release.tag_name: raise UpdateError(f"Latest GitHub release {release.url!r} has no tag name.") return release.tag_name @@ -377,7 +379,7 @@ def resolve_path(self, path: Path) -> Path: if alias_path.name == "python": raise UpdateError( "Unable to resolve PyInstaller executable. Please update manually." - ) + ) from None return alias_path @staticmethod @@ -459,7 +461,7 @@ def to_path(p: str) -> Optional[Path]: try: return Path(p).expanduser().resolve() except Exception as e: - logger.debug(f"Unable to resolve path {p}: {e}") + logger.debug("Unable to resolve path %s: %s", p, e) return None @@ -525,7 +527,7 @@ def get_pipx_bin_dir() -> Optional[Path]: ["pipx", "environment", "--value", "PIPX_BIN_DIR"], text=True ) except subprocess.CalledProcessError as e: - logger.error(f"Failed to get pipx bin dir with command {e.cmd!r}: {e}") + logger.error("Failed to get pipx bin dir with command %r: %s", e.cmd, e) return except FileNotFoundError: return # pipx not installed diff --git a/zabbix_cli/utils/args.py b/zabbix_cli/utils/args.py index e711166f..7179d512 100644 --- a/zabbix_cli/utils/args.py +++ b/zabbix_cli/utils/args.py @@ -29,7 +29,7 @@ def is_set(ctx: typer.Context, option: str) -> bool: src = ctx.get_parameter_source(option) if not src: - logging.warning(f"Parameter {option} not found in context.") + logging.warning("Parameter %s not found in context.", option) return False # HACK: A typer callback that sets an empty list as a default value diff --git a/zabbix_cli/utils/fs.py b/zabbix_cli/utils/fs.py index 0015c4fe..e0861f41 100644 --- a/zabbix_cli/utils/fs.py +++ b/zabbix_cli/utils/fs.py @@ -48,10 +48,10 @@ def open_directory( if not directory.exists(): raise FileNotFoundError directory = directory.resolve(strict=True) - except FileNotFoundError: - raise ZabbixCLIError(f"Directory {directory} does not exist") - except OSError: - raise ZabbixCLIError(f"Unable to resolve symlinks for {directory}") + except FileNotFoundError as e: + raise ZabbixCLIError(f"Directory {directory} does not exist") from e + except OSError as e: + raise ZabbixCLIError(f"Unable to resolve symlinks for {directory}") from e if not directory.is_dir(): raise ZabbixCLIError(f"{directory} is not a directory") @@ -80,7 +80,7 @@ def mkdir_if_not_exists(path: Path) -> None: except Exception as e: raise ZabbixCLIFileError(f"Failed to create directory {path}: {e}") from e else: - logger.info(f"Created directory: {path}") + logger.info("Created directory: %s", path) def sanitize_filename(filename: str) -> str: @@ -120,7 +120,7 @@ def move_file(src: Path, dest: Path, mkdir: bool = True) -> None: except Exception as e: raise ZabbixCLIError(f"Failed to move {src} to {dest}: {e}") from e else: - logger.info(f"Moved {src} to {dest}") + logger.info("Moved %s to %s", src, dest) @contextmanager diff --git a/zabbix_cli/utils/utils.py b/zabbix_cli/utils/utils.py index d016b9fb..af7d58cb 100644 --- a/zabbix_cli/utils/utils.py +++ b/zabbix_cli/utils/utils.py @@ -274,7 +274,7 @@ def try_convert_int(s: str) -> int: try: return int(s) except ValueError: - raise ZabbixCLIError(f"Invalid time value: {s}") + raise ZabbixCLIError(f"Invalid time value: {s}") from None time = time.replace(" ", "") for time_value in TIME_VALUES: