chia7712 commented on code in PR #20598:
URL: https://github.com/apache/kafka/pull/20598#discussion_r3566688272
##########
clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/DescribeFeaturesTest.java:
##########
@@ -29,15 +32,39 @@
import org.apache.kafka.server.common.StreamsVersion;
import org.apache.kafka.server.common.TransactionVersion;
+import java.util.Arrays;
import java.util.Map;
+import java.util.Objects;
import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
public class DescribeFeaturesTest {
+ @ClusterTest(types = {Type.KRAFT})
Review Comment:
```java
@ClusterTest(types = {Type.KRAFT})
public void testApiVersions(ClusterInstance clusterInstance) throws
ExecutionException, InterruptedException {
try (Admin admin = clusterInstance.admin()) {
var versions = ((InternalDescribeFeaturesResult)
admin.describeFeatures()).nodeApiVersions().get();
versions.allSupportedApiVersions().forEach((key, version) ->
assertTrue(key.inScope(ApiMessageType.ListenerType.BROKER)));
}
try (Admin admin = clusterInstance.admin(Map.of(), true)) {
var versions = ((InternalDescribeFeaturesResult)
admin.describeFeatures()).nodeApiVersions().get();
versions.allSupportedApiVersions().forEach((key, version) ->
assertTrue(key.inScope(ApiMessageType.ListenerType.CONTROLLER)));
}
}
```
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -4585,8 +4592,11 @@ void handleResponse(AbstractResponse response) {
final ApiVersionsResponse apiVersionsResponse =
(ApiVersionsResponse) response;
if (apiVersionsResponse.data().errorCode() ==
Errors.NONE.code()) {
future.complete(createFeatureMetadata(apiVersionsResponse));
+
nodeApiVersionsFuture.complete(createNodeApiVersion(apiVersionsResponse));
Review Comment:
Could we use a `try-catch` block to handle potential errors from
`createNodeApiVersion`? It seems unnecessary to expose this specific error to
end-users, as they likely don't care about the versions
##########
clients/src/main/java/org/apache/kafka/clients/admin/DescribeFeaturesResult.java:
##########
@@ -30,7 +30,7 @@ public class DescribeFeaturesResult {
private final KafkaFuture<FeatureMetadata> future;
- DescribeFeaturesResult(KafkaFuture<FeatureMetadata> future) {
+ protected DescribeFeaturesResult(KafkaFuture<FeatureMetadata> future) {
Review Comment:
Would you please add a comment to remind users that inheritance
compatibility is not guaranteed here?
--
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]
