Unify logging configuration and setup#1703
Unify logging configuration and setup#1703samdoran wants to merge 10 commits intolightspeed-core:mainfrom
Conversation
Meroge the existing config from uvicorn with the logging for lightspeed-stack with some modifications and pass that to uvicorn. This ensures the logging configs work together and do not clobber each other. Call setup_logging() early in the main entrypoint.
…y in __name__. The logging library assumes __name__ will be “package.module.module”. Since this project does not have a package, the value for __name__ varies widely in each module. Thise breaks design assumpmtions of logging. To work around this, define a default logger name that is used as the primary configuration and add a helper function to always get the logger with a name that aligns with how logging works.
Since there is no root package for this project, manually set the logger and get the filename in order to show where the log message was emitted.
Use levelprefix for uvicorn.logging.DefaultFormatte. Move filename and position to end of line so that information is arranged in most important order from left to right within the line, where the message came from being least relevant in my thinking compared to the time, log level, and actual message.
When rich is not selected, use the uvicorn.logging.DefaultFormatter for log messages. Modify the default format slightly to include miliseconds in the timestamp.
Add a description of the problem to be addressed in the future.
WalkthroughThis PR refactors logging initialization from inline setup scattered across modules into a centralized, cached ChangesLogging Setup Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runners/uvicorn.py (1)
17-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocstring doesn't document the new
log_configparameter🛠 Proposed fix
"""Start the Uvicorn server using the provided service configuration. Parameters: ---------- configuration (ServiceConfiguration): Configuration providing host, port, workers, and `tls_config` (including `tls_key_path`, `tls_certificate_path`, and `tls_key_password`). TLS fields may be None and will be forwarded to uvicorn.run as provided. + log_config (dict | None): Logging configuration dict passed to uvicorn.run. + When None, the centralized setup_logging() result is used. """As per coding guidelines, all functions must "include descriptive docstrings" covering all parameters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runners/uvicorn.py` around lines 17 - 25, The docstring for the Uvicorn-starting function (start_uvicorn in src/runners/uvicorn.py) is missing documentation for the new log_config parameter; update the function docstring to list and describe log_config (type: optional dict or logging config path, default/behavior: forwarded to uvicorn.run to configure logging) alongside the existing parameters so callers know what to pass and how it is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/constants.py`:
- Line 228: DEFAULT_LOGGER_NAME is missing the Final[str] annotation; update the
constant declaration for DEFAULT_LOGGER_NAME to be annotated with Final[str]
(e.g., DEFAULT_LOGGER_NAME: Final[str] = "lightspeed_stack") and ensure Final is
imported from typing at the top of src/constants.py (add "from typing import
Final" if it's not already present) so the constant is type-locked like the
others.
In `@src/log.py`:
- Line 1: The file src/log.py fails Black formatting; run the project formatter
and commit the changes: execute "uv tool run black src/log.py" (or "uv tool run
black src tests") to reformat the module (src.log) and then stage and commit the
updated file so the CI Black check passes.
- Around line 59-70: The logger helper uses a DEFAULT_LOGGER_NAME prefix to work
around inconsistent __name__ values across different entrypoints (see get_logger
and DEFAULT_LOGGER_NAME); open a tracking issue to remove that workaround by
standardizing the package/entrypoint layout so __name__ always yields the
expected root, document the change, and include a migration plan: update
get_logger to use __name__ (remove the DEFAULT_LOGGER_NAME concatenation),
refactor any callers that relied on the prefixed logger names (adjust tests and
configuration), and add a small integration test that verifies the root logger
name is consistent when run via all supported entrypoints.
- Around line 107-117: The setup_logging() function currently calls
deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf) then mutates
merged_config which aliases uvicorn.config.LOGGING_CONFIG; instead construct all
conditional changes (the "rich" handler branch and the access/default formatter
entries: the handlers for "uvicorn" and "uvicorn.access" and the
formatters["access"] and formatters["default"]["fmt"/"datefmt"]) inside
logging_conf before calling deep_update so the single deep_update call merges
everything and merged_config is never mutated afterwards; update the code paths
that reference handler, merged_config and logging_conf accordingly so no
post-merge assignments (e.g., merged_config["formatters"][...] = ...) remain.
- Line 89: The Rich handler configuration uses "log_time_format" set to
"%Y-%m-%d %H:%M:%S.%f" which emits 6-digit microseconds instead of the intended
3-digit milliseconds; update the Rich-based logging path to remove the "%f"
subsecond directive (or omit log_time_format entirely) and rely on RichHandler's
own millisecond column via show_time=True, ensuring consistency with
DEFAULT_LOG_FORMAT which uses "%(msecs)03d"; modify the code that sets
log_time_format (symbol: log_time_format) and the Rich handler instantiation
(symbol: RichHandler, show_time) so the Rich path does not output 6-digit
microseconds.
- Line 11: The import of pydantic.v1.utils.deep_update in src/log.py uses an
internal v1-compat API — remove that import, add a small local helper function
named _deep_update(base: dict, override: dict) that recursively merges override
into base (preserving dict semantics), and replace all usages of
deep_update(...) with _deep_update(...); update any function or module
references in this file that call deep_update to call _deep_update instead so
there is no dependency on pydantic internals.
- Around line 73-74: Add a clear docstring to the public function
setup_logging() that explains what the function does (e.g., configures and
returns the logging configuration dict, applies any global logging settings or
handlers) and any side effects (such as calling logging.config.dictConfig or
mutating global logger state); ensure the docstring appears immediately below
the def setup_logging() line and follows project docstring style so pydocstyle
D103 is satisfied.
---
Outside diff comments:
In `@src/runners/uvicorn.py`:
- Around line 17-25: The docstring for the Uvicorn-starting function
(start_uvicorn in src/runners/uvicorn.py) is missing documentation for the new
log_config parameter; update the function docstring to list and describe
log_config (type: optional dict or logging config path, default/behavior:
forwarded to uvicorn.run to configure logging) alongside the existing parameters
so callers know what to pass and how it is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c2b1d91-2f32-49b8-ae1e-1cede9a1feb3
📒 Files selected for processing (5)
src/constants.pysrc/lightspeed_stack.pysrc/log.pysrc/runners/uvicorn.pytests/unit/test_log.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: unit_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: Pylinter
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Llama Stack imports: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Useasync deffor I/O operations and external API calls
Use standard log levels with clear purposes:debug()for diagnostic info,info()for program execution,warning()for unexpected events,error()for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes:Configuration,Error/Exception,Resolver,Interface
Abstract classes must use ABC with@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/constants.pysrc/lightspeed_stack.pysrc/runners/uvicorn.pysrc/log.py
src/constants.py
📄 CodeRabbit inference engine (AGENTS.md)
Use
constants.pyfor shared constants with descriptive comments and type hints usingFinal[type]
Files:
src/constants.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/test_log.py
🪛 GitHub Actions: Black / 0_black.txt
src/log.py
[error] 1-1: Black formatting check failed (uv tool run black --check src tests). File would be reformatted. Run 'uv tool run black src tests' (or 'black --write') to fix.
🪛 GitHub Actions: Black / black
src/log.py
[error] 1-1: Black --check failed: would reformat /home/runner/work/lightspeed-stack/lightspeed-stack/src/log.py (1 file would be reformatted). Run 'uv tool run black src/log.py' (or 'uv tool run black --write src tests') to apply formatting.
🪛 GitHub Actions: Pydocstyle / 0_pydocstyle.txt
src/log.py
[error] 74-74: pydocstyle failed (D103): Missing docstring in public function setup_logging.
🪛 GitHub Actions: Pydocstyle / pydocstyle
src/log.py
[error] 74-74: pydocstyle (uv tool run pydocstyle -v src tests) reported: D103: Missing docstring in public function setup_logging.
🔇 Additional comments (2)
src/lightspeed_stack.py (1)
13-19: LGTM — logging initialization order is correct
setup_logging()is called beforeget_logger(__name__), ensuringdictConfigis applied before the logger hierarchy is first accessed.tests/unit/test_log.py (1)
14-16: The fixture is working correctly.deep_updatefrompydantic.v1.utilscreates a new merged dictionary rather than mutatinguvicorn.config.LOGGING_CONFIGin-place, sosetup_logging()does not leave persistent global state mutations across tests. Clearing the@lru_cacheensures the function body re-executes for each test, which is the appropriate behavior since tests modify environment variables viamonkeypatchbetween runs.
| # Environment variable name for configurable log level | ||
| LIGHTSPEED_STACK_LOG_LEVEL_ENV_VAR: Final[str] = "LIGHTSPEED_STACK_LOG_LEVEL" | ||
| # Default log level when environment variable is not set | ||
| DEFAULT_LOGGER_NAME = "lightspeed_stack" |
There was a problem hiding this comment.
DEFAULT_LOGGER_NAME is missing its Final[str] annotation
Every other constant in this file uses Final[type]. This one does not, leaving it open to accidental reassignment and breaking type-checker guarantees.
🛠 Proposed fix
-DEFAULT_LOGGER_NAME = "lightspeed_stack"
+DEFAULT_LOGGER_NAME: Final[str] = "lightspeed_stack"As per coding guidelines, src/constants.py requires "type hints using Final[type]" for shared constants.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DEFAULT_LOGGER_NAME = "lightspeed_stack" | |
| DEFAULT_LOGGER_NAME: Final[str] = "lightspeed_stack" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/constants.py` at line 228, DEFAULT_LOGGER_NAME is missing the Final[str]
annotation; update the constant declaration for DEFAULT_LOGGER_NAME to be
annotated with Final[str] (e.g., DEFAULT_LOGGER_NAME: Final[str] =
"lightspeed_stack") and ensure Final is imported from typing at the top of
src/constants.py (add "from typing import Final" if it's not already present) so
the constant is type-locked like the others.
| @@ -1,14 +1,19 @@ | |||
| """Log utilities.""" | |||
There was a problem hiding this comment.
Black formatting check fails — reformat src/log.py before merge
The CI pipeline reports Black would reformat this file. Run uv tool run black src/log.py (or uv tool run black src tests) to fix.
🧰 Tools
🪛 GitHub Actions: Black / 0_black.txt
[error] 1-1: Black formatting check failed (uv tool run black --check src tests). File would be reformatted. Run 'uv tool run black src tests' (or 'black --write') to fix.
🪛 GitHub Actions: Black / black
[error] 1-1: Black --check failed: would reformat /home/runner/work/lightspeed-stack/lightspeed-stack/src/log.py (1 file would be reformatted). Run 'uv tool run black src/log.py' (or 'uv tool run black --write src tests') to apply formatting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/log.py` at line 1, The file src/log.py fails Black formatting; run the
project formatter and commit the changes: execute "uv tool run black src/log.py"
(or "uv tool run black src tests") to reformat the module (src.log) and then
stage and commit the updated file so the CI Black check passes.
|
|
||
| from rich.logging import RichHandler | ||
| import uvicorn.config | ||
| from pydantic.v1.utils import deep_update |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
pydantic.v1.utils.deep_update is an internal v1-compat shim — replace with a self-contained helper
pydantic.v1.utils is part of the v1 compatibility layer bundled in pydantic v2, not a public API. Functionality marked as deprecated will not be removed until the next major V3 release, but deep_update from pydantic.v1.utils is an internal implementation detail, not a stable public export. Importing it creates an unnecessary coupling to pydantic's internals for a trivial utility. A 5-line replacement eliminates the dependency entirely.
♻️ Proposed replacement
Remove the pydantic import and add a local helper:
-from pydantic.v1.utils import deep_updatedef _deep_update(base: dict, override: dict) -> dict:
"""Recursively merge override into base, returning a new dict."""
result = base.copy()
for key, value in override.items():
if key in result and isinstance(result[key], dict) and isinstance(value, dict):
result[key] = _deep_update(result[key], value)
else:
result[key] = value
return resultThen replace all calls to deep_update(...) with _deep_update(...).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/log.py` at line 11, The import of pydantic.v1.utils.deep_update in
src/log.py uses an internal v1-compat API — remove that import, add a small
local helper function named _deep_update(base: dict, override: dict) that
recursively merges override into base (preserving dict semantics), and replace
all usages of deep_update(...) with _deep_update(...); update any function or
module references in this file that call deep_update to call _deep_update
instead so there is no dependency on pydantic internals.
| """Create a common logger for all modules in this package.""" | ||
| # FIXME: Remove the need for this function. | ||
| # | ||
| # Normally this is derived from the package name (__name__). | ||
| # | ||
| # Since this program is sometimes called from from the entrypoint and | ||
| # sometimes called from src/lightspeed_stack.py, the value for __name__ | ||
| # does not contain a consistent root value. | ||
| # | ||
| # How the application is installed and run needs to be streamlined so that | ||
| # __name__ provides the expected value in all cases. | ||
| return logging.getLogger(f"{DEFAULT_LOGGER_NAME}.{name}") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
FIXME: inconsistent __name__ for the root logger — tracked for follow-up
The FIXME at line 60 acknowledges that get_logger(__name__) produces different root prefixes depending on how the application is launched.
Would you like me to open a new issue to track streamlining the application's package layout so __name__ always resolves to the expected root, removing the need for the DEFAULT_LOGGER_NAME prefix workaround entirely?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/log.py` around lines 59 - 70, The logger helper uses a
DEFAULT_LOGGER_NAME prefix to work around inconsistent __name__ values across
different entrypoints (see get_logger and DEFAULT_LOGGER_NAME); open a tracking
issue to remove that workaround by standardizing the package/entrypoint layout
so __name__ always yields the expected root, document the change, and include a
migration plan: update get_logger to use __name__ (remove the
DEFAULT_LOGGER_NAME concatenation), refactor any callers that relied on the
prefixed logger names (adjust tests and configuration), and add a small
integration test that verifies the root logger name is consistent when run via
all supported entrypoints.
| @lru_cache | ||
| def setup_logging() -> dict[t.Any, t.Any]: |
There was a problem hiding this comment.
setup_logging() is missing a docstring — CI reports D103
The pydocstyle pipeline fails with D103: Missing docstring in public function setup_logging. Add a docstring describing the function's behavior and side effects.
🛠 Proposed fix
`@lru_cache`
def setup_logging() -> dict[t.Any, t.Any]:
+ """Configure and return the merged logging configuration.
+
+ Selects a Rich or uvicorn default handler based on TTY detection and the
+ LIGHTSPEED_STACK_DISABLE_RICH_HANDLER environment variable, merges the
+ computed configuration into uvicorn's default logging config, applies it
+ via logging.config.dictConfig, and returns the merged configuration dict.
+ Result is cached; call setup_logging.cache_clear() to force re-initialization.
+
+ Returns:
+ dict: The merged logging configuration passed to dictConfig.
+ """🧰 Tools
🪛 GitHub Actions: Pydocstyle / 0_pydocstyle.txt
[error] 74-74: pydocstyle failed (D103): Missing docstring in public function setup_logging.
🪛 GitHub Actions: Pydocstyle / pydocstyle
[error] 74-74: pydocstyle (uv tool run pydocstyle -v src tests) reported: D103: Missing docstring in public function setup_logging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/log.py` around lines 73 - 74, Add a clear docstring to the public
function setup_logging() that explains what the function does (e.g., configures
and returns the logging configuration dict, applies any global logging settings
or handlers) and any side effects (such as calling logging.config.dictConfig or
mutating global logger state); ensure the docstring appears immediately below
the def setup_logging() line and follows project docstring style so pydocstyle
D103 is satisfied.
| "rich": { | ||
| "()": "rich.logging.RichHandler", | ||
| "show_time": True, | ||
| "log_time_format": "%Y-%m-%d %H:%M:%S.%f", |
There was a problem hiding this comment.
log_time_format uses %f (microseconds, 6 digits) — PR goal was milliseconds
The %f strftime directive expands to 6-digit microseconds (e.g., 123456). The non-Rich path correctly uses %(msecs)03d for 3-digit milliseconds in DEFAULT_LOG_FORMAT. The Rich handler will show 6-digit precision instead of the intended 3-digit millisecond granularity.
🛠 Proposed fix
- "log_time_format": "%Y-%m-%d %H:%M:%S.%f",
+ "log_time_format": "%Y-%m-%d %H:%M:%S",Rich's RichHandler renders its own millisecond column separately when show_time=True, so the sub-second portion in log_time_format is typically redundant.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "log_time_format": "%Y-%m-%d %H:%M:%S.%f", | |
| "log_time_format": "%Y-%m-%d %H:%M:%S", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/log.py` at line 89, The Rich handler configuration uses "log_time_format"
set to "%Y-%m-%d %H:%M:%S.%f" which emits 6-digit microseconds instead of the
intended 3-digit milliseconds; update the Rich-based logging path to remove the
"%f" subsecond directive (or omit log_time_format entirely) and rely on
RichHandler's own millisecond column via show_time=True, ensuring consistency
with DEFAULT_LOG_FORMAT which uses "%(msecs)03d"; modify the code that sets
log_time_format (symbol: log_time_format) and the Rich handler instantiation
(symbol: RichHandler, show_time) so the Rich path does not output 6-digit
microseconds.
| merged_config = deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf) | ||
|
|
||
| if handler == "rich": | ||
| merged_config["loggers"]["uvicorn"]["handlers"] = [handler] | ||
| merged_config["loggers"]["uvicorn.access"]["handlers"] = [handler] | ||
| else: | ||
| merged_config["formatters"]["access"]["fmt"] = ( | ||
| 'ACCESS %(asctime)s.%(msecs)03d %(levelprefix)s %(client_addr)s - "%(request_line)s" %(status_code)s' | ||
| ) | ||
| merged_config["formatters"]["default"]["fmt"] = DEFAULT_LOG_FORMAT | ||
| merged_config["formatters"]["default"]["datefmt"] = "%Y-%m-%d %H:%M" |
There was a problem hiding this comment.
setup_logging() silently mutates uvicorn.config.LOGGING_CONFIG — corrupts global state on cache replay
deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf) performs only a shallow copy at the top level. Because logging_conf contains no "formatters" key, merged_config["formatters"] is the same dict object as uvicorn.config.LOGGING_CONFIG["formatters"]. The subsequent assignments on lines 113–117 therefore permanently mutate uvicorn's module-level global:
merged_config["formatters"]["access"]["fmt"] = ... # mutates uvicorn.config.LOGGING_CONFIG
merged_config["formatters"]["default"]["fmt"] = ... # mutates uvicorn.config.LOGGING_CONFIG
merged_config["formatters"]["default"]["datefmt"] = ... # mutates uvicorn.config.LOGGING_CONFIGSimilarly in the rich path (lines 110–111), merged_config["loggers"]["uvicorn"] is an alias for the original uvicorn logger dict, so merged_config["loggers"]["uvicorn"]["handlers"] = [handler] also mutates the global.
In tests this causes silent pollution: cache_clear() re-runs the function body against an already-mutated uvicorn.config.LOGGING_CONFIG, making the test environment progressively diverge from a clean state.
Root fix: build all conditional configuration inside logging_conf before the single deep_update call, so deep_update handles all merges correctly and no post-merge mutations are needed.
🛠 Proposed fix
- logging_conf = {
+ # Build formatters/loggers conditionally so deep_update handles all merges
+ # and uvicorn.config.LOGGING_CONFIG is never mutated.
+ logging_conf: dict[t.Any, t.Any] = {
"version": 1,
"disable_existing_loggers": False,
"handlers": {
"rich": {
"()": "rich.logging.RichHandler",
"show_time": True,
- "log_time_format": "%Y-%m-%d %H:%M:%S.%f",
+ "log_time_format": "%Y-%m-%d %H:%M:%S",
"level": log_level,
},
},
"loggers": {
DEFAULT_LOGGER_NAME: {
"handlers": [handler],
"level": log_level,
"propagate": False,
},
"llama_stack_client": {
"handlers": [handler],
"level": log_level,
"propagate": False,
},
},
}
if handler == "rich":
- merged_config = deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf)
- merged_config["loggers"]["uvicorn"]["handlers"] = [handler]
- merged_config["loggers"]["uvicorn.access"]["handlers"] = [handler]
- else:
- merged_config = deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf)
- merged_config["formatters"]["access"]["fmt"] = (
- 'ACCESS %(asctime)s.%(msecs)03d %(levelprefix)s %(client_addr)s - "%(request_line)s" %(status_code)s'
- )
- merged_config["formatters"]["default"]["fmt"] = DEFAULT_LOG_FORMAT
- merged_config["formatters"]["default"]["datefmt"] = "%Y-%m-%d %H:%M"
+ logging_conf["loggers"]["uvicorn"] = {"handlers": ["rich"]}
+ logging_conf["loggers"]["uvicorn.access"] = {"handlers": ["rich"]}
+ else:
+ logging_conf["formatters"] = {
+ "access": {
+ "fmt": 'ACCESS %(asctime)s.%(msecs)03d %(levelprefix)s %(client_addr)s - "%(request_line)s" %(status_code)s',
+ },
+ "default": {
+ "fmt": DEFAULT_LOG_FORMAT,
+ "datefmt": "%Y-%m-%d %H:%M",
+ },
+ }
+ merged_config = deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf)
logging.config.dictConfig(merged_config)
return merged_config🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/log.py` around lines 107 - 117, The setup_logging() function currently
calls deep_update(uvicorn.config.LOGGING_CONFIG, logging_conf) then mutates
merged_config which aliases uvicorn.config.LOGGING_CONFIG; instead construct all
conditional changes (the "rich" handler branch and the access/default formatter
entries: the handlers for "uvicorn" and "uvicorn.access" and the
formatters["access"] and formatters["default"]["fmt"/"datefmt"]) inside
logging_conf before calling deep_update so the single deep_update call merges
everything and merged_config is never mutated afterwards; update the code paths
that reference handler, merged_config and logging_conf accordingly so no
post-merge assignments (e.g., merged_config["formatters"][...] = ...) remain.
1 participant
Description
Major overhaul of the logging system:
setup_logging()early on so that logging is setup once and cache the result.lightspeed_stackso that the logging configuration is applied correctly.The end result of these changes is that calling
get_logger(__name__)results in a logger instance wherelogger.info()and other methods display log messages as expected and formatted consistently.Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
None
Checklist before requesting a review
Testing
To run unit tests:
pytest tests/unit/test_log.pyTo test logging configuration without Rich logging:
LIGHTSPEED_STACK_DISABLE_RICH_HANDLER=1 lightspeed-stackorpython src/lightspeed_stack.pyTo test logging configuration using the default format string:
lightspeed-stackorpython src/lightspeed_stack.pySummary by CodeRabbit