mute logging toolset status for console #1092
Conversation
WalkthroughAdds a mute_log_status flag to Toolset.check_prerequisites and threads it through ToolsetManager methods to optionally suppress prerequisite logging; renames load_toolset_with_status to load_console_toolset_with_status; downgrades some service/prometheus discovery logs to debug; minor CLI string-slicing whitespace fixes and test updates for the renamed method. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ToolsetManager
participant Toolset
participant Logger
Note over Caller,ToolsetManager: High-level prerequisite flow with optional log suppression
Caller->>ToolsetManager: check_toolset_prerequisites(toolsets, mute_log_status=bool)
ToolsetManager->>Toolset: check_prerequisites(mute_log_status=bool)
alt mute_log_status = true
Toolset->>Toolset: Evaluate prerequisites (no status logs)
Note right of Logger: Logging suppressed for failures/success
else mute_log_status = false
Toolset->>Logger: Log prerequisite failures/success
end
Toolset-->>ToolsetManager: result/status
ToolsetManager-->>Caller: aggregated status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
holmes/core/toolset_manager.py (1)
205-222: Fix parameter propagation bug in refresh_toolset_status.The
mute_log_statusparameter on line 205 is defined but ignored on line 221, where a hardcodedTruevalue is passed to_list_all_toolsets. This means the parameter has no effect and will always mute logs, regardless of the caller's intention.Apply this diff to fix the parameter propagation:
def refresh_toolset_status( self, dal: Optional[SupabaseDal] = None, enable_all_toolsets=False, toolset_tags: Optional[List[ToolsetTag]] = None, mute_log_status=True, ): """ Refresh the status of all toolsets and cache the status to a file. Loading cached toolsets status saves the time for runtime tool executor checking the status of each toolset enabled toolset when: - build-in toolset specified in the config and not explicitly disabled - custom toolset not explicitly disabled """ all_toolsets = self._list_all_toolsets( dal=dal, check_prerequisites=True, enable_all_toolsets=enable_all_toolsets, toolset_tags=toolset_tags, - mute_log_status=True, + mute_log_status=mute_log_status, )
🧹 Nitpick comments (1)
holmes/core/toolset_manager.py (1)
481-481: Minor: Extra blank line.An extra blank line was added here, which appears unintentional. Consider removing it to maintain consistent code formatting.
Apply this diff to remove the extra blank line:
- from holmes.core.transformers import registry
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
holmes/core/tools.py(3 hunks)holmes/core/toolset_manager.py(10 hunks)holmes/interactive.py(1 hunks)holmes/plugins/toolsets/prometheus/prometheus.py(4 hunks)holmes/plugins/toolsets/service_discovery.py(1 hunks)tests/core/test_toolset_manager.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff for formatting and linting (configured in pyproject.toml) for all Python code
Type hints are required; code should pass mypy (configured in pyproject.toml)
ALWAYS place Python imports at the top of the file, not inside functions or methods
Files:
holmes/core/tools.pyholmes/interactive.pyholmes/plugins/toolsets/prometheus/prometheus.pytests/core/test_toolset_manager.pyholmes/core/toolset_manager.pyholmes/plugins/toolsets/service_discovery.py
holmes/plugins/toolsets/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Toolsets must be located as holmes/plugins/toolsets/{name}.yaml or holmes/plugins/toolsets/{name}/
Files:
holmes/plugins/toolsets/prometheus/prometheus.pyholmes/plugins/toolsets/service_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Only use pytest markers that are defined in pyproject.toml; never introduce undefined markers/tags
Files:
tests/core/test_toolset_manager.py
tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror the source structure under tests/
Files:
tests/core/test_toolset_manager.py
🧠 Learnings (3)
📚 Learning: 2025-10-01T06:01:13.215Z
Learnt from: mainred
Repo: robusta-dev/holmesgpt PR: 992
File: holmes/plugins/toolsets/__init__.py:106-109
Timestamp: 2025-10-01T06:01:13.215Z
Learning: In the holmesgpt repository (Python), function-scoped imports are acceptable when dealing with optional dependencies that may be missing on some platforms. For example, PrometheusToolset is imported inside the load_python_toolsets function to avoid import-time failures when DISABLE_PROMETHEUS_TOOLSET is true.
Applied to files:
holmes/plugins/toolsets/prometheus/prometheus.py
📚 Learning: 2025-10-05T13:01:12.288Z
Learnt from: CR
Repo: robusta-dev/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T13:01:12.288Z
Learning: Applies to tests/**/test_case.yaml : Do NOT put toolset configuration directly in test_case.yaml; keep toolset config in a separate toolsets.yaml
Applied to files:
tests/core/test_toolset_manager.py
📚 Learning: 2025-08-08T06:15:30.784Z
Learnt from: nilo19
Repo: robusta-dev/holmesgpt PR: 695
File: holmes/core/transformers/registry.py:9-19
Timestamp: 2025-08-08T06:15:30.784Z
Learning: holmes/core/transformers/registry.py: TransformerRegistry is intentionally single-threaded and not designed to be thread-safe; avoid proposing locks unless multi-threaded access is introduced later.
Applied to files:
holmes/core/toolset_manager.py
🧬 Code graph analysis (4)
holmes/core/tools.py (4)
tests/core/test_prompt.py (1)
console(14-15)holmes/core/transformers/transformer.py (1)
Transformer(12-31)holmes/core/transformers/base.py (3)
TransformerError(11-14)BaseTransformer(17-62)name(55-62)holmes/utils/config_utils.py (1)
merge_transformers(11-91)
holmes/plugins/toolsets/prometheus/prometheus.py (2)
holmes/common/openshift.py (1)
load_openshift_token(10-15)holmes/core/tools.py (3)
StructuredToolResult(78-102)StructuredToolResultStatus(51-75)Tool(172-364)
tests/core/test_toolset_manager.py (2)
holmes/core/toolset_manager.py (1)
load_console_toolset_with_status(240-329)holmes/core/tools.py (2)
Toolset(533-782)ToolsetTag(142-145)
holmes/core/toolset_manager.py (1)
holmes/core/tools.py (3)
Toolset(533-782)check_prerequisites(682-761)ToolsetTag(142-145)
🪛 Ruff (0.14.2)
holmes/core/toolset_manager.py
205-205: Unused method argument: mute_log_status
(ARG002)
⏰ 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). (2)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (7)
holmes/interactive.py (1)
210-210: LGTM! Clear guidance for users.The updated welcome banner effectively directs users to the
/toolscommand for viewing toolset information, aligning with the PR objective to consolidate toolset status display.holmes/plugins/toolsets/service_discovery.py (1)
38-40: LGTM! Appropriate log level reduction.Downgrading the service discovery URL log from info to debug reduces console noise while preserving the information for debugging purposes. This aligns well with the PR's goal to streamline console output.
holmes/plugins/toolsets/prometheus/prometheus.py (1)
1552-1552: LGTM! Consistent log level adjustment.The logging level change from info to debug for Prometheus auto-discovery is consistent with similar changes in
service_discovery.pyand supports the PR objective to reduce console verbosity.tests/core/test_toolset_manager.py (1)
127-166: Tests updated correctly, but consider additional coverage.The test updates properly reflect the method rename from
load_toolset_with_statustoload_console_toolset_with_status. All assertions remain valid.However, consider adding test coverage for the new
mute_log_statusparameter behavior. Currently, there are no tests verifying that logging is actually suppressed whenmute_log_status=True. This could be tested by:
- Mocking the logger and asserting specific log calls are/aren't made
- Verifying the parameter is correctly propagated through the call chain
Would you like me to suggest a test implementation?
holmes/core/tools.py (1)
682-761: LGTM! Clean implementation of logging suppression.The
mute_log_statusparameter is well-implemented with:
- Sensible default (
False) maintaining backward compatibility- Clear docstring explaining the parameter's purpose
- Consistent application to both success (✅) and failure (❌) logging paths
holmes/core/toolset_manager.py (2)
240-329: LGTM! Well-structured console-specific loader.The rename from
load_toolset_with_statustoload_console_toolset_with_statusclearly communicates the method's console-focused purpose. The implementation correctly:
- Adds "Loading toolsets..." message for user feedback
- Consistently uses
mute_log_status=Trueto suppress detailed prerequisite logs- Provides a consolidated summary message directing users to
/toolscommand
79-143: LGTM! Consistent mute_log_status propagation.The
mute_log_statusparameter is properly threaded through:
_list_all_toolsetsreceives and passes it tocheck_toolset_prerequisitescheck_toolset_prerequisitesforwards it to each toolset'scheck_prerequisitesmethod- Default value (
False) preserves existing behavior
To avoid printing the lengthy toolset status, we mute the status for console, since we have /tools to print the toolset status.
Also, we unform the output for both using and not using toolset.
With cache
Without cache