[ISSUE #9339]♻️Inject remoting command factories into NameServer owners - #9340
Conversation
|
🔊@mxsm 🚀Thanks for your contribution🎉! 💡CodeRabbit(AI) will review your code first🔥! Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
WalkthroughNameServer builders now accept and retain a ChangesNameServer command factory injection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Builder
participant NameServerRuntime
participant RequestProcessor
participant RemotingCommandFactory
participant RouteTransport
Builder->>NameServerRuntime: configure RemotingCommandFactory
NameServerRuntime->>RequestProcessor: provide owner-scoped factory
RequestProcessor->>RemotingCommandFactory: create request or response
RemotingCommandFactory-->>RequestProcessor: return configured command
RequestProcessor->>RouteTransport: send encoded remoting command
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Which Issue(s) This PR Fixes(Closes)
Brief Description
RemotingCommandFactoryCompatibility is preserved for request and response codes, remarks, bodies, headers, opaque values, flags, and typed error mappings.
Builder::newremains backward compatible by capturing the application factory once, while embedded owners can select independent remoting versions and serialization types. All direct production static command constructions and global transport error-helper bypasses are removed from the migrated NameServer paths.How Did You Test This Change?
cargo test -p rocketmq-protocol --lib(1483 passed)cargo test -p rocketmq-transport --lib(193 passed)cargo test -p rocketmq-namesrv --lib -- --skip config::tests::test_namesrv_config(255 passed)cargo test -p rocketmq-namesrv --all-features --lib -- --skip config::tests::test_namesrv_config(257 passed)cargo clippy -p rocketmq-namesrv --all-targets --all-features -- -D warningscargo clippy -p rocketmq-protocol -p rocketmq-transport --all-targets --all-features -- -D warningscargo clippy --workspace --no-deps --all-targets --all-features -- -D warningscargo fmt -p <package> -- --checkAggregate
cargo fmt --all -- --checkreaches Windows OS error 206; package-scoped checks passed for all 28 workspace packages.config::tests::test_namesrv_configfails unchanged onmainbecause itsallow_insecure_public_listenerassertion disagrees with the current default; all other NameServer default and all-feature tests pass.Summary by CodeRabbit
New Features
Bug Fixes