Skip to content

Conversation

@richtja
Copy link
Contributor

@richtja richtja commented Dec 1, 2025

The job.output.loglevel configuration option was not being respected for external loggers outside of the avocado logging namespace. This fix ensures that external loggers properly inherit and respect the configured log level from job.output.loglevel.

Reference: #6239

Summary by CodeRabbit

  • Bug Fixes
    • Fixed root logger configuration to properly respect the configured log level, ensuring consistent log output visibility and default handling from application startup onwards.

✏️ Tip: You can customize this high-level summary in your review settings.

The job.output.loglevel configuration option was not being respected
for external loggers outside of the avocado logging namespace. This
fix ensures that external loggers properly inherit and respect the
configured log level from job.output.loglevel.

Signed-off-by: Jan Richter <[email protected]>
@richtja richtja added this to the 112 - Codename TBD milestone Dec 1, 2025
@richtja richtja self-assigned this Dec 1, 2025
@richtja richtja added the bug label Dec 1, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

The root logger configuration in avocado/core/utils/messages.py is updated to initialize the root logger's level to the configured log_level value instead of leaving it at NOTSET. This change affects how log messages are handled before individual loggers are explicitly configured, potentially altering the visibility and propagation of early log entries in the application lifecycle.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file modification with a straightforward configuration adjustment
  • No changes to public APIs or exported signatures
  • No control-flow or error-handling logic modifications
  • Review should focus on verifying that the log_level variable is correctly accessed and whether this change could impact logging behavior during application initialization

Pre-merge checks

✅ 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 describes the main fix: addressing job.output.loglevel not applying to external loggers, which aligns with both the code changes and PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74b7379 and 2cb471a.

📒 Files selected for processing (1)
  • avocado/core/utils/messages.py (1 hunks)
⏰ 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). (16)
  • GitHub Check: Linux with Python 3.9
  • GitHub Check: Egg task debian:11.0
  • GitHub Check: Version task ubi:8.8
  • GitHub Check: Version task fedora:40
  • GitHub Check: Fedora selftests
  • GitHub Check: Egg task ubuntu:22.04
  • GitHub Check: Build Egg for Python 3.9
  • GitHub Check: Egg task ubi:9.2
  • GitHub Check: Egg task debian:12.4
  • GitHub Check: Podman Egg task
  • GitHub Check: Version task ubuntu:22.04
  • GitHub Check: Version task debian:12.4
  • GitHub Check: Podman spawner with 3rd party runner plugin
  • GitHub Check: Static checks
  • GitHub Check: macOS with Python 3.11
  • GitHub Check: Code Coverage (3.11)
🔇 Additional comments (1)
avocado/core/utils/messages.py (1)

259-263: Setting root logger level to log_level correctly makes external loggers honor job.output.loglevel

This change does exactly what the PR intends: external loggers that keep their level at NOTSET will now inherit the job-configured log_level via the root logger, instead of being stuck at the default WARNING threshold.

One behavioral nuance to double‑check: start_logging() now unconditionally overrides any pre-existing root level (e.g., if an embedding application configured logging before Avocado runs). If start_logging() is only used in the dedicated runner process that “owns” logging, this is fine and arguably desirable; if Avocado is sometimes embedded into larger applications that expect to keep their own root level, it may be worth documenting this or revisiting the assumption.

Please verify in your typical deployment that:

  • A third-party logger (e.g., logging.getLogger("external.lib") with no explicit setLevel) now respects job.output.loglevel, and
  • There are no supported embedding scenarios that rely on preserving a preconfigured root logger level.

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.

@mr-avocado mr-avocado bot moved this to Review Requested in Default project Dec 1, 2025
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.46%. Comparing base (74b7379) to head (2cb471a).
⚠️ Report is 22 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6248      +/-   ##
==========================================
- Coverage   73.48%   73.46%   -0.03%     
==========================================
  Files         206      206              
  Lines       22494    22494              
==========================================
- Hits        16530    16525       -5     
- Misses       5964     5969       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@richtja
Copy link
Contributor Author

richtja commented Dec 8, 2025

This solution has been validated by @marioaag in #6239 (reply in thread)

Copy link
Contributor

@clebergnu clebergnu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@richtja richtja merged commit 6bc4934 into avocado-framework:master Dec 16, 2025
111 of 115 checks passed
@github-project-automation github-project-automation bot moved this from Review Requested to Done 113 in Default project Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done 113

Development

Successfully merging this pull request may close these issues.

2 participants