-
Couldn't load subscription status.
- Fork 3.2k
[KIP-482]: Update Describe Configs to v4 #5200
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
base: master
Are you sure you want to change the base?
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
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.
Pull Request Overview
Updates the Kafka DescribeConfigs API from version 1 to version 4, implementing KIP-482 to support flexible messaging format and add new configuration metadata fields.
- Adds support for config type and documentation information in ConfigEntry structures
- Updates request/response parsing to handle flexible version protocol with tagged fields
- Exposes new public APIs for accessing config type and documentation
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rdkafka.h | Adds ConfigType enum and new public API functions for type and documentation |
| src/rdkafka_admin.h | Extends ConfigEntry structure with type and documentation fields |
| src/rdkafka_admin.c | Implements new API functions and updates response parsing for v4 protocol |
| src/rdkafka_request.c | Updates request creation to use flexible version format and API v4 |
| tests/0081-admin.c | Updates test output to include new config type and documentation fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| int16_t api_version; | ||
|
|
||
| api_version = rd_kafka_buf_ApiVersion(reply); | ||
|
|
Copilot
AI
Sep 22, 2025
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.
[nitpick] The variable api_version should be declared at the point of initialization rather than separately. Consider combining the declaration and assignment: int16_t api_version = rd_kafka_buf_ApiVersion(reply);
| int16_t api_version; | |
| api_version = rd_kafka_buf_ApiVersion(reply); | |
| int16_t api_version = rd_kafka_buf_ApiVersion(reply); |
| } | ||
|
|
||
| if (api_version >= 3) { | ||
| rd_kafka_buf_read_i8(reply, &config_type); |
Copilot
AI
Sep 22, 2025
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.
The assignment to entry->type should cast config_type to rd_kafka_ConfigType_t for type safety, since config_type is declared as int8_t but the field expects rd_kafka_ConfigType_t.
| rd_kafka_buf_read_i8(reply, &config_type); | |
| entry->type = (rd_kafka_ConfigType_t)config_type; |
| * | ||
| * @param entry Entry to get type for. | ||
| * | ||
| * @remark The lifetime of the returned entry is the same as \p conf . |
Copilot
AI
Sep 22, 2025
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.
The parameter reference \\p conf is incorrect. It should reference \\p entry since that's the actual parameter name for this function.
| * | ||
| * @param entry Entry to get documentation for. | ||
| * | ||
| * @remark The lifetime of the returned entry is the same as \p conf . |
Copilot
AI
Sep 22, 2025
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.
The parameter reference \\p conf is incorrect. It should reference \\p entry since that's the actual parameter name for this function.
Update DescribeConfigs api to version 4(first flexible version)