Skip to content

Commit

Permalink
Ruff rules overhaul (#98)
Browse files Browse the repository at this point in the history
* Improve Ruff config

* Enable `C4` rule

* Enable flake8-bugbear (B) rules, fix bugs

* Fix api params

* Add E and W rules
  • Loading branch information
pederhan authored Jan 17, 2025
1 parent 484eac3 commit 276a0c6
Show file tree
Hide file tree
Showing 16 changed files with 83 additions and 66 deletions.
34 changes: 31 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,49 @@ exclude = ["/.github", "/tests", "/path"]
[tool.hatch.build.targets.wheel]
packages = ["zabbix_auto_config"]


[tool.pyright]
pythonVersion = "3.9"

[tool.ruff]
# Same as Black.
line-length = 88
src = ["zabbix_auto_config"]
extend-exclude = ["zabbix_auto_config/__init__.py"]
extend-include = [
"pyproject.toml",
"zabbix_auto_config/**/*.py",
"tests/**/*.py",
]

[tool.ruff.lint]
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
]
# 2. Avoid enforcing line-length violations (`E501`)
ignore = ["E501"]

# 3. Avoid trying to fix flake8-bugbear (`B`) violations.
unfixable = ["B"]

[tool.ruff.format]
# Enable auto-formatting for docstrings.
docstring-code-format = true

[tool.ruff.lint.pydocstyle]
convention = "google"

[tool.ruff.lint.isort]
# Force one line per import to simplify diffing and merging
force-single-line = true
# Add annotations import to every file
required-imports = ["from __future__ import annotations"]

[tool.pyright]
pythonVersion = "3.9"
1 change: 0 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import pytest
import tomli

from zabbix_auto_config import models


Expand Down
3 changes: 1 addition & 2 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

import pytest
import tomli
import zabbix_auto_config.models as models
from hypothesis import given
from hypothesis import settings
from hypothesis import strategies as st
from inline_snapshot import snapshot
from pydantic import ValidationError

import zabbix_auto_config.models as models


def test_sample_config(sample_config: str):
models.Settings(**tomli.loads(sample_config))
Expand Down
1 change: 0 additions & 1 deletion tests/test_errcount.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from typing import Callable

import pytest

from zabbix_auto_config.errcount import Error
from zabbix_auto_config.errcount import RollingErrorCounter
from zabbix_auto_config.errcount import get_td
Expand Down
1 change: 0 additions & 1 deletion tests/test_failsafe.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from unittest.mock import MagicMock

import pytest

from zabbix_auto_config.exceptions import ZACException
from zabbix_auto_config.failsafe import check_failsafe
from zabbix_auto_config.failsafe import check_failsafe_ok_file
Expand Down
1 change: 0 additions & 1 deletion tests/test_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import datetime

import pytest

from zabbix_auto_config.health import HealthFile
from zabbix_auto_config.health import ProcessInfo
from zabbix_auto_config.state import State
Expand Down
1 change: 0 additions & 1 deletion tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import pytest
from pydantic import ValidationError

from zabbix_auto_config import models

# NOTE: Do not test msg and ctx of Pydantic errors!
Expand Down
1 change: 0 additions & 1 deletion tests/test_processing/test_sourcecollectorprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from typing import List

import pytest

from zabbix_auto_config.models import Host
from zabbix_auto_config.models import SourceCollectorSettings
from zabbix_auto_config.processing import SourceCollectorProcess
Expand Down
6 changes: 3 additions & 3 deletions tests/test_processing/test_zabbixupdater.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
import pytest
from httpx import ConnectTimeout
from httpx import ReadTimeout

from tests.conftest import MockZabbixAPI
from tests.conftest import PicklableMock
from zabbix_auto_config import exceptions
from zabbix_auto_config.models import Settings
from zabbix_auto_config.models import ZabbixSettings
from zabbix_auto_config.processing import ZabbixUpdater
from zabbix_auto_config.state import get_manager

from tests.conftest import MockZabbixAPI
from tests.conftest import PicklableMock


def raises_connect_timeout(*args, **kwargs):
raise ConnectTimeout("connect timeout")
Expand Down
1 change: 0 additions & 1 deletion tests/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import pytest
from inline_snapshot import snapshot

from zabbix_auto_config.exceptions import ZACException
from zabbix_auto_config.processing import BaseProcess
from zabbix_auto_config.state import State
Expand Down
1 change: 0 additions & 1 deletion tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from hypothesis import settings
from hypothesis import strategies as st
from pytest import LogCaptureFixture

from zabbix_auto_config import utils
from zabbix_auto_config.pyzabbix.types import HostTag

Expand Down
2 changes: 1 addition & 1 deletion zabbix_auto_config/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ def merge(self, other: "Host") -> None:
self.hostname,
)
# TODO: Do something different? Is alphabetically first "good enough"? It will be consistent at least.
self.proxy_pattern = sorted(list(proxy_patterns))[0]
self.proxy_pattern = sorted(proxy_patterns)[0]
elif len(proxy_patterns) == 1:
self.proxy_pattern = proxy_patterns.pop()

Expand Down
28 changes: 14 additions & 14 deletions zabbix_auto_config/processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def collect(self) -> None:
f"Collected object is not a Host object: {host!r}. Type: {type(host)}"
)

