Add requestbody and requestHeader interfaces and simplify Encode() on request #59
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
ryan-gang
commented
Jul 16, 2025
ryan-gang
commented
- Changed the test command in local_testing/test.sh to run 'make test' instead of 'make test_all' for more focused testing.
- Updated the SendAndReceive method in kafka_client to use getter methods for ApiKey, ApiVersion, and CorrelationId, enhancing encapsulation and code clarity.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
ryan-gang
commented
Jul 16, 2025
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
rohitpaulk
requested changes
Jul 20, 2025
…estBodyI - Introduced RequestHeaderI and RequestBodyI interfaces to enhance the encoding structure. - Updated GetHeader method to return RequestHeaderI and GetBody method to return RequestBodyI, improving type safety and clarity in request handling. - Added Encode methods to both interfaces for better integration with the encoder package.
- Refactored encoding methods in ApiVersionsRequest, DescribeTopicPartitionsRequest, and FetchRequest to utilize a common encodeRequest function, improving code reuse and readability. - Updated GetHeader and GetBody methods to return builder.RequestHeaderI and builder.RequestBodyI interfaces, enhancing type safety and consistency across requests. - Adjusted Encode methods for various structs to eliminate pointer receivers, simplifying the code structure.
- Updated encoding methods in ApiVersionsRequest, DescribeTopicPartitionsRequest, and FetchRequest to utilize a common encodeRequest function, streamlining the code and improving readability. - Changed pointer receivers to value receivers in various structs for Encode methods, simplifying the code structure. - Added getter methods for ApiKey, ApiVersion, and CorrelationId in RequestHeader to enhance encapsulation and improve access to header information.
- Updated encoding methods in DescribeTopicPartitionsRequest and FetchRequest to use a common encodeRequest function, enhancing code reuse and readability. - Changed pointer receivers to value receivers in various structs for Encode methods, simplifying the code structure and improving performance.
…coding - Added a panic condition in the Encode method of ApiVersionsRequestBody to ensure that the Version is always >= 3, preventing potential runtime errors. - Refactored the Encode method in ApiVersionsRequest to utilize a common encodeRequest function, improving code consistency and readability.
- Refactored Encode methods in ApiVersionsRequest, DescribeTopicPartitionsRequest, and FetchRequest to utilize a shared encodeRequest function, enhancing code reuse and readability. - Introduced a new request.go file to centralize the encoding logic, streamlining the encoding process for various request types.
…access in kafka_client - Changed the test command in local_testing/test.sh to run 'make test' instead of 'make test_all' for more focused testing. - Updated the SendAndReceive method in kafka_client to use getter methods for ApiKey, ApiVersion, and CorrelationId, enhancing encapsulation and code clarity.
- Replaced instances of kafkaapi.ResponseHeader and kafkaapi.RequestHeader with headers.ResponseHeader and headers.RequestHeader across multiple files, improving code organization and clarity. - Introduced a new headers package to encapsulate request and response header definitions, enhancing modularity and maintainability of the codebase.
- Modified the SendAndReceive method in kafka_client to access ApiKey, ApiVersion, and CorrelationId directly from the header struct, improving code clarity and reducing reliance on getter methods.
Base automatically changed from
refactor-interfaces
to
add-interface-for-requests
July 21, 2025 09:10
There was a problem hiding this comment.
Bug: API Versions Handling Breaks Compatibility
ApiVersionsRequestBody.Encode now panics when Version < 3 instead of encoding the request in the protocol-compatible older format.
This newly-introduced panic causes the library to crash for perfectly valid API Versions 0-2 requests, turning a valid input into an unrecoverable failure.
protocol/api/api_versions_request.go#L17-L26
kafka-tester/protocol/api/api_versions_request.go
Lines 17 to 26 in de70723
Was this report helpful? Give feedback by reacting with 👍 or 👎
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.