From e4c3bf12bb28720f1616022747c38736188f1d81 Mon Sep 17 00:00:00 2001
From: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com>
Date: Thu, 21 Mar 2024 11:14:28 +0100
Subject: [PATCH] Add Ruff logging rules (#78)

* Add ruff logging rules

* Add LOG ruff rule
---
 pyproject.toml                   |  4 ++++
 tests/test_models.py             |  4 ++--
 zabbix_auto_config/processing.py | 14 +++++++-------
 zabbix_auto_config/utils.py      | 15 ++++++---------
 4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 7896f0b..61d892f 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -60,7 +60,11 @@ packages = ["zabbix_auto_config"]
 
 [tool.ruff.lint]
 extend-select = [
+    "G", # flake8-logging-format
     "I", # isort
+    "LOG", # flake8-logging
+    "PLE1205", # pylint (too many logging args)
+    "PLE1206", # pylint (too few logging args)
 ]
 
 [tool.ruff.lint.isort]
diff --git a/tests/test_models.py b/tests/test_models.py
index 734d0b9..69c615b 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -125,7 +125,7 @@ def test_host_merge_invalid(full_hosts):
         ["notset", logging.NOTSET],
         ["debug", logging.DEBUG],
         ["info", logging.INFO],
-        ["warn", logging.WARN],
+        ["warn", logging.WARN],  # noqa: LOG009 # we need to test the deprecated name
         ["warning", logging.WARNING],
         ["error", logging.ERROR],
         ["fatal", logging.FATAL],
@@ -149,7 +149,7 @@ def test_zacsettings_log_level_str(level: str, expect: int, upper: bool) -> None
         [0, logging.NOTSET],
         [10, logging.DEBUG],
         [20, logging.INFO],
-        [30, logging.WARN],
+        [30, logging.WARN],  # noqa: LOG009 # we need to test the deprecated name
         [30, logging.WARNING],
         [40, logging.ERROR],
         [50, logging.FATAL],
diff --git a/zabbix_auto_config/processing.py b/zabbix_auto_config/processing.py
index 83181cc..f010ddc 100644
--- a/zabbix_auto_config/processing.py
+++ b/zabbix_auto_config/processing.py
@@ -335,10 +335,10 @@ def get_current_source_hosts(
                 host = models.Host(**result[0])
             except ValidationError as e:
                 # TODO: ensure this actually identifies the faulty host
-                logging.exception(f"Invalid host in source hosts table: {e}")
+                logging.exception("Invalid host in source hosts table: %s", e)
             except Exception as e:
                 logging.exception(
-                    f"Error when parsing host from source hosts table: {e}"
+                    "Error when parsing host from source hosts table: %s", e
                 )
             else:
                 hosts[host.hostname] = host
@@ -524,10 +524,10 @@ def get_source_hosts(self, cursor: "Cursor") -> Dict[str, List[models.Host]]:
                 host_model = models.Host(**host[0])
             except ValidationError as e:
                 # TODO: ensure this actually identifies the faulty host
-                logging.exception(f"Invalid host in source hosts table: {e}")
+                logging.exception("Invalid host in source hosts table: %s", e)
             except Exception as e:
                 logging.exception(
-                    f"Error when parsing host from source hosts table: {e}"
+                    "Error when parsing host from source hosts table: %s", e
                 )
             else:
                 source_hosts[host_model.hostname].append(host_model)
@@ -541,9 +541,9 @@ def get_hosts(self, cursor: "Cursor") -> Dict[str, models.Host]:
                 host_model = models.Host(**host[0])
             except ValidationError as e:
                 # TODO: ensure this log actually identifies the faulty host
-                logging.exception(f"Invalid host in hosts table: {e}")
+                logging.exception("Invalid host in hosts table: %s", e)
             except Exception as e:
-                logging.exception(f"Error when parsing host from hosts table: {e}")
+                logging.exception("Error when parsing host from hosts table: %s", e)
             else:
                 hosts[host_model.hostname] = host_model
         return hosts
@@ -590,7 +590,7 @@ def merge_sources(self):
                 host = hosts.get(hostname)
                 if not source_hosts:
                     logging.warning(
-                        f"Host '{hostname}' not found in source hosts table"
+                        "Host '%s' not found in source hosts table", hostname
                     )
                     continue
 
diff --git a/zabbix_auto_config/utils.py b/zabbix_auto_config/utils.py
index fc76ca9..33dab6f 100644
--- a/zabbix_auto_config/utils.py
+++ b/zabbix_auto_config/utils.py
@@ -69,17 +69,16 @@ def read_map_file(path: Union[str, Path]) -> Dict[str, List[str]]:
                     )
             except ValueError:
                 logging.warning(
-                    "Invalid format at line {lineno} in map file '{path}'. Expected 'key:value', got '{line}'.".format(
-                        lineno=lineno, path=path, line=line
-                    ),
+                    "Invalid format at line %d in map file '%s'. Expected 'key:value', got '%s'.",
+                    lineno,
+                    path,
+                    line,
                 )
                 continue
 
             if key in _map:
                 logging.warning(
-                    "Duplicate key {key} at line {lineno} in map file '{path}'.".format(
-                        key=key, lineno=lineno, path=path
-                    )
+                    "Duplicate key %s at line %d in map file '%s'.", key, lineno, path
                 )
                 _map[key].extend(values)
             else:
@@ -90,9 +89,7 @@ def read_map_file(path: Union[str, Path]) -> Dict[str, List[str]]:
         values_dedup = list(dict.fromkeys(values))  # dict.fromkeys() guarantees order
         if len(values) != len(values_dedup):
             logging.warning(
-                "Ignoring duplicate values for key '{key}' in map file '{path}'.".format(
-                    key=key, path=path
-                )
+                "Ignoring duplicate values for key '%s' in map file '%s'.", key, path
             )
         _map[key] = values_dedup
     return _map