AndrewJSchofield commented on code in PR #20691:
URL: https://github.com/apache/kafka/pull/20691#discussion_r2463896225
##########
tools/src/test/java/org/apache/kafka/tools/consumer/group/ShareGroupCommandTest.java:
##########
@@ -245,7 +245,7 @@ public void testDescribeOffsetsOfExistingGroupWithNulls()
throws Exception {
DescribeShareGroupsResult describeShareGroupsResult =
mock(DescribeShareGroupsResult.class);
ShareGroupDescription exp = new ShareGroupDescription(
firstGroup,
- List.of(new ShareMemberDescription("memid1", "clId1", "host1",
new ShareMemberAssignment(
+ List.of(new ShareMemberDescription("memid1",
Optional.of("rackId1"), "clId1", "host1", new ShareMemberAssignment(
Review Comment:
Let's have an `Optional.empty()` in this case so we are at least exercising
that case too.
##########
clients/src/test/java/org/apache/kafka/clients/admin/internals/DescribeConsumerGroupsHandlerTest.java:
##########
@@ -158,6 +158,7 @@ public void testSuccessfulHandleConsumerGroupResponse() {
new MemberDescription(
"memberId",
Optional.of("instanceId"),
+ Optional.of("rackid"),
Review Comment:
nit: `rackId`?
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -81,7 +111,7 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
*/
@Deprecated
Review Comment:
`@Deprecated(since = "4.0", forRemoval = true)`
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -102,7 +132,7 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
Review Comment:
4.0
##########
clients/src/test/java/org/apache/kafka/clients/admin/internals/DescribeConsumerGroupsHandlerTest.java:
##########
@@ -172,6 +173,7 @@ public void testSuccessfulHandleConsumerGroupResponse() {
new MemberDescription(
"memberId-classic",
Optional.of("instanceId-classic"),
+ Optional.of("rackid"),
Review Comment:
Let's have `Optional.empty()` too to increase the variety of the parameters
being used.
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -57,7 +60,34 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
+ */
+ @Deprecated
+ public MemberDescription(
+ String memberId,
+ Optional<String> groupInstanceId,
+ String clientId,
+ String host,
+ MemberAssignment assignment,
+ Optional<MemberAssignment> targetAssignment,
+ Optional<Integer> memberEpoch,
+ Optional<Boolean> upgraded
+ ) {
+ this(
+ memberId,
+ groupInstanceId,
+ Optional.empty(),
+ clientId,
+ host,
+ assignment,
+ targetAssignment,
+ memberEpoch,
+ upgraded
+ );
+ }
+
+ /**
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
Review Comment:
This one was deprecated in 4.0.
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -57,7 +60,34 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
+ */
+ @Deprecated
+ public MemberDescription(
+ String memberId,
+ Optional<String> groupInstanceId,
+ String clientId,
+ String host,
+ MemberAssignment assignment,
+ Optional<MemberAssignment> targetAssignment,
+ Optional<Integer> memberEpoch,
+ Optional<Boolean> upgraded
+ ) {
+ this(
+ memberId,
+ groupInstanceId,
+ Optional.empty(),
+ clientId,
+ host,
+ assignment,
+ targetAssignment,
+ memberEpoch,
+ upgraded
+ );
+ }
+
+ /**
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
*/
@Deprecated
Review Comment:
`@Deprecated(since = "4.0", forRemoval = true)`
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -57,7 +60,34 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
+ */
+ @Deprecated
Review Comment:
Better would be `@Deprecated(since = "4.2", forRemoval = true)`. You may
need to add more annotations to test cases which use the old constructor.
##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -4758,6 +4758,7 @@ public void testDescribeOldAndNewConsumerGroups() throws
Exception {
new MemberDescription(
"memberId",
Optional.of("instanceId"),
+ Optional.of("rackid"),
Review Comment:
nit: `rackId`?
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -81,7 +111,7 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
Review Comment:
Since 4.0
##########
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java:
##########
@@ -102,7 +132,7 @@ public MemberDescription(
}
/**
- * @deprecated Since 4.0. Use {@link #MemberDescription(String, Optional,
String, String, MemberAssignment, Optional, Optional, Optional)} instead.
+ * @deprecated Since 4.2. Use {@link #MemberDescription(String, Optional,
Optional, String, String, MemberAssignment, Optional, Optional, Optional)}
instead.
*/
@Deprecated
Review Comment:
`@Deprecated(since = "4.0", forRemoval = true)`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
