fix(logger): do not output WARN level logs for non-root modules#3760
fix(logger): do not output WARN level logs for non-root modules#3760aignas wants to merge 1 commit intobazel-contrib:mainfrom
Conversation
A user has reported issues which actually resulted from a particular `bazel` version being used and once he updated, the issues disappeared because of changes in implicit non-root module dependencies. In order to reduce the warning spam of non-root modules, set the default level to `ERROR` for non-root module parsing context and `WARN` for root module context. This means that the users will not get console output that they have no way to fix without patching upstream dependencies. At the same time ensure that the name of the module is printed together with the logs to better understand where something is coming from. Fixes bazel-contrib#3749
There was a problem hiding this comment.
Code Review
This pull request reduces the default log verbosity for non-root modules in Bzlmod by passing the module context to the logger. Review feedback identifies that the verbosity mapping in repo_utils.bzl must be updated to include ERROR and WARN levels to effectively silence warnings for non-root modules. Additionally, it is recommended to pass the module context to the global logger in python.bzl to ensure consistent behavior across the extension.
| verbosity = { | ||
| "DEBUG": 2, |
There was a problem hiding this comment.
The verbosity mapping is missing entries for "ERROR" and "WARN". Because of the .get(verbosity_level, 0) call on line 76, both "ERROR" and "WARN" currently resolve to a verbosity of 0. Since the warn method (line 108) is enabled when the verbosity is >= 0, setting the default level to "ERROR" for non-root modules (line 62) does not actually silence the warnings.
To fix this, "ERROR" should be mapped to -1 (to match "FAIL"), which will correctly silence warnings (level 0) while still allowing failures to be reported. Additionally, "WARN" should be explicitly mapped to 0 for clarity.
| verbosity = { | |
| "DEBUG": 2, | |
| verbosity = { | |
| "DEBUG": 2, | |
| "ERROR": -1, | |
| "WARN": 0, |
| # For all other processing(after parsing the modules) let's use a single logger. | ||
| logger = repo_utils.logger(module_ctx, "python") |
There was a problem hiding this comment.
This logger is created without passing the mod argument, which means it will default to WARN verbosity even when the extension is being used in a non-root module context. To be consistent with the goal of reducing log spam for non-root modules, this logger should also respect the root vs. non-root status.
Since module_ctx.modules[0] is guaranteed to be the root module if it uses the extension (and a non-root module otherwise), passing it as the mod argument will correctly set the default verbosity for the global extension processing. Also, there is a minor typo in the comment (missing space).
| # For all other processing(after parsing the modules) let's use a single logger. | |
| logger = repo_utils.logger(module_ctx, "python") | |
| # For all other processing (after parsing the modules) let's use a single logger. | |
| logger = repo_utils.logger(module_ctx, "python", mod = module_ctx.modules[0]) |
A user has reported issues which actually resulted from a particular
bazelversion being used and once he updated, the issues disappearedbecause of changes in implicit non-root module dependencies.
In order to reduce the warning spam of non-root modules, set the default
level to
ERRORfor non-root module parsing context andWARNfor rootmodule context. This means that the users will not get console output
that they have no way to fix without patching upstream dependencies.
At the same time ensure that the name of the module is printed together
with the logs to better understand where something is coming from.
Fixes #3749