kafka: decouple topic manager tests from Sarama - #5961
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughKafka admin operations now normalize authorization failures as ChangesKafka authorization handling
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to This change is limited to decoupling Kafka topic-manager tests and normalizing authorization errors at the adapter boundary; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant TopicManager
participant KafkaAdmin
participant Sarama
TopicManager->>KafkaAdmin: Retrieve topic metadata or create topic
KafkaAdmin->>Sarama: Execute Kafka admin request
Sarama-->>KafkaAdmin: Return authorization or general error
KafkaAdmin-->>TopicManager: Return normalized Kafka error
TopicManager->>TopicManager: Apply partition fallback for authorization failure
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
/test all |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lidezhu, wk989898 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-cdc-kafka-integration-light-next-gen-legacy-safepoint |
What problem does this PR solve?
Issue Number: close #5885
Kafka topic manager tests depend on Sarama-specific authorization errors. This couples the behavior tests to the current Kafka client implementation and makes them harder to reuse during the franz-go migration tracked by #5881.
What is changed and how it works?
ErrKafkaAdminAuthorizationFailederror.AdminClientmock and TiCDC-owned errors instead of Sarama types.Check List
Tests
Questions
Will it cause performance regression or break compatibility?
No. The change only normalizes Kafka admin authorization errors at the adapter boundary and preserves the existing topic manager fallback behavior.
Do you need to update user documentation, design documentation or monitoring documentation?
No.
Release note
Summary by CodeRabbit
Bug Fixes
Tests