logging rethink

* well lets replace f-strings by %s as it is originally recommended
* use syslog handler by default

.pylintrc

@@ -149,7 +149,6 @@ disable=print-statement,
         too-few-public-methods,
         too-many-instance-attributes,
         broad-except,
-        logging-fstring-interpolation,
         too-many-ancestors,
         fixme,
         too-many-arguments,

package/archlinux/ahriman.tmpfiles

@@ -1,2 +1 @@
-d /var/lib/ahriman 0775 ahriman log
 d /var/log/ahriman 0755 ahriman ahriman

package/etc/ahriman.ini.d/logging.ini

@@ -2,10 +2,10 @@
 keys = root,builder,build_details,http
 
 [handlers]
-keys = console_handler,build_file_handler,file_handler,http_handler
+keys = console_handler,build_file_handler,file_handler,http_handler,syslog_handler
 
 [formatters]
-keys = generic_format
+keys = generic_format,syslog_format
 
 [handler_console_handler]
 class = StreamHandler
@@ -31,29 +31,39 @@ level = DEBUG
 formatter = generic_format
 args = ("/var/log/ahriman/http.log", "a", 20971520, 20)
 
+[handler_syslog_handler]
+class = logging.handlers.SysLogFileHandler
+level = DEBUG
+formatter = syslog_format
+args = ("/dev/log",)
+
 [formatter_generic_format]
 format = [%(levelname)s %(asctime)s] [%(filename)s:%(lineno)d] [%(funcName)s]: %(message)s
 datefmt =
 
+[formatter_syslog_format]
+format = [%(levelname)s] [%(filename)s:%(lineno)d] [%(funcName)s]: %(message)s
+datefmt =
+
 [logger_root]
 level = DEBUG
-handlers = file_handler
+handlers = syslog_handler
 qualname = root
 
 [logger_builder]
 level = DEBUG
-handlers = file_handler
+handlers = syslog_handler
 qualname = builder
 propagate = 0
 
 [logger_build_details]
 level = DEBUG
-handlers = build_file_handler
+handlers = syslog_handler
 qualname = build_details
 propagate = 0
 
 [logger_http]
 level = DEBUG
-handlers = http_handler
+handlers = syslog_handler
 qualname = http
 propagate = 0

src/ahriman/application/application.py

@@ -185,7 +185,7 @@ class Application:
                 continue
             for archive in package.packages.values():
                 if archive.filepath is None:
-                    self.logger.warning(f"filepath is empty for {package.base}")
+                    self.logger.warning("filepath is empty for %s", package.base)
                     continue  # avoid mypy warning
                 src = self.repository.paths.repository / archive.filepath
                 dst = self.repository.paths.packages / archive.filepath
@@ -224,6 +224,6 @@ class Application:
         # process manual packages
         tree = Tree.load(updates)
         for num, level in enumerate(tree.levels()):
-            self.logger.info(f"processing level #{num} {[package.base for package in level]}")
+            self.logger.info("processing level #%i %s", num, [package.base for package in level])
             packages = self.repository.process_build(level)
             process_update(packages)

src/ahriman/application/lock.py

@@ -94,8 +94,10 @@ class Lock:
         """
         status = self.reporter.get_internal()
         if status.version is not None and status.version != version.__version__:
-            logging.getLogger("root").warning(f"status watcher version mismatch, "
+            logging.getLogger("root").warning(
-                                              f"our {version.__version__}, their {status.version}")
+                "status watcher version mismatch, our %s, their %s",
+                version.__version__,
+                status.version)
 
     def check_user(self) -> None:
         """

src/ahriman/core/build_tools/task.py

@@ -99,7 +99,7 @@ class Task:
         command.extend(self.archbuild_flags)
         command.extend(["--"] + self.makechrootpkg_flags)
         command.extend(["--"] + self.makepkg_flags)
