-
Notifications
You must be signed in to change notification settings - Fork 175
🐛 Fixes some bugs for UpdateAndCreateTopic, GetTopicConfig and GetConsumerConnectionList #4171
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
Conversation
…et_connection_set()` returns a cloned copy.
🔊@BrightX 🚀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💥. |
WalkthroughSwitched topic registration to pass the broker's name, added a public insertion method for ConsumerConnection, applied serde attributes to TopicConfigAndQueueMapping fields, and a formatting-only argument adjustment in TopicConfigManager. No control-flow or error-handling behavior changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant BrokerRuntime
participant BrokerOuterAPI
Note over BrokerRuntime,BrokerOuterAPI #D6EAF8: Topic registration now uses broker_name
BrokerRuntime->>BrokerOuterAPI: register_single_topic_all(topic, broker_name, ...)
BrokerOuterAPI-->>BrokerRuntime: registration result
sequenceDiagram
autonumber
participant AdminHandler
participant ConsumerConnection
Note over AdminHandler,ConsumerConnection #F9E79F: Insert connection via public API
AdminHandler->>ConsumerConnection: connection_set_insert(connection)
ConsumerConnection-->>AdminHandler: ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4171 +/- ##
=======================================
Coverage 26.51% 26.51%
=======================================
Files 576 576
Lines 81510 81510
=======================================
Hits 21611 21611
Misses 59899 59899 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🔊@BrightX 🚀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💥. |
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.
Thanks for your contribution. LGTM
Which Issue(s) This PR Fixes(Closes)
Fixes: #4176
TopicConfigAndQueueMapping
serde forTopicConfig
andmappingDetail
UpdateAndCreateTopic
cannot setbroker_name
andattributes
connection_set
ofbody_data
is always empty, becauseget_connection_set()
returns a cloned copy.Brief Description
How Did You Test This Change?
Summary by CodeRabbit
New Features
Refactor