Skip to content

#686 - Add verbose option for config show POOL#690

Merged
RyaliNvidia merged 2 commits intomainfrom
ecolter/verbose-pool-configs
Mar 11, 2026
Merged

#686 - Add verbose option for config show POOL#690
RyaliNvidia merged 2 commits intomainfrom
ecolter/verbose-pool-configs

Conversation

@ecolternv
Copy link
Contributor

@ecolternv ecolternv commented Mar 11, 2026

Description

Add an option to show POOL config --verbose

Issue #686

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • New Features

    • Added --verbose / -v option to the show command to produce enhanced output for POOL configurations.
  • Bug Fixes

    • Prevented --verbose from being used with historical revisions and non-POOL types; now returns a clear error when invalid.
    • Verbose requests are correctly transmitted to the service to produce detailed POOL output.
  • Documentation

    • Updated help and examples to clarify POOL-only verbose usage.

@ecolternv ecolternv requested a review from a team as a code owner March 11, 2026 21:11
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b765b706-db7e-49fc-975d-703429de4a41

📥 Commits

Reviewing files that changed from the base of the PR and between 5d27f3f and a863312.

📒 Files selected for processing (1)
  • src/cli/config.py

📝 Walkthrough

Walkthrough

Added an optional params argument to _get_current_config to forward query parameters to the service client; added a --verbose / -v flag to the show subcommand (applies only to POOL configs) with guards to error on historical or non-POOL uses and passes {'verbose': True} for POOL show requests.

Changes

Cohort / File(s) Summary
Config CLI Enhancement
src/cli/config.py
Extended _get_current_config signature to accept params: Dict[str, Any] | None and forward it to service_client.request; added --verbose / -v option to show subcommand, set request_params = {'verbose': True} for POOL current-config shows, and added guards to error when --verbose is used with historical revisions or non-POOL types; updated help/epilog text accordingly.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as "CLI (show command)"
    participant Parser as "Arg parser"
    participant ConfigFn as "_get_current_config"
    participant Service as "ServiceClient.request"
    participant API as "Remote API"

    CLI->>Parser: parse args (type, revision?, --verbose?)
    Parser-->>CLI: parsed args
    CLI->>ConfigFn: call with service_client, config_type, params?
    ConfigFn->>Service: request(GET /config?type=...&verbose=true?)
    Service->>API: HTTP GET with query params
    API-->>Service: 200 OK + config payload
    Service-->>ConfigFn: response
    ConfigFn-->>CLI: return config or raise error (guard for non-POOL/historical verbose)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through flags and query streams,

I taught the CLI some verbosely dreams,
If not a POOL, I loudly warn—
Else fetch the pool and cheer at dawn! 🥕

🚥 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 describes the main change: adding a --verbose option for the config show POOL command, matching the core functionality added in the PR.
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
  • Commit unit tests in branch ecolter/verbose-pool-configs

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/config.py`:
- Around line 377-382: The code currently only rejects --verbose for
non-revision branches, so ensure you detect revision-style config inputs and
fail fast: if args.verbose is set and args.config represents a revision (e.g.,
contains ':' or otherwise denotes CONFIG:revision) raise
osmo_errors.OSMOUserError with a clear message (similar to the existing one)
instead of silently ignoring it; perform this check before calling
_get_current_config and before any revision-path logic so both POOL:3 or
SERVICE:3 forms trigger the same rejection as non-revision checks (use
args.verbose, args.config and the existing config_history.ConfigHistoryType
constants to guide placement).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6e82735a-63e1-4448-bc96-3578a5ec51d0

📥 Commits

Reviewing files that changed from the base of the PR and between 58fdb99 and 5d27f3f.

📒 Files selected for processing (1)
  • src/cli/config.py

@ecolternv ecolternv force-pushed the ecolter/verbose-pool-configs branch from 5d27f3f to a863312 Compare March 11, 2026 21:55
@RyaliNvidia RyaliNvidia merged commit 5edeab4 into main Mar 11, 2026
9 checks passed
@RyaliNvidia RyaliNvidia deleted the ecolter/verbose-pool-configs branch March 11, 2026 22:21
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.

3 participants