-        self.logger.info(f"using {command} for {self.package.base}")
+        self.logger.info("using %s for %s", command, self.package.base)
 
         Task._check_output(
             *command,

src/ahriman/core/repository/executor.py

@@ -61,7 +61,7 @@ class Executor(Cleaner):
                 build_single(single)
             except Exception:
                 self.reporter.set_failed(single.base)
-                self.logger.exception(f"{single.base} ({self.architecture}) build exception")
+                self.logger.exception("%s (%s) build exception", single.base, self.architecture)
         self.clear_build()
 
         return self.packages_built()
@@ -76,7 +76,7 @@ class Executor(Cleaner):
             try:
                 self.repo.remove(package, fn)
             except Exception:
-                self.logger.exception(f"could not remove {package}")
+                self.logger.exception("could not remove %s", package)
 
         requested = set(packages)
         for local in self.packages():
@@ -132,7 +132,7 @@ class Executor(Cleaner):
         """
         def update_single(fn: Optional[str], base: str) -> None:
             if fn is None:
-                self.logger.warning(f"received empty package name for base {base}")
+                self.logger.warning("received empty package name for base %s", base)
                 return  # suppress type checking, it never can be none actually
             # in theory it might be NOT packages directory, but we suppose it is
             full_path = self.paths.packages / fn
@@ -150,7 +150,7 @@ class Executor(Cleaner):
                 local = Package.load(filename, self.pacman, self.aur_url)
                 updates.setdefault(local.base, local).packages.update(local.packages)
             except Exception:
-                self.logger.exception(f"could not load package from {filename}")
+                self.logger.exception("could not load package from %s", filename)
 
         for local in updates.values():
             try:
@@ -159,7 +159,7 @@ class Executor(Cleaner):
                 self.reporter.set_success(local)
             except Exception:
                 self.reporter.set_failed(local.base)
-                self.logger.exception(f"could not process {local.base}")
+                self.logger.exception("could not process %s", local.base)
         self.clear_packages()
 
         return self.repo.repo_path

src/ahriman/core/repository/repository.py

@@ -42,7 +42,7 @@ class Repository(Executor, UpdateHandler):
                 local = Package.load(full_path, self.pacman, self.aur_url)
                 result.setdefault(local.base, local).packages.update(local.packages)
             except Exception:
-                self.logger.exception(f"could not load package from {full_path}")
+                self.logger.exception("could not load package from %s", full_path)
                 continue
         return list(result.values())
 

src/ahriman/core/repository/update_handler.py

@@ -59,7 +59,7 @@ class UpdateHandler(Cleaner):
                     result.append(remote)
             except Exception:
                 self.reporter.set_failed(local.base)
-                self.logger.exception(f"could not load remote package {local.base}")
+                self.logger.exception("could not load remote package %s", local.base)
                 continue
 
         return result
@@ -81,7 +81,7 @@ class UpdateHandler(Cleaner):
                 else:
                     self.reporter.set_pending(local.base)
             except Exception:
-                self.logger.exception(f"could not add package from {fn}")
+                self.logger.exception("could not add package from %s", fn)
         self.clear_manual()
 
         return result

src/ahriman/core/sign/gpg.py

@@ -104,7 +104,7 @@ class GPG:
             })
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not download key {key} from {server}: {exception_response_text(e)}")
+            self.logger.exception("could not download key %s from %s: %s", key, server, exception_response_text(e))
             raise
         return response.text
 
@@ -142,7 +142,7 @@ class GPG:
             return [path]
         key = self.configuration.get("sign", f"key_{base}", fallback=self.default_key)
         if key is None:
-            self.logger.error(f"no default key set, skip package {path} sign")
+            self.logger.error("no default key set, skip package %s sign", path)
             return [path]
         return self.process(path, key)
 

src/ahriman/core/status/watcher.py

@@ -90,7 +90,7 @@ class Watcher:
             try:
                 parse_single(item)
             except Exception:
-                self.logger.exception(f"cannot parse item f{item} to package")
+                self.logger.exception("cannot parse item %s to package", item)
 
     def _cache_save(self) -> None:
         """

src/ahriman/core/status/web_client.py

@@ -84,9 +84,9 @@ class WebClient(Client):
             response = requests.post(self._package_url(package.base), json=payload)
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not add {package.base}: {exception_response_text(e)}")
+            self.logger.exception("could not add %s: %s", package.base, exception_response_text(e))
         except Exception:
-            self.logger.exception(f"could not add {package.base}")
+            self.logger.exception("could not add %s", package.base)
 
     def get(self, base: Optional[str]) -> List[Tuple[Package, BuildStatus]]:
         """
@@ -104,9 +104,9 @@ class WebClient(Client):
                 for package in status_json
             ]
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not get {base}: {exception_response_text(e)}")
+            self.logger.exception("could not get %s: %s", base, exception_response_text(e))
         except Exception:
-            self.logger.exception(f"could not get {base}")
+            self.logger.exception("could not get %s", base)
         return []
 
     def get_internal(self) -> InternalStatus:
@@ -121,7 +121,7 @@ class WebClient(Client):
             status_json = response.json()
             return InternalStatus.from_json(status_json)
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not get web service status: {exception_response_text(e)}")
+            self.logger.exception("could not get web service status: %s", exception_response_text(e))
         except Exception:
             self.logger.exception("could not get web service status")
         return InternalStatus()
@@ -138,7 +138,7 @@ class WebClient(Client):
             status_json = response.json()
             return BuildStatus.from_json(status_json)
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not get service status: {exception_response_text(e)}")
+            self.logger.exception("could not get service status: %s", exception_response_text(e))
         except Exception:
             self.logger.exception("could not get service status")
         return BuildStatus()
@@ -152,9 +152,9 @@ class WebClient(Client):
             response = requests.delete(self._package_url(base))
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not delete {base}: {exception_response_text(e)}")
+            self.logger.exception("could not delete %s: %s", base, exception_response_text(e))
         except Exception:
-            self.logger.exception(f"could not delete {base}")
+            self.logger.exception("could not delete %s", base)
 
     def update(self, base: str, status: BuildStatusEnum) -> None:
         """
@@ -168,9 +168,9 @@ class WebClient(Client):
             response = requests.post(self._package_url(base), json=payload)
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not update {base}: {exception_response_text(e)}")
+            self.logger.exception("could not update %s: %s", base, exception_response_text(e))
         except Exception:
-            self.logger.exception(f"could not update {base}")
+            self.logger.exception("could not update %s", base)
 
     def update_self(self, status: BuildStatusEnum) -> None:
         """
@@ -183,6 +183,6 @@ class WebClient(Client):
             response = requests.post(self._ahriman_url(), json=payload)
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
-            self.logger.exception(f"could not update service status: {exception_response_text(e)}")
+            self.logger.exception("could not update service status: %s", exception_response_text(e))
         except Exception:
             self.logger.exception("could not update service status")

tests/testresources/core/logging.ini

@@ -2,10 +2,10 @@
 keys = root,builder,build_details,http
 
 [handlers]
-keys = console_handler,build_file_handler,file_handler,http_handler
+keys = console_handler,build_file_handler,file_handler,http_handler,syslog_handler
 
 [formatters]
-keys = generic_format
+keys = generic_format,syslog_format
 
 [handler_console_handler]
 class = StreamHandler
@@ -31,29 +31,39 @@ level = DEBUG
 formatter = generic_format
 args = ("/var/log/ahriman/http.log", "a", 20971520, 20)
 
+[handler_syslog_handler]
+class = logging.handlers.SysLogFileHandler
+level = DEBUG
+formatter = syslog_format
+args = ("/dev/log",)
+
 [formatter_generic_format]
 format = [%(levelname)s %(asctime)s] [%(filename)s:%(lineno)d] [%(funcName)s]: %(message)s
 datefmt =
 
+[formatter_syslog_format]
+format = [%(levelname)s] [%(filename)s:%(lineno)d] [%(funcName)s]: %(message)s
+datefmt =
+
 [logger_root]
 level = DEBUG
-handlers = file_handler
+handlers = syslog_handler
 qualname = root
 
 [logger_builder]
 level = DEBUG
-handlers = file_handler
+handlers = syslog_handler
 qualname = builder
 propagate = 0
 
 [logger_build_details]
 level = DEBUG
-handlers = build_file_handler
+handlers = syslog_handler
 qualname = build_details
 propagate = 0
 
 [logger_http]
 level = DEBUG
-handlers = http_handler
+handlers = syslog_handler
 qualname = http
 propagate = 0