fix: file-log rotation never fires again after process start - #2284
Open
raajkumars wants to merge 1 commit into
Open
fix: file-log rotation never fires again after process start#2284raajkumars wants to merge 1 commit into
raajkumars wants to merge 1 commit into
Conversation
The per-process file log's rotation trigger, introduced in exo-explore#1438, is a generator that returns True exactly once and False forever after. loguru calls the rotation callable on every message, so this correctly rotated once on startup (a fresh log file per run) but then silently disabled every subsequent rotation for the rest of the process's life. The retention and compression settings on that same logger.add() call implied ongoing rotation was happening; it never was, so a long-running process's log file grows without bound. Replace the generator with a stateful rotation policy: still rotate once on the first message (preserving the fresh-log-per-run behavior), then rotate again once the file crosses 200MB (hardcoded, not currently exposed as a setting) -- the check the generator never actually performed despite the retention/compression config being set up as if it had. Added unit tests for the new policy: startup rotation, no rotation under the size threshold, rotation once the threshold is crossed, no repeated startup-style rotation, and no crash if the file handle's tell() raises.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The per-process file log's rotation trigger, introduced in #1438, is a generator that returns
Trueexactly once andFalseforever after (_once_then_never). loguru calls the rotation callable on every message, so this correctly rotates once on startup — giving a fresh log file each run — but then silently disables every subsequent rotation for the rest of the process's life. Theretention/compressionsettings on the samelogger.add()call imply ongoing rotation is happening; it never is, so the log file grows without bound on a long-running process.Fix
Replace the generator with a stateful rotation policy (
_rotation_policy): still force-rotate on the first message (preserving the original fresh-log-per-run intent), then rotate again once the file crosses 200MB — the check the generator never actually performed despite the retention/compression config being set up as if it had. The threshold is currently hardcoded (_ROTATION_MAX_BYTES), not exposed as a CLI flag or env var.Tests
Added
src/exo/shared/tests/test_logging_rotation.py, 5 tests against the new policy directly (no I/O, a minimal fake file object standing in for what loguru passes to the rotation callable):tell()raisingOSErroris handled without propagatingAll pass.
ruff check,ruff format --check, andbasedpyrightare clean on both changed files.