KAFKA-19644: Enhance the documentation for producer headers and integration tests #20524
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
Yunyung
changed the title
apalan60
reviewed
Sep 11, 2025
...integration-tests/src/test/java/org/apache/kafka/clients/consumer/PlaintextConsumerTest.java
Show resolved
Hide resolved
chia7712
reviewed
Sep 11, 2025
| headerKey: String | ||
| headerValueLength: varint | ||
| Value: byte[]</code></pre> | ||
| <p>The key of a record header is guaranteed to be non-null, while the value of a record header may be null. The order of headers in a record is preserved when producing and consuming.</p> |
There was a problem hiding this comment.
Could you please add comments to the public API Header?
m1a2st
reviewed
Sep 12, 2025
Comment on lines
198
to
210
| @ClusterTest | ||
| public void testClassicConsumerHeadersOrderPreserved() throws Exception { | ||
| testHeadersOrderPreserved(Map.of( | ||
| GROUP_PROTOCOL_CONFIG, GroupProtocol.CLASSIC.name().toLowerCase(Locale.ROOT) | ||
| )); | ||
| } | ||
|
|
||
| @ClusterTest | ||
| public void testAsyncConsumerHeadersOrderPreserved() throws Exception { | ||
| testHeadersOrderPreserved(Map.of( | ||
| GROUP_PROTOCOL_CONFIG, GroupProtocol.CONSUMER.name().toLowerCase(Locale.ROOT) | ||
| )); | ||
| } |
There was a problem hiding this comment.
Could you move testClassicConsumerHeadersOrderPreserved and testAsyncConsumerHeadersOrderPreserved methods under the testHeaders test and group the related ones together?
|
Thanks for the review, all! Updated based on the comments. |
chia7712
reviewed
Sep 13, 2025
| package org.apache.kafka.common.header; | ||
|
|
||
| /** | ||
| * The key of a header is guaranteed to be non-null, while the value of a header may be null. |
There was a problem hiding this comment.
Please add the comments to each method
|
the flaky is traced by KAFKA-7542 |
chia7712
reviewed
Sep 15, 2025
| } | ||
|
|
||
| @Test | ||
| public void testAddHeadersPreserveOder() { |
| * @param key to return the headers for; must not be null. | ||
| * @return all headers for the given key, in the order they were added in, if NO headers are present an empty iterable is returned. | ||
| */ | ||
| Iterable<Header> headers(String key); |
There was a problem hiding this comment.
Could you please add documents and ut for the Iterable#remove?
chia7712
reviewed
Sep 19, 2025
|
|
||
| /** | ||
| * Returns all headers for the given key, in the order they were added in, if present. | ||
| * The returned iterators {@link org.apache.kafka.common.utils.AbstractIterator#remove()} always throws {@link UnsupportedOperationException}. |
There was a problem hiding this comment.
AbstractIterator is not a public API. Perhaps we could just state the Iterable#remove is unsupported?
There was a problem hiding this comment.
Good point. Updated.
chia7712
approved these changes
Sep 20, 2025
jim0987795064
pushed a commit
to jim0987795064/kafka
that referenced
this pull request
Sep 25, 2025
…ation tests (apache#20524) - Improve the docs for Record Headers. - Add integration tests to verify that the order of headers in a record is preserved when producing and consuming. - Add unit tests for RecordHeaders.java. Reviewers: Ken Huang <[email protected]>, Hong-Yi Chen <[email protected]>, Chia-Ping Tsai <[email protected]>
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.
is preserved when producing and consuming.
Reviewers: Ken Huang [email protected], Hong-Yi Chen
[email protected], Chia-Ping Tsai [email protected]