adafruit / adafruit/Adafruit_CircuitPython_Logging

Performance, issue in `doRollover`, bug in `exception`, inconsistency in default formatting, static analysis issues, documentation/API mismatch

Open
#68 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
15
Forks
23
PR merge metrics
No merged PRs in 30d

Description

Fixes in `https://github.com/adafruit/Adafruit_CircuitPython_Logging/compare/main...co-rc:Adafruit_CircuitPython_Logging:main`

#### 1. Performance Optimization in `_log()`
* **Issue:** The library was performing string formatting (`msg % args`) and creating a `LogRecord` object (including a `time.monotonic()` call) *before* checking if the message level met the logger's or handlers' thresholds. In resource-constrained environments like CircuitPython, this causes unnecessary CPU and RAM overhead for logs that are ultimately suppressed.
* **Fix:** Added early exit checks in `Logger._log()`. Now, formatting and record creation only occur if the level passes the logger's threshold AND at least one handler (or the default handler) is configured to accept that level.

#### 2. Fixed `RotatingFileHandler.doRollover()` Logic
* **Issue:**
* **Broken Rename Sequence:** The previous implementation attempted to rename files from oldest to newest (e.g., `.1` -> `.2`, then `.2` -> `.3`). This fails if the target filename already exists.
* **Off-by-one Error:** The rollover logic incorrectly handled the `backupCount` when deleting the oldest file.
* **Typos:** Misspelling `exsist` instead of `exists` in `OSError` comments and handling.
* **Fix:** Rewrote `doRollover` to rename files from newest to oldest (e.g., `.4` -> `.5`, then `.3` -> `.4`), which is the standard way to avoid collisions. Corrected the `backupCount` logic and fixed the typos.

#### 3. Bug in `Logger.exception()`
* **Issue:** The `exception()` method was duplicating the exception message and incorrectly joining traceback lines, leading to redundant and poorly formatted output.
* **Fix:** Simplified the method to use `traceback.format_exception(err)` properly, which inherently includes the exception description and correctly formatted traceback lines.

#### 4. Consistency in Default Formatting
* **Issue:** `Handler.format()` used a hardcoded default format that was inconsistent with the default behavior of the `Formatter` class (which defaults to just the message).
* **Fix:** Unified the behavior. If no formatter is set, the handler now returns `record.msg`, ensuring consistency across the API.

#### 5. IDE Support for Logging Constants
* **Issue:** Logging levels (DEBUG, INFO, etc.) were being injected into `globals()` dynamically. This prevents IDEs and static analyzers from recognizing these constants, leading to "Unresolved reference" warnings.
* **Fix:** Explicitly defined `NOTSET`, `DEBUG`, `INFO`, `WARNING`, `ERROR`, and `CRITICAL` as module-level constants.

#### 6. Documentation/API Mismatch
* **Issue:** The `Formatter` docstring stated the default style is `{`, whereas the `__init__` method actually defaulted to `%`.
* **Fix:** Updated the docstring to correctly reflect the default `%` style.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.