fix(tools/cosmovisor): fix wrong dir prepare-upgrade when non archive
#23652
Conversation
📝 WalkthroughWalkthroughThe changes update the cosmovisor tool by introducing a new version entry (v1.7.1) in the changelog that documents a bug fix for incorrect directory placement in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Cosmovisor CLI
participant CFG as Config
participant L as Logger
U->>C: Initiate prepare-upgrade command
C->>CFG: Retrieve upgrade directory (using UpgradeDir)
CFG-->>C: Return directory path
C->>C: Execute upgrade binary using computed path
C->>L: Log errors if encountered
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Nitpick comments (1)
tools/cosmovisor/CHANGELOG.md (1)
43-43: Clear Bug Fix Description with Minor Suggestion
The bug fix entry "* #23652 Fix issue with wrong directory placement when usingprepare-upgradefor non archive." clearly references the issue and describes the correction. Consider inserting a hyphen to read "non-archive" for improved clarity and consistency.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tools/cosmovisor/CHANGELOG.md(1 hunks)tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.md`: "Assess the documentation for misspellings, gramm...
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
tools/cosmovisor/CHANGELOG.md
`**/*.go`: Review the Golang code for conformity with the Ub...
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: tests (00)
- GitHub Check: test-system-v2
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (3)
tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go (1)
75-75: LGTM! The directory path construction fix looks correct.The change from manually constructing the path to using
cfg.UpgradeDirmethod is a good improvement that:
- Fixes the incorrect directory placement issue for non-archive upgrades
- Makes the code more maintainable by centralizing the directory path logic
- Reduces the chance of path construction errors
tools/cosmovisor/CHANGELOG.md (2)
39-39: Version Entry Added Correctly
The addition of the## v1.7.1 - 2025-01-12version entry adheres to the changelog guidelines, clearly indicating the new version and its release date.
41-41: Proper Definition of Bug Fixes Section
The "### Bug Fixes" header is appropriately defined and maintains consistency with previous entries, ensuring that users can quickly identify the nature of the fixes included in this release.
|
Can you walk us though the flow of this and what the actual error is? It's unclear wha the error was. What does |
|
I was just going to open a PR to fix this but it looks like someone else already ran into it. Given our the correct structure is |
Description
Fixes the wrong upgrade directory when using cosmovisor prepare-upgrade with non archives.
It actually worked for archives by luck as DownloadUpgrade[1] is opinionated, the way to get the upgrade directory was simply wrong:
[1]:
cc @johnletey
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit