Skip to content

RSPEED-2477: Deduplicate logging setup across entry point, log utility, and uvicorn runner#1212

Merged
tisnik merged 3 commits intolightspeed-core:mainfrom
major:refactor/deduplicate-logging
Feb 24, 2026
Merged

RSPEED-2477: Deduplicate logging setup across entry point, log utility, and uvicorn runner#1212
tisnik merged 3 commits intolightspeed-core:mainfrom
major:refactor/deduplicate-logging

Conversation

@major
Copy link
Contributor

@major major commented Feb 24, 2026

Description

PRs #1201 and #1202 fixed TTY-aware logging in two places and log-level resolution in three places, leaving identical logic copy-pasted across lightspeed_stack.py, log.py, and runners/uvicorn.py.

This extracts two shared functions into log.py:

  • resolve_log_level() — reads env var, validates, falls back to INFO (was in 3 files)
  • create_log_handler() — TTY → RichHandler, non-TTY → StreamHandler (was in 2 files)

The hardcoded format string moves to DEFAULT_LOG_FORMAT in constants.py.

get_logger() signature is unchanged — zero impact on the 72 modules that import it. lightspeed_stack.py keeps its own basicConfig(force=True) call since root logger config is entry-point-specific.

Type of change

  • Refactor
  • Unit tests improvement

Tools used to create PR

  • Assisted-by: Claude

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • uv run make test-unit — 1500 tests pass (86% coverage)
  • uv run make verify — all linters pass (black, pylint, pyright, ruff, docstyle, mypy)
  • New tests for resolve_log_level() (parametrized levels, invalid fallback, unset default)
  • New tests for create_log_handler() (TTY → RichHandler, non-TTY → StreamHandler, format verification)

Summary by CodeRabbit

  • Refactor

    • Centralized logging setup logic into dedicated utility functions for improved consistency across the application.
    • Enhanced log level validation with fallback to default values when invalid inputs are provided.
  • Tests

    • Added comprehensive test coverage for new logging utilities, including handler selection based on terminal detection.

…nto log.py

Add DEFAULT_LOG_FORMAT constant to constants.py and extract two public
functions into log.py: resolve_log_level() for env-var-based level
resolution and create_log_handler() for TTY-aware handler selection.

Simplify get_logger() to delegate to these new functions while keeping
its signature unchanged.

Signed-off-by: Major Hayden <major@redhat.com>
Replace inline log-level resolution and handler creation in
lightspeed_stack.py and runners/uvicorn.py with calls to the shared
resolve_log_level() and create_log_handler() from log.py.

Remove the private _resolve_log_level() from uvicorn.py and the direct
RichHandler import from lightspeed_stack.py.

Signed-off-by: Major Hayden <major@redhat.com>
Add parametrized tests for resolve_log_level() covering valid levels,
invalid fallback, and unset default. Add tests for create_log_handler()
verifying TTY returns RichHandler, non-TTY returns StreamHandler with
the correct format.

Update test_uvicorn_runner.py to import resolve_log_level from log
instead of the deleted private _resolve_log_level from runners.uvicorn.

Signed-off-by: Major Hayden <major@redhat.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa48185 and bf2ccce.

📒 Files selected for processing (6)
  • src/constants.py
  • src/lightspeed_stack.py
  • src/log.py
  • src/runners/uvicorn.py
  • tests/unit/runners/test_uvicorn_runner.py
  • tests/unit/test_log.py

Walkthrough

The PR centralizes logging configuration by introducing utility functions in the log module and constants for log format, removing duplicated log level resolution logic from individual modules and creating a single source of truth for logging setup.

Changes

Cohort / File(s) Summary
Logging Configuration Constants
src/constants.py
Added DEFAULT_LOG_FORMAT constant defining plain-text log format for non-TTY environments.
Centralized Logging Utilities
src/log.py
Introduced resolve_log_level() to read and validate LIGHTSPEED_STACK_LOG_LEVEL_ENV_VAR with fallback to DEFAULT_LOG_LEVEL; introduced create_log_handler() to return RichHandler for TTY or StreamHandler with DEFAULT_LOG_FORMAT for non-TTY; updated get_logger() to use these utilities.
Logging Refactoring
src/lightspeed_stack.py, src/runners/uvicorn.py
Replaced manual environment-based log level parsing and handler creation with calls to resolve_log_level() and create_log_handler(); removed local _resolve_log_level() from uvicorn.py.
Test Updates
tests/unit/test_log.py, tests/unit/runners/test_uvicorn_runner.py
Expanded test coverage for resolve_log_level() and create_log_handler() with parameterized cases for log levels, TTY detection, and fallback behavior; updated imports to use centralized utilities instead of local helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

ok-to-test

Suggested reviewers

  • jrobertboos
  • tisnik
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: deduplicating logging setup logic across three locations. It is concise, directly related to the core purpose of the PR, and uses a ticket reference for traceability.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tisnik tisnik merged commit 051c06c into lightspeed-core:main Feb 24, 2026
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants