Skip to content

Commit 104e906

Browse files
Merge pull request #629 from frankieramirez/feat/log-level-names
feat: Accept log level names, and deprecate --verbose
2 parents 164063f + 16fe65d commit 104e906

16 files changed

Lines changed: 264 additions & 68 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"comicarr": patch
3+
---
4+
5+
The log level can now be set by name as well as by number. `warning`, `info`, and `debug` work anywhere the number did — `--log-level debug` on the command line, `COMICARR_LOG_LEVEL=debug` in your compose file, or `LOG_LEVEL = debug` in `config.ini`. Numbers are unchanged, so nothing you already have needs editing, and capitalisation does not matter.
6+
7+
The names describe what each level actually does: level `0` is `warning` because it emits warnings and errors. It was previously described as "quiet", which suggested silence and was never true — turning the dial down has always kept failures visible.
8+
9+
Startup messages now name the level both ways, so it is obvious which setting produced it: `Log level 2 (debug) from startup argument overrides 1 (info) from the config file`.
10+
11+
`--verbose` and `-v` are now deprecated aliases for `--log-level debug`, joining `--quiet` and `-q` (aliases for `--log-level warning`). All four keep working and will continue to — they print a note pointing at `--log-level`, which is the one flag that sets the level directly.

CONTEXT.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ Who issued a Series' identifier: ComicVine, MangaDex (`md-` prefix), or MyAnimeL
2020

2121
The MangaDex UUID a manga Series polls for new chapters. MangaDex Series carry it in their ComicID; MyAnimeList Series supply metadata from MAL but keep the chapter source in `MangaDexID`, and have none until it is resolved.
2222

23+
## Log level
24+
25+
Comicarr's single verbosity dial, named by the severity it admits: `0` warning, `1` info, `2` debug. Level `0` means warnings and errors, not silence, which is why it is never called "quiet" — `--quiet` and `--verbose` are flag spellings, not level names.
26+
2327
## Support bundle
2428

2529
A downloadable archive of allowlisted diagnostic data, engineered for public issue attachment after operator review. If its contents appear sensitive, the operator shares it privately with maintainers instead; CarePackage is the legacy implementation name, not the user-facing term.

Comicarr.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,23 +91,26 @@ def main():
9191
parser.add_argument(
9292
"--log-level",
9393
dest="log_level",
94-
type=int,
9594
default=None,
96-
metavar="{0,1,2}",
95+
metavar="{0,1,2|warning,info,debug}",
9796
help=(
98-
"Logging verbosity: 0 warnings and errors, 1 normal, 2 everything. "
97+
"Logging verbosity: 0/warning warnings and errors, 1/info normal, 2/debug everything. "
9998
"Overrides COMICARR_LOG_LEVEL and the config file; omit it and those apply."
10099
),
101100
)
102101
parser.add_argument(
103-
"-v", "--verbose", action="store_true", default=False, help="Increase console logging verbosity"
102+
"-v",
103+
"--verbose",
104+
action="store_true",
105+
default=False,
106+
help="Deprecated alias for --log-level debug (everything)",
104107
)
105108
parser.add_argument(
106109
"-q",
107110
"--quiet",
108111
action="store_true",
109112
default=False,
110-
help="Deprecated alias for --log-level 0 (warnings and errors only)",
113+
help="Deprecated alias for --log-level warning (warnings and errors only)",
111114
)
112115
parser.add_argument("-d", "--daemon", action="store_true", default=False, help="Run as a daemon")
113116
parser.add_argument("-p", "--port", type=int, default=0, help="Force Comicarr to run on a specified port")
@@ -300,15 +303,25 @@ def main():
300303
# Startup args are the top of the precedence chain (args > COMICARR_LOG_LEVEL
301304
# > config), but only when one was actually passed: leaving this None is what
302305
# lets the environment and the config file be heard at all.
306+
#
307+
# `--log-level` accepts a number or a name; an unusable value supplies
308+
# nothing, so it falls through to the aliases and then to the layers below
309+
# rather than blocking the boot over a typo.
303310
comicarr.LOG_LEVEL = None
304311
if args_log_level is not None:
305-
comicarr.LOG_LEVEL = log_level_source.clamp_level(args_log_level)
306-
print("Log level set to %s by startup argument." % comicarr.LOG_LEVEL)
307-
elif args_verbose:
308-
print("Verbose/Debugging mode enabled...")
312+
argument_level, argument_notices = log_level_source.parse_level(
313+
args_log_level, log_level_source.SOURCE_ARGUMENT
314+
)
315+
for notice in argument_notices:
316+
print(notice)
317+
if argument_level is not None:
318+
comicarr.LOG_LEVEL = argument_level
319+
print("Log level set to %s by startup argument." % log_level_source.describe_level(argument_level))
320+
if comicarr.LOG_LEVEL is None and args_verbose:
321+
print("--verbose is deprecated; use --log-level debug. Log level set to 2 (debug).")
309322
comicarr.LOG_LEVEL = 2
310-
elif args_quiet:
311-
print("--quiet is deprecated; use --log-level 0. Logging warnings and errors only...")
323+
if comicarr.LOG_LEVEL is None and args_quiet:
324+
print("--quiet is deprecated; use --log-level warning. Log level set to 0 (warning).")
312325
comicarr.LOG_LEVEL = 0
313326

