fix: simplify config get to read directly from viper#974
Conversation
Replace the heavyweight GetBackplaneConfiguration() call with direct viper reads from the config file, avoiding unnecessary OCM connection logic and proxy-testing side effects for a simple config display command. Signed-off-by: Daniel Hall <danhall@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarthall The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe ChangesConfiguration retrieval
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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
🤖 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 `@cmd/ocm-backplane/config/get.go`:
- Around line 32-38: Update the config-path handling around os.Stat to
distinguish a missing file from other filesystem errors: retain the fallback
only for os.IsNotExist(err), and return a wrapped, user-contextual error for
permission, invalid-path, and other failures. Preserve the existing
viper.ReadInConfig behavior when the file exists.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4d69f168-d4d0-4bcf-b135-5c6941ad67c4
📒 Files selected for processing (1)
cmd/ocm-backplane/config/get.go
Return a user-contextual error for permission, invalid-path, and other filesystem failures instead of silently falling through to viper defaults. A missing file still falls through as before. Signed-off-by: Daniel Hall <danhall@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@smarthall: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #974 +/- ##
==========================================
- Coverage 54.58% 54.53% -0.05%
==========================================
Files 82 82
Lines 6308 6313 +5
==========================================
Hits 3443 3443
- Misses 2416 2421 +5
Partials 449 449
🚀 New features to boost your workflow:
|
|
Okay I've written some Ansible modules for setting up your machine for SREP access, one of the things it does is configure the backplane |
|
|
||
| func getConfig(cmd *cobra.Command, args []string) error { | ||
| config, err := config.GetBackplaneConfiguration() | ||
| configPath, err := config.GetConfigFilePath() |
There was a problem hiding this comment.
This may overwrite environment configuration settings. For example : GitHub - backplane-cli config.
3 participants
Summary
GetBackplaneConfiguration()call inconfig getwith direct viper reads from the config fileproxy-urlTest plan
ocm backplane config get urland verify it prints the configured URLocm backplane config get proxy-urlwith both string and array formats in configocm backplane config get alland verify all values display correctlyocm backplane config getwith no config file present (should print empty values)🤖 Generated with Claude Code
Summary by CodeRabbit
getcommand output now includes the proxy URL and the GovCloud setting.alloption now displays all supported configuration values from the saved JSON configuration.