jolshan commented on code in PR #14444:
URL: https://github.com/apache/kafka/pull/14444#discussion_r1350932166
##########
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java:
##########
@@ -67,20 +69,31 @@ public ProduceResponse(ProduceResponseData
produceResponseData) {
*/
@Deprecated
public ProduceResponse(Map<TopicPartition, PartitionResponse> responses) {
- this(responses, DEFAULT_THROTTLE_TIME);
+ this(responses, DEFAULT_THROTTLE_TIME, Collections.emptyList());
}
/**
- * Constructor for the latest version
+ * Constructor for versions <= 9
* @param responses Produced data grouped by topic-partition
* @param throttleTimeMs Time in milliseconds the response was throttled
*/
@Deprecated
public ProduceResponse(Map<TopicPartition, PartitionResponse> responses,
int throttleTimeMs) {
- this(toData(responses, throttleTimeMs));
+ this(toData(responses, throttleTimeMs, Collections.emptyList()));
+ }
+
+ /**
+ * Constructor for the latest version
+ * @param responses Produced data grouped by topic-partition
+ * @param throttleTimeMs Time in milliseconds the response was throttled
+ * @param nodeEndpoints List of node endpoints
+ */
+ @Deprecated
Review Comment:
KAFKA-10730 is a pretty dormant JIRA. I do agree that there is some level of
conversion. I wonder if folks have a strong opinion about this conversion
still.
Looking into this further, I see the change would need to be made to
appendRecords and the ProducePartitionStatus. It doesn't look too crazy, but
also understandable this is not the scope for this PR.
I wonder if KAFKA-9682 was premature in deprecating the constructor. I guess
our options are leaving it deprecated and adding a deprecated method or
removing the deprecation until KAFKA-10730 is completed. (I almost just want to
fix it so this doesn't happen in the future 😂 )
##########
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java:
##########
@@ -67,20 +69,31 @@ public ProduceResponse(ProduceResponseData
produceResponseData) {
*/
@Deprecated
public ProduceResponse(Map<TopicPartition, PartitionResponse> responses) {
- this(responses, DEFAULT_THROTTLE_TIME);
+ this(responses, DEFAULT_THROTTLE_TIME, Collections.emptyList());
}
/**
- * Constructor for the latest version
+ * Constructor for versions <= 9
* @param responses Produced data grouped by topic-partition
* @param throttleTimeMs Time in milliseconds the response was throttled
*/
@Deprecated
public ProduceResponse(Map<TopicPartition, PartitionResponse> responses,
int throttleTimeMs) {
- this(toData(responses, throttleTimeMs));
+ this(toData(responses, throttleTimeMs, Collections.emptyList()));
+ }
+
+ /**
+ * Constructor for the latest version
+ * @param responses Produced data grouped by topic-partition
+ * @param throttleTimeMs Time in milliseconds the response was throttled
+ * @param nodeEndpoints List of node endpoints
+ */
+ @Deprecated
Review Comment:
KAFKA-10730 is a pretty dormant JIRA. I do agree that there is some level of
conversion. I wonder if folks have a strong opinion about this conversion
still.
Looking into this further, I see the change would need to be made to
appendRecords and the ProducePartitionStatus. It doesn't look too crazy, but
also understandable this is not the scope for this PR.
I wonder if KAFKA-9682 was premature in deprecating the constructor. I guess
our options are leaving it deprecated and adding a deprecated method or
removing the deprecation until KAFKA-10730 is completed. (I almost just want to
fix it so this doesn't happen in the future 😂 )
--
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
