Remove ROSA cluster discovery from fetch_pricing#17
Merged
Conversation
The kubeconfig scan (oc get nodes across all contexts) provided no value and touched clusters unnecessarily. Remove the rosa_cluster_instance table, v_rosa_estimated_cost view, rosa_cluster_costs MCP tool, and --rosa-only/--skip-rosa CLI flags. ROSA service fee pricing (static published rates) is retained in cloud_pricing since it requires no cluster access. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThe PR replaces dynamic ROSA cluster discovery with static service fee pricing. Database schema elements supporting dynamic instance lookup are removed, static ROSA fees are introduced and fetched alongside AWS pricing, the MCP tool exposing cluster costs is deleted, and tests are updated to reflect the new schema. ChangesROSA Pricing Model Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/fetch_pricing.py (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve ruff formatting failure.
The CI pipeline indicates this file fails
ruff format --check. Runruff format scripts/fetch_pricing.pyto auto-fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/fetch_pricing.py` at line 1, The file scripts/fetch_pricing.py is failing ruff format --check; run the formatter (e.g., `ruff format scripts/fetch_pricing.py`) to auto-apply style fixes so the shebang and file formatting conform to ruff rules, then re-run the CI check and commit the formatted file (targeting the fetch_pricing.py module).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/fetch_pricing.py`:
- Around line 559-566: The if-condition for ROSA service fees is redundant
because fetch_aws is already set as not args.claude_only; replace the compound
condition "if fetch_aws or (not args.claude_only)" with the simpler "if
fetch_aws" where the ROSA block calls fetch_rosa_service_fees(...) and upserts
via upsert_pricing(...), leaving the rest of the block unchanged.
---
Outside diff comments:
In `@scripts/fetch_pricing.py`:
- Line 1: The file scripts/fetch_pricing.py is failing ruff format --check; run
the formatter (e.g., `ruff format scripts/fetch_pricing.py`) to auto-apply style
fixes so the shebang and file formatting conform to ruff rules, then re-run the
CI check and commit the formatted file (targeting the fetch_pricing.py module).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 445eeeb1-029e-4367-83c6-37a252d1a709
📒 Files selected for processing (3)
mcp_server.pyscripts/fetch_pricing.pyscripts/test.sh
💤 Files with no reviewable changes (1)
- mcp_server.py
Comment on lines
+559
to
+566
| # --- ROSA service fees --- | ||
| if fetch_aws or (not args.claude_only): | ||
| print("\n=== ROSA Service Fees (aws.amazon.com/rosa/pricing) ===") | ||
| rows = fetch_rosa_service_fees(now) | ||
| if rows: | ||
| count = upsert_rosa_instances(conn, rows) | ||
| count = upsert_pricing(conn, rows) | ||
| conn.commit() | ||
| print(f" Total: {count} cluster nodes upserted") | ||
| conn.execute( | ||
| "INSERT INTO _meta (key, value) VALUES ('last_rosa_fetch', ?) " | ||
| "ON CONFLICT(key) DO UPDATE SET value=excluded.value", | ||
| (now,), | ||
| ) | ||
| else: | ||
| errors.append("ROSA: no nodes discovered (oc may not be available)") | ||
| print(f" Total: {count} ROSA fee entries upserted") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Redundant condition — simplify to if fetch_aws:
Since fetch_aws = not args.claude_only (line 511), the condition fetch_aws or (not args.claude_only) is logically equivalent to just fetch_aws. The second disjunct is always equal to the first.
♻️ Proposed fix
- # --- ROSA service fees ---
- if fetch_aws or (not args.claude_only):
+ # --- ROSA service fees (bundled with AWS) ---
+ if fetch_aws:
print("\n=== ROSA Service Fees (aws.amazon.com/rosa/pricing) ===")
rows = fetch_rosa_service_fees(now)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # --- ROSA service fees --- | |
| if fetch_aws or (not args.claude_only): | |
| print("\n=== ROSA Service Fees (aws.amazon.com/rosa/pricing) ===") | |
| rows = fetch_rosa_service_fees(now) | |
| if rows: | |
| count = upsert_rosa_instances(conn, rows) | |
| count = upsert_pricing(conn, rows) | |
| conn.commit() | |
| print(f" Total: {count} cluster nodes upserted") | |
| conn.execute( | |
| "INSERT INTO _meta (key, value) VALUES ('last_rosa_fetch', ?) " | |
| "ON CONFLICT(key) DO UPDATE SET value=excluded.value", | |
| (now,), | |
| ) | |
| else: | |
| errors.append("ROSA: no nodes discovered (oc may not be available)") | |
| print(f" Total: {count} ROSA fee entries upserted") | |
| # --- ROSA service fees (bundled with AWS) --- | |
| if fetch_aws: | |
| print("\n=== ROSA Service Fees (aws.amazon.com/rosa/pricing) ===") | |
| rows = fetch_rosa_service_fees(now) | |
| if rows: | |
| count = upsert_pricing(conn, rows) | |
| conn.commit() | |
| print(f" Total: {count} ROSA fee entries upserted") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/fetch_pricing.py` around lines 559 - 566, The if-condition for ROSA
service fees is redundant because fetch_aws is already set as not
args.claude_only; replace the compound condition "if fetch_aws or (not
args.claude_only)" with the simpler "if fetch_aws" where the ROSA block calls
fetch_rosa_service_fees(...) and upserts via upsert_pricing(...), leaving the
rest of the block unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
oc get nodesacross all contexts) fromfetch_pricing.py— it touched clusters unnecessarily and provided no valuerosa_cluster_instancetable,v_rosa_estimated_costview, androsa_cluster_costsMCP tool--rosa-only/--skip-rosaCLI flagsTest plan
uv run scripts/fetch_pricing.pycompletes without anyoccallsscripts/test.shpasses (pricing table checks updated)🤖 Generated with Claude Code