[GitHub] [kafka] ableegoldman commented on a change in pull request #10675: KAFKA-12574: remove internal Producer config and auto downgrade logic
ableegoldman commented on a change in pull request #10675:
URL: https://github.com/apache/kafka/pull/10675#discussion_r633185500
##
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsProducer.java
##
@@ -237,13 +237,15 @@ private static boolean isRecoverable(final KafkaException
uncaughtException) {
* @throws TaskMigratedException
*/
protected void commitTransaction(final Map offsets,
- final ConsumerGroupMetadata consumerGroupMetadata) {
+ final ConsumerGroupMetadata
consumerGroupMetadata) {
if (!eosEnabled()) {
throw new IllegalStateException(formatException("Exactly-once is
not enabled"));
}
maybeBeginTransaction();
try {
-producer.sendOffsetsToTransaction(offsets, consumerGroupMetadata);
+// Older brokers don't understand any group metadata beyond the
group id, thus we must downgrade the request for eos-v1
Review comment:
Ack
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10675: KAFKA-12574: remove internal Producer config and auto downgrade logic
ableegoldman commented on a change in pull request #10675:
URL: https://github.com/apache/kafka/pull/10675#discussion_r633185500
##
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsProducer.java
##
@@ -237,13 +237,15 @@ private static boolean isRecoverable(final KafkaException
uncaughtException) {
* @throws TaskMigratedException
*/
protected void commitTransaction(final Map offsets,
- final ConsumerGroupMetadata consumerGroupMetadata) {
+ final ConsumerGroupMetadata
consumerGroupMetadata) {
if (!eosEnabled()) {
throw new IllegalStateException(formatException("Exactly-once is
not enabled"));
}
maybeBeginTransaction();
try {
-producer.sendOffsetsToTransaction(offsets, consumerGroupMetadata);
+// Older brokers don't understand any group metadata beyond the
group id, thus we must downgrade the request for eos-v1
Review comment:
Ack
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10675: KAFKA-12574: remove internal Producer config and auto downgrade logic
ableegoldman commented on a change in pull request #10675:
URL: https://github.com/apache/kafka/pull/10675#discussion_r630628119
##
File path:
clients/src/test/java/org/apache/kafka/clients/producer/MockProducerTest.java
##
@@ -417,6 +417,7 @@ public void
shouldPublishConsumerGroupOffsetsOnlyAfterCommitIfTransactionsAreEna
assertEquals(Collections.singletonList(expectedResult),
producer.consumerGroupOffsetsHistory());
}
+@SuppressWarnings("deprecation")
@Test
Review comment:
Well, it does call deprecated code. But I didn't think to just deprecate
the test itself. That makes more sense, will do
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10675: KAFKA-12574: remove internal Producer config and auto downgrade logic
ableegoldman commented on a change in pull request #10675:
URL: https://github.com/apache/kafka/pull/10675#discussion_r630614141
##
File path:
clients/src/test/java/org/apache/kafka/clients/producer/MockProducerTest.java
##
@@ -417,6 +417,7 @@ public void
shouldPublishConsumerGroupOffsetsOnlyAfterCommitIfTransactionsAreEna
assertEquals(Collections.singletonList(expectedResult),
producer.consumerGroupOffsetsHistory());
}
+@SuppressWarnings("deprecation")
@Test
Review comment:
These are testing the deprecated method specifically. There is an
identical test below for the non-deprecated version of this API. Are you
suggesting we should just go ahead and remove the tests for the deprecated
functionality?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
