fix(katana): merging server args from file #3044
Conversation
WalkthroughOhayo sensei! This update modifies the default behavior of the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (3)
🪧 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)
crates/katana/cli/src/options.rs (1)
102-106: Ohayo sensei! Consider enhancing the documentation forhttp_modules.The documentation should clarify that no RPC modules are enabled by default and users need to explicitly specify which modules they want to enable.
- /// API's offered over the HTTP-RPC interface. + /// API's offered over the HTTP-RPC interface. + /// + /// By default, no RPC modules are enabled. You must explicitly specify + /// which modules you want to enable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/katana/cli/src/options.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: clippy
- GitHub Check: docs
- GitHub Check: ensure-wasm
- GitHub Check: build
🔇 Additional comments (1)
crates/katana/cli/src/options.rs (1)
146-146:❓ Verification inconclusive
Verify the impact of removing default RPC modules.
This change modifies the default behavior by not enabling any RPC modules by default. This could be a breaking change for systems that expect certain modules to be available.
Let's check for any direct dependencies on the default RPC modules:
🏁 Script executed:
#!/bin/bash # Search for direct usage of ServerOptions where default modules might be expected rg -A 5 "ServerOptions::default\(\)" # Search for places where RPC modules are accessed without explicit checks ast-grep --pattern 'match $obj.http_modules { $$$ }'Length of output: 2215
Ohayo, sensei! Please verify the breaking change regarding default RPC modules.
The change at
crates/katana/cli/src/options.rs:146setshttp_modulestoNoneby default, meaning no RPC modules will be enabled unless explicitly configured. Our search shows several spots in the code (e.g., incrates/katana/cli/src/file.rs,crates/katana/cli/src/args.rs, andcrates/torii/cli/src/args.rs) that compare options againstServerOptions::default(). This indicates that some parts of the system expect the previous default behavior—i.e., having certain modules enabled. We didn’t find explicit runtime checks (via pattern matching onhttp_modules), but the reliance onServerOptions::default()suggests potential impacts.
- Location:
crates/katana/cli/src/options.rs, line 146 (http_modules: None,)- Impact: Systems expecting default RPC modules may break unless they update their configurations.
- Recommendation:
- Double-check that clients and tests dependent on
ServerOptions::default()are updated accordingly.- Update documentation to clearly state the new default behavior and advise explicit configuration for RPC modules.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3044 +/- ##
==========================================
- Coverage 57.76% 57.71% -0.05%
==========================================
Files 437 437
Lines 59305 59336 +31
==========================================
- Hits 34256 34248 -8
- Misses 25049 25088 +39 ☔ View full report in Codecov by Sentry. |
kariy
force-pushed
the
katana/fix-merge-file-config
branch
from
3073a14 to
794f72c
Compare
Summary by CodeRabbit