Skip to content

Commit 79eb533

Browse files
Merge pull request #5076 from springfall2008/feat/log-rotation-count
feat(log): make the number of retained log files configurable
2 parents b422eb7 + 314777e commit 79eb533

10 files changed

Lines changed: 392 additions & 15 deletions

File tree

‎apps/predbat/config.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2523,6 +2523,7 @@
25232523
"db_primary": {"type": "boolean"},
25242524
"threads": {"type": "string|integer", "allowed": ["auto", 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]},
25252525
"prediction_kernel_enable": {"type": "boolean"},
2526+
"log_count": {"type": "integer", "min": 2, "max": 100},
25262527
"ha_url": {"type": "string", "empty": False},
25272528
"ha_key": {"type": "string", "empty": False},
25282529
"load_filter_threshold": {"type": "integer"},

‎apps/predbat/config/apps.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ pred_bat:
2424
# Can be auto for automatic, 0 for off or values 1-N for a fixed number
2525
threads: auto
2626

27+
# Number of log files to keep, including the live predbat.log (2-100, default 10)
28+
# Rotated logs are predbat.01.log .. predbat.99.log
29+
#log_count: 10
30+
2731
# If you are using Predbat outside of HA then set the HA URL and Key (long lived access token here)
2832
#ha_url: 'http://homeassistant.local:8123'
2933
#ha_key: !secret ha_key

‎apps/predbat/hass.py‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def write_git_version_marker():
3636
write_git_version_marker()
3737

3838
import predbat
39-
from utils import load_apps_yaml
39+
from utils import load_apps_yaml, predbat_log_count, predbat_log_name, rotate_predbat_logs
4040
import time
4141
from datetime import datetime, timedelta
4242
from multiprocessing import set_start_method
@@ -188,21 +188,17 @@ def log(self, msg, quiet=True):
188188
if not quiet or msg_lower.startswith("error") or msg_lower.startswith("warn") or msg_lower.startswith("info"):
189189
print(message, end="")
190190

191-
# maximum number of historic logfiles to retain
192-
max_logs = 9
191+
# Total logfiles to keep including the live one, so max_logs rotated copies.
192+
max_logs = predbat_log_count(self.args) - 1
193193

194194
log_size = self.logfile.tell()
195195
if log_size > 10000000 and threading.current_thread() is threading.main_thread():
196196
# Only rotate from the main thread to avoid race conditions with
197197
# component threads that also call log().
198-
for num_logs in range(max_logs - 1, 0, -1):
199-
filename = "predbat." + format(num_logs) + ".log"
200-
if os.path.isfile(filename):
201-
newfile = "predbat." + format(num_logs + 1) + ".log"
202-
os.rename(filename, newfile)
198+
rotate_predbat_logs(max_logs)
203199

204200
self.logfile.close()
205-
os.rename("predbat.log", "predbat.1.log")
201+
os.rename("predbat.log", predbat_log_name(1))
206202
self.logfile = open("predbat.log", "w")
207203

208204
async def run_in_executor(self, callback, *args):