314327
if args_ignoreupdate:

comicarr/app/config/log_level.py

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
2. the `COMICARR_LOG_LEVEL` environment variable
1717
3. `LOG_LEVEL` in the config file (the Settings UI writes this one)
1818
19+
Each of them accepts the level in either notation -- `0`/`1`/`2` or
20+
`warning`/`info`/`debug` -- and the integer is what gets stored, whichever form
21+
was typed.
22+
1923
A source only counts when it *explicitly supplies* a value. That qualifier is
2024
the whole point: Docker used to pass `--quiet` on every start, which pinned the
2125
escape hatch permanently open and left an operator with no way to raise
@@ -38,6 +42,19 @@
3842
MIN_LEVEL = 0
3943
MAX_LEVEL = 2
4044

45+
# One name per level, and the threshold picks it: level 0 is `WARNING`, so it is
46+
# called `warning`. The tempting `quiet`/`normal`/`verbose` triple is not here on
47+
# purpose -- level 0 emits warnings and errors, and a dial labelled "quiet" is
48+
# the same lie #610 was about. `warn`, `error`, and `critical` are rejected too:
49+
# the first is a second name for one level, and the last two name a severity no
50+
# level can deliver on its own.
51+
LEVEL_NAMES = {"warning": 0, "info": 1, "debug": 2}
52+
NAME_FOR_LEVEL = {level: name for name, level in LEVEL_NAMES.items()}
53+
54+
# Every source accepts both notations, so every rejection describes both. One
55+
# string because the CLI notice and the Settings HTTP error must not drift.
56+
ACCEPTED_FORMS = "%s-%s or one of %s" % (MIN_LEVEL, MAX_LEVEL, ", ".join(LEVEL_NAMES))
57+
4158
# Named for the source, not the mechanism, because these strings are echoed to
4259
# the operator when the level is decided.
4360
SOURCE_ARGUMENT = "startup argument"
@@ -65,9 +82,24 @@ def clamp_level(level: int) -> int:
6582
return max(MIN_LEVEL, min(MAX_LEVEL, level))
6683

6784