host.sources = set([self.name])
host.sources = {self.name}
valid_hosts.append(host)

# Add source hosts to queue
Expand Down Expand Up @@ -381,7 +381,7 @@ def __init__(
# TODO: Test connection? Cursor?
except psycopg2.OperationalError as e:
logging.error("Unable to connect to database.")
raise ZACException(*e.args)
raise ZACException(*e.args) from e

self.source_hosts_queues = source_hosts_queues
for source_hosts_queue in self.source_hosts_queues:
Expand Down Expand Up @@ -696,7 +696,7 @@ def __init__(
# TODO: Test connection? Cursor?
except psycopg2.OperationalError as e:
logging.error("Unable to connect to database. Process exiting with error")
raise ZACException(*e.args)
raise ZACException(*e.args) from e

self.config = settings.zabbix
self.settings = settings
Expand Down Expand Up @@ -732,15 +732,15 @@ def login(self) -> None:
self.api.login(self.config.username, self.config.password)
except httpx.ConnectError as e:
logging.error("Error while connecting to Zabbix: %s", self.config.url)
raise ZACException(*e.args)
raise ZACException(*e.args) from e
except httpx.TimeoutException as e:
logging.error(
"Timed out while connecting to Zabbix API: %s", self.config.url
)
raise ZACException(*e.args)
raise ZACException(*e.args) from e
except (ZabbixAPIException, httpx.HTTPError) as e:
logging.error("Unable to login to Zabbix API: %s", str(e))
raise ZACException(*e.args)
raise ZACException(*e.args) from e

def work(self) -> None:
start_time = time.time()
Expand Down Expand Up @@ -778,7 +778,7 @@ def get_hostgroups(self, name: Optional[str] = None) -> List[HostGroup]:
names = [name] if name else []
hostgroups = self.api.get_hostgroups(*names)
except ZabbixAPIException as e:
raise ZACException("Error when fetching hostgroups: %s", e)
raise ZACException("Error when fetching hostgroups: %s", e) from e
return hostgroups


Expand Down Expand Up @@ -1250,7 +1250,7 @@ def do_update(self) -> None:
zabbix_managed_hosts: List[Host] = []
zabbix_manual_hosts: List[Host] = []

for hostname, host in zabbix_hosts.items():
for host in zabbix_hosts.values():
if self.stop_event.is_set():
logging.debug("Told to stop. Breaking")
break
Expand Down Expand Up @@ -1688,7 +1688,7 @@ def create_hostgroup(self, hostgroup_name: str) -> Optional[str]:
def create_extra_hostgroups(self, existing_hostgroups: List[HostGroup]) -> None:
"""Creates additonal host groups based on the prefixes specified
in the config file. These host groups are not assigned hosts by ZAC."""
hostgroup_names = set(h.name for h in existing_hostgroups)
hostgroup_names = {h.name for h in existing_hostgroups}

for prefix in self.config.extra_siteadmin_hostgroup_prefixes:
mapping = utils.mapping_values_with_prefix(
Expand Down Expand Up @@ -1730,7 +1730,7 @@ def create_templategroups(self, existing_hostgroups: List[HostGroup]) -> None:
# The prefix is determined by the separator defined in the config file.
# If we use the template group prefix `Templates-`, we go from
# `Siteadmin-bob-hosts` to `Templates-bob-hosts`.
tgroups = set(
tgroups = {
utils.with_prefix(
tg,
self.config.templategroup_prefix,
Expand All @@ -1739,7 +1739,7 @@ def create_templategroups(self, existing_hostgroups: List[HostGroup]) -> None:
for tg in itertools.chain.from_iterable(
self.siteadmin_hostgroup_map.values()
)
)
}
if compat.templategroups_supported(self.zabbix_version):
logging.debug(
"Zabbix version is %s. Will create template groups.",
Expand All @@ -1760,7 +1760,7 @@ def _create_templategroups(self, tgroups: Set[str]) -> None:
tgroups: A set of template group names to create.
"""
res = self.api.get_templategroups()
existing_tgroups = set(tg.name for tg in res)
existing_tgroups = {tg.name for tg in res}
for tgroup in tgroups:
if tgroup in existing_tgroups:
continue
Expand All @@ -1777,7 +1777,7 @@ def _create_templategroups_pre_62_compat(
Args:
tgroups: A set of host group names to create.
"""
existing_hgroup_names = set(h.name for h in existing_hostgroups)
existing_hgroup_names = {h.name for h in existing_hostgroups}
for tgroup in tgroups:
if tgroup in existing_hgroup_names:
continue
Expand Down Expand Up @@ -1846,7 +1846,7 @@ def do_update(self) -> None:

# Determine host groups to sync for host
# Sync host groups derived from its properties, siteadmins, sources, etc.
synced_hostgroup_names = set([self.config.hostgroup_all])
synced_hostgroup_names = {self.config.hostgroup_all}
for prop in db_host.properties:
if prop in self.property_hostgroup_map:
synced_hostgroup_names.update(self.property_hostgroup_map[prop])
Expand Down
Loading

0 comments on commit 276a0c6

Please sign in to comment.