Skip to content

Commit

Permalink
Add pre-commit config for linting and formatting (#77)
Browse files Browse the repository at this point in the history
* Add pre-commit config

* Add pre-commit section to README

* Use single line imports, add  `from __future__ import annotations`
  • Loading branch information
pederhan authored Mar 20, 2024
1 parent 7fbb4c7 commit d85d8b4
Show file tree
Hide file tree
Showing 28 changed files with 845 additions and 264 deletions.
1 change: 0 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,3 @@ jobs:
run: pip install hatch
- name: Run tests
run: hatch run test -vv

23 changes: 23 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: check-added-large-files
- repo: https://github.com/abravalheri/validate-pyproject
rev: v0.15
hooks:
- id: validate-pyproject
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.3.3
hooks:
# Run the linter.
- id: ruff
args: [ --fix ]
# Run the formatter.
- id: ruff-format
26 changes: 20 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ Here's an example of a source collector module that reads hosts from a file:
from typing import Any, Dict, List
from zabbix_auto_config.models import Host

DEFAULT_FILE = "hosts.json"
DEFAULT_FILE = "hosts.json"

def collect(*args: Any, **kwargs: Any) -> List[Host]:
filename = kwargs.get("filename", DEFAULT_FILE)
Expand Down Expand Up @@ -206,13 +206,13 @@ If `error_tolerance` number of errors occur within `error_duration` seconds, the

#### error_tolerance

`error_tolerance` (default: 0) is the maximum number of errors tolerated within `error_duration` seconds.
`error_tolerance` (default: 0) is the maximum number of errors tolerated within `error_duration` seconds.

#### error_duration

`error_duration` (default: 0) specifies the duration in seconds to track and log errors. This value should be at least equal to `error_tolerance * update_interval` to ensure correct error detection.
`error_duration` (default: 0) specifies the duration in seconds to track and log errors. This value should be at least equal to `error_tolerance * update_interval` to ensure correct error detection.

For instance, with an `error_tolerance` of 5 and an `update_interval` of 60, `error_duration` should be no less than 300 (5 * 60). However, it is advisable to choose a higher value to compensate for processing intervals between error occurrences and the subsequent error count checks, as well as any potential delays from the source collectors.
For instance, with an `error_tolerance` of 5 and an `update_interval` of 60, `error_duration` should be no less than 300 (5 * 60). However, it is advisable to choose a higher value to compensate for processing intervals between error occurrences and the subsequent error count checks, as well as any potential delays from the source collectors.

A useful guide is to set `error_duration` as `(error_tolerance + 1) * update_interval`, providing an additional buffer equivalent to one update interval.

Expand Down Expand Up @@ -249,7 +249,7 @@ def modify(host: Host) -> Host:

Any module that contains a function named `modify` which takes a `Host` and returns a `Host` is recognized as a host modifier module. Type annotations are optional, but recommended.

See the [`Host`](https://github.com/unioslo/zabbix-auto-config/blob/2b45f1cb7da0d46b8b218005ebbf751cb17f8793/zabbix_auto_config/models.py#L111-L123) class in `zabbix_auto_config/models.py` for the available fields that can be accessed and modified. One restriction applies: the `modify` function should _never_ modify the hostname of the host. Attempting to do so will result in an error.
See the [`Host`](https://github.com/unioslo/zabbix-auto-config/blob/2b45f1cb7da0d46b8b218005ebbf751cb17f8793/zabbix_auto_config/models.py#L111-L123) class in `zabbix_auto_config/models.py` for the available fields that can be accessed and modified. One restriction applies: the `modify` function should _never_ modify the hostname of the host. Attempting to do so will result in an error.

## Host inventory

Expand All @@ -275,7 +275,7 @@ Install the application with Hatch and enter the virtual environment:
hatch shell
```

The path to the current Hatch environment can always be found with:
The path to the current Hatch environment can always be found with:

```bash
hatch env find
Expand Down Expand Up @@ -307,3 +307,17 @@ If you just want to run tests without Hatch, you can do so by installing the dev
# Set up venv or similar ...
pip install .[test]
```

### Pre-commit

We use [pre-commit](https://pre-commit.com/) to manage pre-commit hooks. Install the hooks with:

```bash
pre-commit install
```

This will install the hooks in the `.git/hooks` directory. The hooks will run automatically when you commit changes. If you want to run the hooks manually, you can do so with:

```bash
pre-commit run --all-files
```
4 changes: 2 additions & 2 deletions config.sample.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ health_file = "/tmp/zac_health.json"
# File containing hostnames of hosts to add/remove when failsafe is reached.
failsafe_file = "/tmp/zac_failsafe.json"

# File that admin can create to signal manual approval of adding/removing hosts
# File that admin can create to signal manual approval of adding/removing hosts
# when failsafe is reached.
# The file is automatically deleted after changes are made.
failsafe_ok_file = "/tmp/zac_failsafe_ok"
Expand Down Expand Up @@ -83,7 +83,7 @@ exit_on_error = false # Disable source if it fails
# How long to wait before reactivating a disabled source
disable_duration = 3600 # Time in seconds to wait before reactivating a disabled source

# Any other options are passed as keyword arguments to the source collector's
# Any other options are passed as keyword arguments to the source collector's
# `collect()` function
kwarg_passed_to_source = "value" # extra fields are passed to the source module as kwargs
another_kwarg = "value2" # We can pass an arbitrary number of kwargs to the source module
Expand Down
13 changes: 12 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ dynamic = ["version"]
description = "Zabbix auto config - ZAC"
readme = "README.md"
requires-python = ">=3.8"
license = "MIT"
license.file = "LICENSE"
keywords = []
authors = [{ name = "Paal Braathen", email = "[email protected]" }]
maintainers = [{ name = "Peder Hovdan Andresen", email = "[email protected]" }]
Expand Down Expand Up @@ -57,3 +57,14 @@ exclude = ["/.github", "/tests", "/path"]

[tool.hatch.build.targets.wheel]
packages = ["zabbix_auto_config"]

[tool.ruff.lint]
extend-select = [
"I", # isort
]

[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"]
14 changes: 9 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from __future__ import annotations

import multiprocessing
import os
from pathlib import Path
from typing import Iterable, Type
from unittest.mock import MagicMock
import pytest
from typing import Iterable
from typing import Type
from unittest import mock
from unittest.mock import MagicMock

import pytest
import tomli

from zabbix_auto_config import models
Expand Down Expand Up @@ -110,6 +113,7 @@ def sample_config():
def config(sample_config: str) -> Iterable[models.Settings]:
yield models.Settings(**tomli.loads(sample_config))


@pytest.fixture
def hostgroup_map_file(tmp_path: Path) -> Iterable[Path]:
contents = """
Expand All @@ -118,8 +122,8 @@ def hostgroup_map_file(tmp_path: Path) -> Iterable[Path]:
# Example: <siteadm>:<host/user groupname>
#
#****************************************************************************************
# ATT: First letter will be capitilazed, leading and trailing spaces will be removed and
# spaces within the hostgroupname will be replaced with "-" by the script automatically
# ATT: First letter will be capitilazed, leading and trailing spaces will be removed and
# spaces within the hostgroupname will be replaced with "-" by the script automatically
#****************************************************************************************
#
[email protected]:Hostgroup-user1-primary
Expand Down
2 changes: 2 additions & 0 deletions tests/data/host_modifier_typed.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

from zabbix_auto_config.models import Host


Expand Down
3 changes: 3 additions & 0 deletions tests/data/host_modifier_untyped.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from __future__ import annotations


def modify(host):
if host.hostname == "bar.example.com":
host.properties.add("barry")
Expand Down
6 changes: 5 additions & 1 deletion tests/data/source_collector_typed.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
from typing import Any, List
from __future__ import annotations

from typing import Any
from typing import List

from zabbix_auto_config.models import Host

HOSTS = [
Expand Down
4 changes: 3 additions & 1 deletion tests/data/source_collector_untyped.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import zabbix_auto_config.models

HOSTS = [
Expand All @@ -20,4 +22,4 @@ def collect(*args, **kwargs):
if source:
host["properties"].append(source)
hosts.append(zabbix_auto_config.models.Host(**host))
return hosts
return hosts
6 changes: 4 additions & 2 deletions tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from __future__ import annotations

import logging
import tomli

import pytest
import tomli
from pydantic import ValidationError

import zabbix_auto_config.models as models


Expand Down Expand Up @@ -51,7 +54,6 @@ def test_sourcecollectorsettings_defaults():
assert settings.disable_duration == 3600



def test_sourcecollectorsettings_no_tolerance() -> None:
"""Setting no error tolerance will cause the error_duration to be set
to a non-zero value.
Expand Down
8 changes: 6 additions & 2 deletions tests/test_errcount.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
from __future__ import annotations

import datetime
import operator
import time
from typing import Callable

import pytest

from zabbix_auto_config.errcount import Error, RollingErrorCounter, get_td
from zabbix_auto_config.errcount import Error
from zabbix_auto_config.errcount import RollingErrorCounter
from zabbix_auto_config.errcount import get_td


def test_get_td():
Expand Down Expand Up @@ -136,4 +140,4 @@ def test_type_error(
]
# Comparison of Error with non-Error
for op in operators:
test_type_error(op, err1, "foo")
test_type_error(op, err1, "foo")
44 changes: 23 additions & 21 deletions tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from __future__ import annotations

import logging
from typing import Optional

import pytest
from pydantic import ValidationError
from zabbix_auto_config import models

from zabbix_auto_config import models

# NOTE: Do not test msg and ctx of Pydantic errors!
# They are not guaranteed to be stable between minor versions.
Expand Down Expand Up @@ -63,7 +66,6 @@ def test_duplicate_interface(invalid_hosts):
assert error["type"] == "assertion_error"



def test_invalid_importance(invalid_hosts):
host = find_host_by_hostname(invalid_hosts, "invalid-importance")
with pytest.raises(ValidationError) as exc_info:
Expand All @@ -76,7 +78,6 @@ def test_invalid_importance(invalid_hosts):
assert error["type"] == "greater_than_equal"



def test_host_merge(full_hosts):
"""Tests Host.merge()"""
host = find_host_by_hostname(full_hosts, "foo")
Expand Down Expand Up @@ -179,24 +180,25 @@ def test_zacsettings_log_level_serialize() -> None:
settings_json = settings.model_dump_json()
assert '"log_level":"INFO"' in settings_json


@pytest.mark.parametrize(
"timeout,expect",
[
(1, 1),
(60, 60),
(1234, 1234),
(0, None),
pytest.param(
-1,
None,
marks=pytest.mark.xfail(
reason="Timeout must be 0 or greater.",
strict=True,
raises=ValidationError,
),
id="-1"
)
]
"timeout,expect",
[
(1, 1),
(60, 60),
(1234, 1234),
(0, None),
pytest.param(
-1,
None,
marks=pytest.mark.xfail(
reason="Timeout must be 0 or greater.",
strict=True,
raises=ValidationError,
),
id="-1",
),
],
)
def test_zabbix_settings_timeout(timeout: int, expect: Optional[int]) -> None:
settings = models.ZabbixSettings(
Expand All @@ -207,4 +209,4 @@ def test_zabbix_settings_timeout(timeout: int, expect: Optional[int]) -> None:
dryrun=False,
timeout=timeout,
)
assert settings.timeout == expect
assert settings.timeout == expect
5 changes: 4 additions & 1 deletion tests/test_processing/test_sourcecollectorprocess.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from __future__ import annotations

import multiprocessing
import time
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
from zabbix_auto_config.models import Host, SourceCollectorSettings
from zabbix_auto_config.state import get_manager


Expand Down
Loading

0 comments on commit d85d8b4

Please sign in to comment.