peter
0cff964ff2
25 literal names covered 34 modules. plex/client.py, plex/stream.py, plex/subtitles.py, plex/sync.py and two Plex route modules all called themselves `siftlode.plex`; downloads/pipeline.py called itself `siftlode.worker`; routes/channels.py called itself `siftlode.api`. The `logger` column is in every stored row so E22.6's console can filter on it, and so an operator can pin a level to one part of the system — neither works when six modules share a name. `logs.get_logger(__name__)` derives it: app/plex/sync.py -> siftlode.plex.sync. One name per module, and it cannot drift from the file it sits in. tests/test_logger_names.py pins the derivation, that everything stays under the one `siftlode` tree the handler and the tests attach to, and that no module hand-rolls a name again unless the line says why (`# logger-name-ok: <why>`). Two do: get_logger's own call, and `siftlode.http`, which is named for the traffic it carries rather than the module it lives in — it replaced uvicorn's access log and is what an operator pins when they want request lines and nothing else. app/scheduler.py called its logger `logger`; every other module calls it `log`. ruff gains LOG, G, T20, TRY400, TRY401 and S112. Four findings cleared with them: worker.py logged `str(exc)[:120]` where the traceback was the whole question, and three prints/f-strings that are deliberate now say so on the line. S110 and BLE001 stay off until S3 — 23 handlers are outstanding and each is a judgement about one of them; enabling the rules first would just mean 23 blanket noqas.
80 lines
3.3 KiB
Python
80 lines
3.3 KiB
Python
"""E22.5 S2 — every module's logger is named after the module.
|
|
|
|
The names used to be literals, and 25 of them covered 34 modules: `plex/client.py`, `plex/stream.py`,
|
|
`plex/subtitles.py`, `plex/sync.py` and two route modules all called themselves `siftlode.plex`, and
|
|
`downloads/pipeline.py` called itself `siftlode.worker`. A level pinned per logger could not separate
|
|
them, and neither can E22.6's console filter — the `logger` field is in every stored row precisely so
|
|
it can be filtered on.
|
|
|
|
So the name is derived, not written: `logs.get_logger(__name__)`. These cases pin the derivation and
|
|
the rule, because the rule is only worth having if a hand-rolled name fails the gate.
|
|
"""
|
|
|
|
import ast
|
|
import logging
|
|
import pathlib
|
|
|
|
import pytest
|
|
|
|
from app import logs
|
|
|
|
APP = pathlib.Path(logs.__file__).resolve().parent
|
|
_ESCAPE = "logger-name-ok"
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("module", "expected"),
|
|
[
|
|
("app.plex.sync", "siftlode.plex.sync"),
|
|
("app.plex.stream", "siftlode.plex.stream"),
|
|
("app.routes.channels", "siftlode.routes.channels"),
|
|
("app.worker", "siftlode.worker"),
|
|
# Not a module of ours (a script, a doctest): it lands on the root of our tree rather than
|
|
# inventing `siftlode.__main__`, which nothing would think to filter on.
|
|
("__main__", "siftlode"),
|
|
],
|
|
)
|
|
def test_the_name_follows_the_module(module, expected):
|
|
assert logs.get_logger(module).name == expected
|
|
|
|
|
|
def test_every_derived_logger_stays_under_the_one_tree():
|
|
"""`configure_logging` hangs the level and the handler off the root, and the tests attach their
|
|
sink to `siftlode` — a name outside that tree would be invisible to both."""
|
|
for module in ("app.main", "app.plex.sync", "app.routes.plex.people", "app"):
|
|
assert logs.get_logger(module).name.startswith("siftlode")
|
|
|
|
|
|
def test_the_module_logger_is_reachable_by_its_derived_name():
|
|
"""Not a new logger object per call: the stdlib registry is keyed by name, and an operator
|
|
pinning `siftlode.plex.sync` has to reach the same instance the module logs through."""
|
|
from app.plex import sync
|
|
|
|
assert sync.log is logging.getLogger("siftlode.plex.sync")
|
|
|
|
|
|
def test_no_module_hand_rolls_a_logger_name():
|
|
offenders = []
|
|
for path in sorted(APP.rglob("*.py")):
|
|
src = path.read_text(encoding="utf-8")
|
|
lines = src.splitlines()
|
|
for node in ast.walk(ast.parse(src)):
|
|
if not isinstance(node, ast.Call):
|
|
continue
|
|
func = node.func
|
|
if not (isinstance(func, ast.Attribute) and func.attr == "getLogger"):
|
|
continue
|
|
# A no-argument `logging.getLogger()` is the ROOT logger — level plumbing, not a
|
|
# module's own logger, and nothing to name.
|
|
if not node.args:
|
|
continue
|
|
start = max(0, node.lineno - 2) # the call's lines, plus the one above it
|
|
span = " ".join(lines[start : (node.end_lineno or node.lineno)])
|
|
if _ESCAPE in span:
|
|
continue
|
|
offenders.append(f"{path.relative_to(APP)}:{node.lineno}")
|
|
assert offenders == [], (
|
|
"a module names its own logger; use `logs.get_logger(__name__)`, or say why the name is "
|
|
f"not the module's with `# {_ESCAPE}: <why>` — {offenders}"
|
|
)
|