-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: create v2 upgrading guide skeleton #22696
Conversation
Warning Rate limit exceeded@julienrbrt has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new document Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant OldApp as v0.50/v0.52 App
participant NewApp as v2.0.0 App
Dev->>OldApp: Current Application
Dev->>NewApp: Migrate Application
Note over Dev: Review SimApp Section
Note over Dev: Update Module Configurations
Note over Dev: Migrate sdk.Context
Note over Dev: Update AnteHandler to TxValidator
NewApp-->>Dev: Upgraded Application
Possibly related issues
Possibly related PRs
Suggested reviewers
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
|
Opening, I'll complete after my holiday, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
UPGRADING_v2.md (2)
1-4
: Fix grammar and enhance version informationThe introduction should be more specific about the versions covered in this guide.
Apply these changes:
# Upgrading Cosmos SDK v2.x.y -This guide provides instructions for upgrading to specific versions of Cosmos SDK. +This guide provides instructions for upgrading to Cosmos SDK v2.0.0 from v0.50.x and v0.52.x versions.
39-40
: Fix documentation link and improve clarityThe link to the v0.52 UPGRADING.md file needs to be more robust.
Apply these changes:
-Read the module section from the v0.52 [UPGRADING.md](UPGRADING.md) file for more information. -Then simply follow the instructions from the [v0.52 section](#upgrading-from-v052x-to-v200) from this file. +1. First, follow the module migration steps in the [v0.52 Upgrade Guide](https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/UPGRADING.md#modules) +2. Then, complete the v2.0.0 migration by following the [v0.52.x to v2.0.0](#upgrading-from-v052x-to-v200) section above.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UPGRADING_v2.md
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
UPGRADING_v2.md (1)
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
UPGRADING_v2.md
[grammar] ~8-~8: Consider using the singular form after the singular determiner “This”.
Context: ...dule and application dependencies. This guides assumes you have already made your modu...
(AGREEMENT_SENT_START)
[grammar] ~18-~18: It appears that a hyphen is missing in the plural noun “to-dos”?
Context: ...odule configuration #22371 ### Server TODOs: * Telemetry server is another port (1...
(TO_DO_HYPHEN)
UPGRADING_v2.md
Outdated
#### `app.go` | ||
|
||
#### `root.go` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add content to empty sections
The app.go
and root.go
sections are empty but crucial for the upgrade process.
Would you like me to help draft the content for these sections? They should include:
- Required changes to application initialization
- New dependency injection setup
- Configuration changes
UPGRADING_v2.md
Outdated
TODOs: | ||
|
||
* Telemetry server is another port (1328) | ||
* New REST server for querying modules (8080) -> Use post and type_url (docs at server/v2/api/rest/README.md) | ||
* gRPC: new service to query the modules gRPC messages, without going via module services | ||
* gRPC: external gRPC services no more registered in the application router (e.g. nodeservice, cmtservice, authtx service) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Format and expand TODOs section
The TODOs section needs proper formatting and more detailed explanations.
Apply these changes:
-TODOs:
-
-* Telemetry server is another port (1328)
-* New REST server for querying modules (8080) -> Use post and type_url (docs at server/v2/api/rest/README.md)
-* gRPC: new service to query the modules gRPC messages, without going via module services
-* gRPC: external gRPC services no more registered in the application router (e.g. nodeservice, cmtservice, authtx service)
+### Breaking Changes
+
+#### Telemetry Server
+* The telemetry server now runs on a dedicated port (1328)
+* Migration required: Update your monitoring configuration to use the new port
+
+#### REST Server
+* New dedicated REST server for module queries runs on port 8080
+* Uses POST method with type_url for queries
+* Refer to detailed documentation at `server/v2/api/rest/README.md`
+
+#### gRPC Changes
+* New service implementation for module gRPC message queries
+* External gRPC services (nodeservice, cmtservice, authtx service) are no longer registered in the application router
+* Migration required: Update your service discovery and client configurations
📝 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.
TODOs: | |
* Telemetry server is another port (1328) | |
* New REST server for querying modules (8080) -> Use post and type_url (docs at server/v2/api/rest/README.md) | |
* gRPC: new service to query the modules gRPC messages, without going via module services | |
* gRPC: external gRPC services no more registered in the application router (e.g. nodeservice, cmtservice, authtx service) | |
### Breaking Changes | |
#### Telemetry Server | |
* The telemetry server now runs on a dedicated port (1328) | |
* Migration required: Update your monitoring configuration to use the new port | |
#### REST Server | |
* New dedicated REST server for module queries runs on port 8080 | |
* Uses POST method with type_url for queries | |
* Refer to detailed documentation at `server/v2/api/rest/README.md` | |
#### gRPC Changes | |
* New service implementation for module gRPC message queries | |
* External gRPC services (nodeservice, cmtservice, authtx service) are no longer registered in the application router | |
* Migration required: Update your service discovery and client configurations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
UPGRADING_v2.md (3)
1-5
: Enhance the introduction sectionThe introduction should provide more context about what v2 brings and why users should upgrade.
# Upgrading Cosmos SDK v2 [DRAFT] This guide provides instructions for upgrading to specific versions of Cosmos SDK. Note, always read the **SimApp** section for more information on application wiring updates. + +## Overview + +Cosmos SDK v2 introduces significant architectural changes including: +- Dependency injection with depinject +- New server architecture +- Core service migrations + +This guide will help you navigate these changes and successfully upgrade your application.
6-6
: Fix incomplete version numberThe version number in the heading is incomplete.
-## Upgrading from v0.52.x to v2. +## Upgrading from v0.52.x to v2.0.0🧰 Tools
🪛 Markdownlint (0.37.0)
6-6: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
16-16
: Fix documentation referenceThe documentation reference needs proper linking.
-* [Documentation]: Module configuration #22371 +* [Documentation: Module configuration](https://github.com/cosmos/cosmos-sdk/issues/22371)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UPGRADING_v2.md
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
UPGRADING_v2.md (1)
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
UPGRADING_v2.md
[uncategorized] ~27-~27: Possible missing comma found.
Context: ... the migration to server/v2 and runtime/v2 some changes are required in the `root....
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.37.0)
UPGRADING_v2.md
6-6: Punctuation: '.'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
62-62: null
Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (3)
UPGRADING_v2.md (3)
13-16
: Include external documentation inline
The HackMD link should be replaced with inline documentation to ensure long-term availability and consistency.
29-34
: Add content to app.go section
The app.go
section is empty but crucial for the upgrade process.
20-23
: 🛠️ Refactor suggestion
Improve server section structure
The server changes need better organization and more detailed explanations.
-* Telemetry server is another port (7180)
-* New REST server for querying modules (8080) -> Use post and type_url (docs at server/v2/api/rest/README.md)
-* gRPC: new service to query the modules gRPC messages, without going via module services
-* gRPC: external gRPC services no more registered in the application router (e.g. nodeservice, cmtservice, authtx service)
+### Server Changes
+
+#### Telemetry Server
+* Now runs on dedicated port `7180`
+* Configuration required: Update monitoring setup to use new port
+
+#### REST API
+* New query server on port `8080`
+* Endpoints accept POST requests with `type_url`
+* Detailed documentation: `server/v2/api/rest/README.md`
+
+#### gRPC Services
+* New direct query service for module messages
+* External services (nodeservice, cmtservice, authtx) moved out of application router
+* Migration required: Update service discovery and client configurations
Likely invalid or redundant comment.
```go | ||
// wire server commands | ||
return serverv2.AddCommands[T]( | ||
rootCmd, | ||
logger, | ||
simApp, | ||
deps.GlobalConfig, | ||
initServerConfig(), | ||
deps.ConsensusServer, | ||
grpcServer, | ||
storeComponent, | ||
telemetryServer, | ||
restServer, | ||
grpcgatewayServer, | ||
) | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Document server configuration parameters
The server configuration example needs explanation of each parameter.
```go
// wire server commands
return serverv2.AddCommands[T](
rootCmd,
logger,
simApp,
deps.GlobalConfig,
initServerConfig(),
deps.ConsensusServer,
grpcServer,
storeComponent,
telemetryServer,
restServer,
grpcgatewayServer,
)
+Parameters explained:
+* rootCmd
: Root command of your application
+* logger
: Application logger instance
+* simApp
: Your application instance
+* deps.GlobalConfig
: Global configuration settings
+* initServerConfig()
: Server-specific configuration
+* deps.ConsensusServer
: Consensus server component
+* grpcServer
: gRPC server instance
+* storeComponent
: State store component
+* telemetryServer
: Metrics server (port 7180)
+* restServer
: REST API server (port 8080)
+* grpcgatewayServer
: gRPC-Gateway server
<!-- This is an auto-generated comment by CodeRabbit -->
Read the module section from the v0.52 [UPGRADING.md](UPGRADING.md) file for more information. | ||
Then simply follow the instructions from the [v0.52 section](#upgrading-from-v052x-to-v200) from this file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix broken link and enhance migration path
The link fragment is incorrect and the migration path needs more detail.
-Read the module section from the v0.52 [UPGRADING.md](UPGRADING.md) file for more information.
-Then simply follow the instructions from the [v0.52 section](#upgrading-from-v052x-to-v200) from this file.
+1. First, read the module section from the v0.52 [UPGRADING.md](UPGRADING.md) file
+2. Then follow the instructions in the [v0.52.x to v2.0.0 section](#upgrading-from-v052x-to-v200) above
+
+### Migration Checklist
+- [ ] Update module dependencies to v0.52.x compatible versions
+- [ ] Apply all breaking changes from v0.52.x
+- [ ] Follow the v2.0.0 upgrade steps above
+- [ ] Test thoroughly with the new architecture
📝 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.
Read the module section from the v0.52 [UPGRADING.md](UPGRADING.md) file for more information. | |
Then simply follow the instructions from the [v0.52 section](#upgrading-from-v052x-to-v200) from this file. | |
1. First, read the module section from the v0.52 [UPGRADING.md](UPGRADING.md) file | |
2. Then follow the instructions in the [v0.52.x to v2.0.0 section](#upgrading-from-v052x-to-v200) above | |
### Migration Checklist | |
- [ ] Update module dependencies to v0.52.x compatible versions | |
- [ ] Apply all breaking changes from v0.52.x | |
- [ ] Follow the v2.0.0 upgrade steps above | |
- [ ] Test thoroughly with the new architecture |
🧰 Tools
🪛 Markdownlint (0.37.0)
62-62: null
Link fragments should be valid
(MD051, link-fragments)
(cherry picked from commit b2c39e2)
Description
ref: #22531
I am upgrading Ignite CLI to support v2 scaffolding and modules (ignite/cli#4432 & more), so I best can just document everything needed here as well :)See #22696 (comment)
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.md
Reviewers 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
UPGRADING_v2.md
detailing migration processes and key changes for users upgrading from earlier versions.