‎apps/predbat/predbat.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,6 +1538,12 @@ def validate_config(self):
15381538
matches = True
15391539
if required_entries is not None and len(value) > required_entries:
15401540
value = value[:required_entries]
1541+
# min/max are declared on some APPS_SCHEMA entries (log_count, #5076)
1542+
# but were never actually enforced here - the schema promised a range
1543+
# check that validate_config() silently skipped (Copilot review on
1544+
# #5076).
1545+
schema_min = spec.get("min", None)
1546+
schema_max = spec.get("max", None)
15411547
for item in value:
15421548
if not self.validate_is_int(item):
15431549
self.log("Warn: Validation of apps.yaml found configuration item '{}' element {} is not an integer".format(name, item))
@@ -1549,6 +1555,11 @@ def validate_config(self):
15491555
self.arg_errors[name] = "Invalid value, expected non-zero integer item {}".format(item)
15501556
errors += 1
15511557
break
1558+
if (schema_min is not None and int(item) < schema_min) or (schema_max is not None and int(item) > schema_max):
1559+
self.log("Warn: Validation of apps.yaml found configuration item '{}' value {} is outside the allowed range [{}, {}]".format(name, item, schema_min, schema_max))
1560+
self.arg_errors[name] = "Invalid value, expected between {} and {}".format(schema_min, schema_max)
1561+
errors += 1
1562+
break
15521563
elif expected_type == "float" or expected_type == "float_list":
15531564
if expected_type == "float" and (isinstance(value, float) or isinstance(value, int)):
15541565
value = [value]
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
# -----------------------------------------------------------------------------
2+
# Predbat Home Battery System
3+
# Copyright Trefor Southwell 2026 - All Rights Reserved
4+
# This application maybe used for personal use only and not for commercial use
5+
# -----------------------------------------------------------------------------
6+
# fmt off
7+
# pylint: disable=consider-using-f-string
8+
# pylint: disable=line-too-long
9+
# pylint: disable=attribute-defined-outside-init
10+
11+
"""
12+
Unit tests for configurable log rotation (#5076) - the log_count apps.yaml setting, the
13+
two-digit predbat.NN.log naming, and the migration path that reads the single-digit names an
14+
older Predbat wrote.
15+
"""
16+
17+
import os
18+
import tempfile
19+
20+
from utils import (
21+
PREDBAT_LOG_COUNT_DEFAULT,
22+
PREDBAT_LOG_COUNT_MAX,
23+
PREDBAT_LOG_COUNT_MIN,
24+
predbat_log_count,
25+
predbat_log_file_prev,
26+
predbat_log_name,
27+
predbat_log_name_legacy,
28+
read_predbat_log,
29+
rotate_predbat_logs,
30+
)
31+
32+
33+
def _touch(name, content=""):
34+
"""Create a file with known content, for asserting which file ended up where."""
35+
with open(name, "w", encoding="utf-8") as handle:
36+
handle.write(content)
37+
38+
39+
def _rotate(args):
40+
"""
41+
Run the rotation exactly as Hass.log() does, against the current working directory.
42+
43+
Calls the same rotate_predbat_logs() Hass.log() calls, rather than a hand-copy of its loop -
44+
a copy can diverge from the code it is meant to be testing and still pass, which is exactly
45+
what let the off-by-one Copilot found on #5076 ship with a green test suite (the copy had the
46+
same bug, so it agreed with itself). Only the parts of Hass.log() this file does not otherwise
47+
exercise - closing/reopening the live predbat.log - are still done directly here.
48+
"""
49+
max_logs = predbat_log_count(args) - 1
50+
rotate_predbat_logs(max_logs)
51+
os.rename("predbat.log", predbat_log_name(1))
52+
_touch("predbat.log", "live")
53+
54+
55+
def run_log_rotation_tests(my_predbat):
56+
"""
57+
Test configurable log rotation, two-digit naming and single-digit migration (#5076).
58+
"""
59+
failed = False
60+
print("**** Running log_rotation tests ****")
61+
62+
saved_cwd = os.getcwd()
63+
try:
64+
with tempfile.TemporaryDirectory() as tmpdir:
65+
os.chdir(tmpdir)
66+
67+
print("Test 1: log_count defaults and clamps")
68+
for args, expected, why in (
69+
({}, PREDBAT_LOG_COUNT_DEFAULT, "absent"),
70+
({"log_count": 25}, 25, "in range"),
71+
({"log_count": 1}, PREDBAT_LOG_COUNT_MIN, "below minimum"),
72+
({"log_count": 500}, PREDBAT_LOG_COUNT_MAX, "above maximum"),
73+
({"log_count": "30"}, 30, "numeric string"),
74+
({"log_count": "banana"}, PREDBAT_LOG_COUNT_DEFAULT, "non-numeric"),
75+
({"log_count": None}, PREDBAT_LOG_COUNT_DEFAULT, "null"),
76+
# YAML accepts non-finite numeric scalars (.inf/-.inf), which parse as a float
77+
# int() cannot convert - OverflowError, not caught by the (TypeError, ValueError)
78+
# this already guarded against (Copilot review on #5076).
79+
({"log_count": float("inf")}, PREDBAT_LOG_COUNT_DEFAULT, "positive infinity"),
80+
({"log_count": float("-inf")}, PREDBAT_LOG_COUNT_DEFAULT, "negative infinity"),
81+
):
82+
actual = predbat_log_count(args)
83+
if actual != expected:
84+
print(" ERROR: log_count {} ({}) should give {}, got {}".format(args, why, expected, actual))
85+
failed = True
86+
87+
print("Test 2: names are zero-padded to two digits")
88+
if predbat_log_name(1) != "predbat.01.log" or predbat_log_name(99) != "predbat.99.log":
89+
print(" ERROR: unexpected padded names: {!r} {!r}".format(predbat_log_name(1), predbat_log_name(99)))
90+
failed = True
91+
if predbat_log_name_legacy(1) != "predbat.1.log":
92+
print(" ERROR: unexpected legacy name: {!r}".format(predbat_log_name_legacy(1)))
93+
failed = True
94+
95+
print("Test 3: a rotation shifts every log up one slot")
96+
_touch("predbat.log", "current")
97+
_touch(predbat_log_name(1), "one")
98+
_touch(predbat_log_name(2), "two")
99+
_rotate({"log_count": 10})
100+
for name, expected in ((predbat_log_name(1), "current"), (predbat_log_name(2), "one"), (predbat_log_name(3), "two")):
101+
if not os.path.isfile(name):
102+
print(" ERROR: expected {} to exist after rotation".format(name))
103+
failed = True
104+
elif open(name, encoding="utf-8").read() != expected:
105+
print(" ERROR: {} should hold {!r}, got {!r}".format(name, expected, open(name, encoding="utf-8").read()))
106+
failed = True
107+
108+
print("Test 4: single-digit logs from an older Predbat migrate to two-digit")
109+
for name in os.listdir("."):
110+
os.remove(name)
111+
_touch("predbat.log", "current")
112+
_touch(predbat_log_name_legacy(1), "old-one")
113+
_touch(predbat_log_name_legacy(2), "old-two")
114+
_rotate({"log_count": 10})
115+
if os.path.isfile(predbat_log_name_legacy(2)) or os.path.isfile(predbat_log_name_legacy(3)):
116+
print(" ERROR: single-digit files should have been renamed, found {}".format(sorted(os.listdir("."))))
117+
failed = True
118+
for name, expected in ((predbat_log_name(2), "old-one"), (predbat_log_name(3), "old-two")):
119+
if not os.path.isfile(name) or open(name, encoding="utf-8").read() != expected:
120+
print(" ERROR: expected {} to hold {!r}, listing is {}".format(name, expected, sorted(os.listdir("."))))
121+
failed = True
122+
123+
print("Test 4b: a legacy single-digit log in the OLDEST kept slot is migrated, not stranded")
124+
# #5076 Copilot review: the shift previously only walked slots 1..max_logs-1, so a
125+
# legacy predbat.9.log sitting in the oldest kept slot (max_logs=9 for the default
126+
# log_count=10) was never a rename source - it does not collide with anything landing
127+
# on slot 9 from below, since that only writes the two-digit name. Left on disk under
128+
# the old name forever, invisible to predbat_log_file_prev()'s two-digit-first lookup.
129+
for name in os.listdir("."):
130+
os.remove(name)
131+
_touch("predbat.log", "current")
132+
_touch(predbat_log_name_legacy(9), "oldest-legacy")
133+
_rotate({"log_count": 10}) # max_logs = 9
134+
if os.path.isfile(predbat_log_name_legacy(9)):
135+
print(" ERROR: the legacy file in the oldest kept slot should not survive under its old name, found {}".format(sorted(os.listdir("."))))
136+
failed = True
137+
if os.path.isfile(predbat_log_name(10)) or os.path.isfile(predbat_log_name_legacy(10)):
138+
print(" ERROR: log_count=10 keeps only slots 1-9 plus the live log, found {}".format(sorted(os.listdir("."))))
139+
failed = True
140+
141+
print("Test 5: logs beyond the configured count are removed")
142+
for name in os.listdir("."):
143+
os.remove(name)
144+
_touch("predbat.log", "current")
145+
for num in range(1, 6):
146+
_touch(predbat_log_name(num), "log{}".format(num))
147+
_rotate({"log_count": 3})
148+
# Keeping 3 files total means the live log plus slots 01 and 02.
149+
if os.path.isfile(predbat_log_name(3)):
150+
print(" ERROR: slot 03 should have been removed with log_count 3, listing is {}".format(sorted(os.listdir("."))))
151+
failed = True
152+
remaining = sorted(name for name in os.listdir(".") if name != "predbat.log")
153+
if remaining != [predbat_log_name(1), predbat_log_name(2)]:
154+
print(" ERROR: expected only slots 01 and 02 to remain, got {}".format(remaining))
155+
failed = True
156+
157+
print("Test 6: lowering log_count clears files stranded above the new count")
158+
for name in os.listdir("."):
159+
os.remove(name)
160+
_touch("predbat.log", "current")
161+
for num in range(1, 21):
162+
_touch(predbat_log_name(num), "log{}".format(num))
163+
_rotate({"log_count": 4})
164+
stranded = [name for name in os.listdir(".") if name not in ("predbat.log", predbat_log_name(1), predbat_log_name(2), predbat_log_name(3))]
165+
if stranded:
166+
print(" ERROR: files above the new count should be removed, found {}".format(sorted(stranded)))
167+
failed = True
168+
169+
print("Test 7: the viewer prefers the two-digit previous log")
170+
for name in os.listdir("."):
171+
os.remove(name)
172+
_touch("predbat.log", "live-line")
173+
_touch(predbat_log_name(1), "padded-prev")
174+
if predbat_log_file_prev() != predbat_log_name(1):
175+
print(" ERROR: expected the padded name to win, got {!r}".format(predbat_log_file_prev()))
176+
failed = True
177+
data = read_predbat_log()
178+
if "padded-prev" not in data or "live-line" not in data:
179+
print(" ERROR: the viewer should show both logs, got {!r}".format(data))
180+
failed = True
181+
if data.index("padded-prev") > data.index("live-line"):
182+
print(" ERROR: the previous log should be prefixed, not appended, got {!r}".format(data))
183+
failed = True
184+
185+
print("Test 8: the viewer falls back to a single-digit previous log after an upgrade")
186+
for name in os.listdir("."):
187+
os.remove(name)
188+
_touch("predbat.log", "live-line")
189+
_touch(predbat_log_name_legacy(1), "legacy-prev")
190+
if predbat_log_file_prev() != predbat_log_name_legacy(1):
191+
print(" ERROR: expected the legacy name as fallback, got {!r}".format(predbat_log_file_prev()))
192+
failed = True
193+
data = read_predbat_log()
194+
if "legacy-prev" not in data:
195+
print(" ERROR: an upgrade must not lose the previous log, got {!r}".format(data))
196+
failed = True
197+
198+
print("Test 9: no previous log at all is not an error")
199+
for name in os.listdir("."):
200+
os.remove(name)
201+
_touch("predbat.log", "only-line")
202+
if predbat_log_file_prev() is not None:
203+
print(" ERROR: expected no previous log, got {!r}".format(predbat_log_file_prev()))
204+
failed = True
205+
if read_predbat_log().strip() != "only-line":
206+
print(" ERROR: expected just the live log, got {!r}".format(read_predbat_log()))
207+
failed = True
208+
finally:
209+
os.chdir(saved_cwd)
210+
211+
if failed:
212+
print("\n**** log_rotation tests: FAILED ****")
213+
else:
214+
print("\n**** log_rotation tests: PASSED ****")
215+
216+
return failed

‎apps/predbat/tests/test_validate_config.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,23 @@ def test_validate_config(my_predbat):
128128
print(" [integer zero:False] zero fails")
129129
_run(my_predbat, {"gateway_mqtt_port": 0}, expect_errors=["gateway_mqtt_port"])
130130

131+
# min/max constraint (log_count: {"type": "integer", "min": 2, "max": 100}) - declared in the
132+
# schema but not actually enforced by validate_config()'s integer branch, so an out-of-range
133+
# value silently passed validation despite the schema promising a range check
134+
# (Copilot review on #5076).
135+
print(" [integer min/max] in-range value passes")
136+
_run(my_predbat, {"log_count": 25}, expect_clean=["log_count"])
137+
138+
print(" [integer min/max] below minimum fails")
139+
_run(my_predbat, {"log_count": 1}, expect_errors=["log_count"])
140+
141+
print(" [integer min/max] above maximum fails")
142+
_run(my_predbat, {"log_count": 101}, expect_errors=["log_count"])
143+
144+
print(" [integer min/max] exactly at the boundary passes")
145+
_run(my_predbat, {"log_count": 2}, expect_clean=["log_count"])
146+
_run(my_predbat, {"log_count": 100}, expect_clean=["log_count"])
147+
131148
# ==========================================================================
132149
# INTEGER_LIST type (days_previous: {"type": "integer_list"})
133150
# ==========================================================================

‎apps/predbat/unit_test.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@
231231
from tests.test_rate_add_io_slots import run_rate_add_io_slots_tests
232232
from tests.test_iog_charge_skew import run_iog_charge_skew_tests
233233
from tests.test_dispatch_timeline import run_dispatch_timeline_tests
234+
from tests.test_log_rotation import run_log_rotation_tests
234235
from tests.test_battery_curve_keys import run_battery_curve_keys_tests
235236
from tests.test_balance_inverters import run_balance_inverters_tests
236237
from tests.test_octopus_download_rates import test_octopus_download_rates_wrapper
@@ -550,6 +551,7 @@ def main():
550551
("rate_add_io_slots", run_rate_add_io_slots_tests, "Rate add IO slots tests", False),
551552
("iog_charge_skew", run_iog_charge_skew_tests, "IOG earlier-charge skew characterisation tests", False),
552553
("dispatch_timeline", run_dispatch_timeline_tests, "Dispatch timeline diagnostic tests (#4516 Stage 1)", False),
554+
("log_rotation", run_log_rotation_tests, "Configurable log rotation and two-digit naming (#5076)", False),
553555
("rate_replicate", test_rate_replicate, "Rate replicate comprehensive tests (missing slots, IO, offsets, gas)", False),
554556
("find_charge_window", test_find_charge_window, "Find charge window gap handling tests", False),
555557
("find_charge_rate", test_find_charge_rate, "Find charge rate tests", False),

0 commit comments

Comments
 (0)