junrao commented on code in PR #18727:
URL: https://github.com/apache/kafka/pull/18727#discussion_r1937678804
##########
core/src/test/scala/unit/kafka/server/ProduceRequestTest.scala:
##########
@@ -283,6 +286,48 @@ class ProduceRequestTest extends BaseRequestTest {
assertEquals(-1, partitionProduceResponse1.logAppendTimeMs)
}
+ /**
+ * See `ApiKeys.PRODUCE_OLDEST_VERSION` for the details of why we need
special handling for produce request v0-v2 (inclusive).
+ */
+ @Test
+ def testProduceRequestV0ToV2IsRejectedByBroker(): Unit = {
Review Comment:
> We would have to remove testProduceRequestV0ToV2IsRejectedByBroker since
there would be no way to produce with v0-v2. We would probably have to create a
ducktape test to replace that. I am not sure how easy or hard that would be
(ducktape tests typically use clients instead of testing specific requests).
We removed a bunch of old version of other requests. If we just follow their
approach by removing the old version from the protocol definition, there is no
need to test the server rejection logic just for produce request, right?
##########
clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java:
##########
@@ -264,8 +273,11 @@ public boolean hasValidVersion() {
return oldestVersion() <= latestVersion();
}
- public Optional<ApiVersionsResponseData.ApiVersion> toApiVersion(boolean
enableUnstableLastVersion) {
- short oldestVersion = oldestVersion();
+ public Optional<ApiVersionsResponseData.ApiVersion> toApiVersion(boolean
enableUnstableLastVersion,
+
Optional<ApiMessageType.ListenerType> listenerType) {
Review Comment:
When listenerType is not empty, it's always Broker. Perhaps this can just be
a boolean like forBroker?
##########
clients/src/test/java/org/apache/kafka/common/requests/ProduceRequestTest.java:
##########
@@ -277,6 +276,21 @@ public void testMixedIdempotentData() {
assertTrue(RequestTestUtils.hasIdempotentRecords(request));
}
+ @Test
+ public void testBuilderOldestAndLatestAllowed() {
+ ProduceRequest.Builder builder = ProduceRequest.builder(new
ProduceRequestData()
+ .setTopicData(new
ProduceRequestData.TopicProduceDataCollection(Collections.singletonList(
+ new ProduceRequestData.TopicProduceData()
+ .setName("topic")
+ .setPartitionData(Collections.singletonList(new
ProduceRequestData.PartitionProduceData()
+ .setIndex(1)
Review Comment:
This is for PartitionProduceData(). Could we make the indentation clearer?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