85+
def describe_level(level: int) -> str:
86+
"""Render a level the way every operator-facing surface says it: `2 (debug)`.
87+
88+
The number is what an operator's `config.ini` and compose file contain; the
89+
name is what `--help` and the Settings dial show them. Saying both keeps the
90+
two notations from drifting apart in anyone's head.
91+
"""
92+
return "%s (%s)" % (level, NAME_FOR_LEVEL[clamp_level(level)])
93+
94+
6895
def parse_level(raw, origin: str) -> tuple[int | None, list[str]]:
6996
"""Read one source's value into a usable level.
7097
98+
Both notations are accepted from every source -- `2` and `debug` are the same
99+
instruction, so an operator who reads `Debug` on the Settings dial can type
100+
`--log-level debug` and have it work. Names are matched case-insensitively
101+
and never need clamping; a number does.
102+
71103
Returns `(None, notices)` when the source supplied nothing usable, so the
72104
caller falls through to the next layer rather than starting at a level
73105
nobody asked for. Out-of-range numbers are clamped rather than rejected: a
@@ -85,13 +117,16 @@ def parse_level(raw, origin: str) -> tuple[int | None, list[str]]:
85117
text = str(raw).strip()
86118
if not text:
87119
return None, notices
120+
named = LEVEL_NAMES.get(text.casefold())
121+
if named is not None:
122+
return named, notices
88123
try:
89124
parsed = int(text)
90125
except ValueError:
91-
return None, [f"Ignoring {origin}: {text!r} is not a number. Expected {MIN_LEVEL}-{MAX_LEVEL}."]
126+
return None, [f"Ignoring {origin}: {text!r} is not a log level. Expected {ACCEPTED_FORMS}."]
92127
clamped = clamp_level(parsed)
93128
if clamped != parsed:
94-
notices.append(f"Log level {parsed} from {origin} is out of range; using {clamped}.")
129+
notices.append(f"Log level {parsed} from {origin} is out of range; using {describe_level(clamped)}.")
95130
return clamped, notices
96131

97132

@@ -124,8 +159,10 @@ def resolve_startup_log_level(
124159
# Say what lost, so an operator who edits the Settings dial and sees nothing
125160
# change has the reason in front of them rather than in the source.
126161
overridden = [
127-
f"{other_level} from {other_source}" for other_level, other_source in supplied[1:] if other_level != level
162+
f"{describe_level(other_level)} from {other_source}"
163+
for other_level, other_source in supplied[1:]
164+
if other_level != level
128165
]
129166
if overridden:
130-
notices.append(f"Log level {level} from {source} overrides {', '.join(overridden)}.")
167+
notices.append(f"Log level {describe_level(level)} from {source} overrides {', '.join(overridden)}.")
131168
return LogLevelResolution(level=level, source=source, notices=notices)

comicarr/app/system/service.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
from comicarr.app.acquisition.models import DispatchState
4545
from comicarr.app.common.dates import normalize_utc_datetime
4646
from comicarr.app.common.redaction import redact_sensitive_text
47-
from comicarr.app.config.log_level import MAX_LEVEL, MIN_LEVEL, SOURCE_SETTINGS, parse_level
47+
from comicarr.app.config.log_level import ACCEPTED_FORMS, SOURCE_SETTINGS, parse_level
4848
from comicarr.app.config.registry import (
4949
readable_keys,
5050
scheduler_job_intervals,
@@ -407,16 +407,18 @@ def update_config(ctx, key_values):
407407
level_notices = []
408408
if "LOG_LEVEL" in filtered:
409409
# Read by the same rules as every other source of the level, so a value
410-
# typed into Settings behaves like one passed on the command line:
411-
# out of range clamps, non-numeric is refused. A startup source is
412-
# clamped rather than rejected because refusing to boot helps nobody;
413-
# an HTTP request can simply be told it was wrong, and persisting
414-
# garbage would leave the level silently ignored at the next start.
410+
# typed into Settings behaves like one passed on the command line: both
411+
# notations are accepted, out of range clamps, and anything else is
412+
# refused. A startup source is clamped rather than rejected because
413+
# refusing to boot helps nobody; an HTTP request can simply be told it
414+
# was wrong, and persisting garbage would leave the level silently
415+
# ignored at the next start. Whichever form arrives, an integer is what
416+
# gets stored.
415417
level, level_notices = parse_level(filtered["LOG_LEVEL"], SOURCE_SETTINGS)
416418
if level is None:
417419
return {
418420
"success": False,
419-
"error": "LOG_LEVEL must be a number between %s and %s" % (MIN_LEVEL, MAX_LEVEL),
421+
"error": "LOG_LEVEL must be %s" % ACCEPTED_FORMS,
420422
}
421423
filtered["LOG_LEVEL"] = level
422424

docs/architecture/logging-levels.md

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,31 @@ the logger and to every sink it feeds. There is no second dial.
88

99
| Level | Threshold | Console | `comicarr.log` | Web UI log list |
1010
| --- | --- | --- | --- | --- |
11-
| `0`quiet | `WARNING` | warnings and errors | warnings and errors | warnings and errors |
12-
| `1`normal (default) | `INFO` | info and above | info and above | info and above |
13-
| `2`verbose | `DEBUG` | everything | everything | everything |
11+
| `0``warning` | `WARNING` | warnings and errors | warnings and errors | warnings and errors |
12+
| `1``info` (default) | `INFO` | info and above | info and above | info and above |
13+
| `2``debug` | `DEBUG` | everything | everything | everything |
1414

1515
Levels below `0` clamp to `WARNING`; above `2` clamp to `DEBUG`.
1616

17+
### The names are determined, not chosen
18+
19+
Each level *is* a stdlib threshold, so the threshold names it. That rules out
20+
the obvious `quiet` / `normal` / `verbose` triple, which this document used
21+
until #620: level `0` emits warnings and errors, and calling it "quiet" is the
22+
same class of lie as #610 — a control describing behaviour the process does not
23+
have. An operator who reads "quiet" and hears "silence" will turn the dial down
24+
and believe failures stopped.
25+
26+
There is exactly one name per level. `warn` is refused as a second spelling of
27+
one level; `error` and `critical` are refused because no level delivers them —
28+
`--log-level error` could only mean "warnings and errors", which is level `0`
29+
under a name that promises something narrower.
30+
31+
`quiet` and `verbose` survive only as the flag spellings `--quiet` and
32+
`--verbose`, which is a different thing from a level name and is covered below.
33+
Neither is accepted as a *value*: `--log-level` was `type=int` before #620, so
34+
nobody could ever have typed them and there is no back-compatibility to keep.
35+
1736
`logger.threshold_for_level()` is the only place this mapping lives, and
1837
`logger.current_log_level()` is the only supported way to ask what the dial is
1938
currently set to. Nothing else may branch on verbosity.
@@ -31,6 +50,18 @@ Three sources, highest priority first:
3150

3251
If none of them supplies a value, the level is `1`.
3352

53+
**Every source accepts both notations.** `--log-level debug`,
54+
`COMICARR_LOG_LEVEL=debug`, `LOG_LEVEL = debug` in `config.ini`, and a `"debug"`
55+
sent to the Settings endpoint all mean level `2`. Matching is
56+
case-insensitive and tolerates surrounding whitespace. One grammar everywhere is
57+
the point: an operator who reads `Debug` on the Settings dial and types
58+
`--log-level debug` must not meet an error, and a source that quietly accepted
59+
less than its neighbours would only ever be discovered by tripping over it.
60+
61+
**The stored value is always the integer.** A name is normalised by
62+
`parse_level` at the boundary and never reaches `config.ini`, so `LOG_LEVEL`
63+
stays the `int` the registry declares and the generated frontend types expect.
64+
3465
**A source counts only when it explicitly supplies a value.** That qualifier is
3566
the whole rule, and it is what #610 got wrong: the Docker entrypoint passed
3667
`--quiet` on every start, so the top of the chain was permanently occupied and
@@ -46,7 +77,13 @@ overrode.
4677

4778
Values outside `0``2` are clamped rather than rejected: a compose file asking
4879
for `3` wants maximum verbosity, and refusing to boot over it helps nobody. A
49-
non-numeric value is ignored with a notice, and the next source down is used.
80+
value that is neither a number nor one of the three names is ignored with a
81+
notice, and the next source down is used.
82+
83+
Operator-facing text names the level in both notations — `Log level 2 (debug)
84+
from startup argument overrides 1 (info) from the config file`. The number is
85+
what an operator's config and compose files contain; the name is what `--help`
86+
and the Settings dial show them. `describe_level()` is the single renderer.
5087

5188
`COMICARR_LOG_LEVEL` is a deliberate one-off for this one key. Comicarr does not
5289
read the environment for any other setting, and a general `COMICARR_<KEY>`
@@ -69,22 +106,37 @@ is the realistic case), the save still reports success and the failure is
69106
logged; the level applies at the next start.
70107

71108
The value is read by `parse_level`, exactly as the three startup sources are,
72-
so a level typed into Settings clamps to range the same way. It differs on one
73-
point: a non-numeric value is *refused* with an error rather than ignored. A
74-
startup source has a layer beneath it to fall through to, and an HTTP request
75-
has somewhere to put the complaint — persisting `"verbose"` would leave the
76-
operator's setting silently discarded at the next start.
109+
so a level typed into Settings clamps to range the same way and accepts the same
110+
two notations. It differs on one point: an unrecognised value is *refused* with
111+
an error rather than ignored. A startup source has a layer beneath it to fall
112+
through to, and an HTTP request has somewhere to put the complaint — persisting
113+
`"loud"` would leave the operator's setting silently discarded at the next
114+
start. The rejection names both accepted forms, from the same `ACCEPTED_FORMS`
115+
string the startup notice uses, so the two can never describe different rules.
77116

78117
On the next start the chain runs again, so a startup argument or
79118
`COMICARR_LOG_LEVEL` will override what Settings saved. That is the documented
80119
precedence, and it is why the winning source announces what it overrode.
81120

121+
The dial itself shows number, name, and consequence together — `0 · Warning —
122+
warnings and errors` — rather than a bare one-word label. The number is what the
123+
operator's config file contains, the name is what the CLI accepts, and the
124+
consequence is what stops the next "I set it to 0 and expected silence". This
125+
supersedes the `quiet / normal / verbose` labels locked while designing the
126+
surface; the layout of that design is unaffected.
127+
82128
### `--quiet` and `--verbose`
83129

84-
`--quiet` is a deprecated alias for `--log-level 0` and prints a notice saying
85-
so. It stays because it is in existing compose files and systemd units; deleting
86-
it would break them for a cosmetic gain. `--verbose` maps to level `2`. When
87-
more than one is passed, `--log-level` wins, then `--verbose`, then `--quiet`.
130+
Both are deprecated aliases — `--quiet` for `--log-level warning`, `--verbose`
131+
for `--log-level debug` — and both print a notice saying so. Both keep their
132+
short forms `-q` and `-v`, and neither has a removal date: they are in existing
133+
compose files and systemd units, and deleting them would break those for a
134+
cosmetic gain. `--log-level` is the only flag that sets the dial directly.
135+
136+
When more than one is passed, `--log-level` wins, then `--verbose`, then
137+
`--quiet`. A `--log-level` whose value is unusable supplies *nothing*, so it
138+
loses to an alias that was also passed — same rule as the precedence chain, one
139+
level down.
88140

89141
### The two helpers disagree about `None`, on purpose
90142

frontend/package-lock.json

Lines changed: 3 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/src/components/prototype/PrototypeSwitcher.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,3 @@ export function PrototypeSwitcher({
109109
</div>
110110
);
111111
}
112-

0 commit comments

Comments
 (0)