Skip to content

Fix CLI single-pool auto-selection - #1398

Open
abhinavgautam01 wants to merge 1 commit into
NVIDIA:mainfrom
abhinavgautam01:fix/1396-single-pool-auto-selection
Open

abhinavgautam01 wants to merge 1 commit into
NVIDIA:mainfrom
abhinavgautam01:fix/1396-single-pool-auto-selection

Conversation

@abhinavgautam01

@abhinavgautam01 abhinavgautam01 commented Sep 15, 2026 •

Copy link
Copy Markdown

Description

Fix CLI default-pool auto-selection when no profile default is configured and the service exposes exactly one pool.

The fallback requests /api/pool, which returns a pools mapping, but previously read the node_sets structure belonging to /api/pool_quota. This produced an empty set and incorrectly raised “No default pool set.”

Extract pool names from the correct mapping and update test fixtures to match the API response. Replace the node-set deduplication test with missing-pools coverage and correct a test-helper argument name for Pylint. Preserve existing profile defaults and error handling.

Validation:

  • Confirmed the corrected regression test fails before the fix and passes afterward.
  • Passed 238 tests across the pool, resources, workflow and app suites.
  • Passed Pylint, Bazel type checks and git diff --check.

Fixes #1396

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

  • Bug Fixes
    • Improved default pool detection when pool information is provided in the current response format.
    • Added clearer handling for responses with missing, empty, or ambiguous pool lists, preventing incorrect pool selection.
    • Updated pool validation to provide more consistent behavior across supported response formats.

Signed-off-by: abhinavgautam01 <abgautam1017@gmail.com>
@abhinavgautam01
abhinavgautam01 requested a review from a team as a code owner September 15, 2026 05:42
@github-actions github-actions Bot added the external The author is not in @NVIDIA/osmo-dev label Sep 15, 2026
@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5ccb7b27-af67-4915-b3c6-8292b7f1b0b6

📥 Commits

Reviewing files that changed from the base of the PR and between bca4f74 and f67186d.

📒 Files selected for processing (2)
  • src/cli/pool.py
  • src/cli/tests/test_pool.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Default pool handling

Layer / File(s) Summary
Flat pool response selection and tests
src/cli/pool.py, src/cli/tests/test_pool.py
fetch_default_pool reads pool names from the top-level pools mapping. Tests use the flat response format and cover missing, empty, and multiple pools.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: vvnpn-nv, fernandol-nvidia

Merge Risk: ⚪ Minimal · up to f6718

Default pool selection now matches the response-level pool mapping, with coverage for the relevant empty and missing-pool cases. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing CLI auto-selection when exactly one pool is available.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external The author is not in @NVIDIA/osmo-dev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI single-pool auto-selection reads the wrong API response schema

1 participant