AndrewJSchofield commented on code in PR #19743:
URL: https://github.com/apache/kafka/pull/19743#discussion_r2099600892
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1775,12 +1775,36 @@ default FenceProducersResult
fenceProducers(Collection<String> transactionalIds)
FenceProducersResult fenceProducers(Collection<String> transactionalIds,
FenceProducersOptions options);
+ /**
+ * List the configuration resources available in the cluster which matches
config resource type.
+ * If no config resource types are specified, all configuration resources
will be listed.
+ *
+ * @param configResourceTypes The set of configuration resource types to
list.
+ * @param options The options to use when listing the configuration
resources.
+ * @return The ListConfigurationResourcesResult.
+ */
+ ListConfigResourcesResult listConfigResources(Set<ConfigResource.Type>
configResourceTypes, ListConfigResourcesOptions options);
+
+ /**
+ * List all configuration resources available in the cluster with the
default options.
+ * <p>
+ * This is a convenience method for {@link #listConfigResources(Set,
ListConfigResourcesOptions)}
+ * with default options. See the overload for more details.
+ *
+ * @return The ListConfigurationResourcesResult.
+ */
+ default ListConfigResourcesResult listConfigResources() {
+ return listConfigResources(Set.of(), new ListConfigResourcesOptions());
+ }
+
/**
* List the client metrics configuration resources available in the
cluster.
*
* @param options The options to use when listing the client metrics
resources.
* @return The ListClientMetricsResourcesResult.
+ * @deprecated Since 4.1. Use {@link #listConfigResources(Set,
ListConfigResourcesOptions)} instead.
*/
+ @Deprecated(since = "4.1")
Review Comment:
Probably ought to add `, forRemoval = true`.
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1790,7 +1814,9 @@ FenceProducersResult fenceProducers(Collection<String>
transactionalIds,
* with default options. See the overload for more details.
*
* @return The ListClientMetricsResourcesResult.
+ * @deprecated Since 4.1. Use {@link #listConfigResources()} instead.
*/
+ @Deprecated(since = "4.1")
Review Comment:
`, forRemoval = true`.
##########
clients/src/main/java/org/apache/kafka/common/requests/ListConfigResourcesRequest.java:
##########
@@ -36,6 +38,15 @@ public Builder(ListConfigResourcesRequestData data) {
@Override
public ListConfigResourcesRequest build(short version) {
+ if (version == 0) {
+ // The v0 only supports CLIENT_METRICS resource type.
+ // Empty resource types means all supported resource types. In
v0, it means CLIENT_METRICS, so it's acceptable.
+ Set<Byte> resourceTypes = new HashSet<>(data.resourceTypes());
+ if ((resourceTypes.size() == 1 &&
!resourceTypes.contains(ConfigResource.Type.CLIENT_METRICS.id())) ||
Review Comment:
I think it's only valid if `resourceTypes` is non-empty (because empty means
all types) and if it only contains `CLIENT_METRICS.id()`. wdyt?
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3868,6 +3868,92 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
+ @Test
+ def testListConfigResources(): Unit = {
+ client = createAdminClient
+
+ // Alter group and client metric config to add group and client metric
config resource
+ val clientMetric = "client-metrics"
+ val group = "group"
+ val clientMetricResource = new
ConfigResource(ConfigResource.Type.CLIENT_METRICS, clientMetric)
+ val groupResource = new ConfigResource(ConfigResource.Type.GROUP, group)
+ val alterResult = client.incrementalAlterConfigs(util.Map.of(
Review Comment:
nit: Can't this just be `Map.of`?
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3868,6 +3868,92 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
+ @Test
+ def testListConfigResources(): Unit = {
+ client = createAdminClient
+
+ // Alter group and client metric config to add group and client metric
config resource
+ val clientMetric = "client-metrics"
+ val group = "group"
+ val clientMetricResource = new
ConfigResource(ConfigResource.Type.CLIENT_METRICS, clientMetric)
+ val groupResource = new ConfigResource(ConfigResource.Type.GROUP, group)
+ val alterResult = client.incrementalAlterConfigs(util.Map.of(
+ clientMetricResource,
+ util.Set.of(new AlterConfigOp(new ConfigEntry("interval.ms", "111"),
AlterConfigOp.OpType.SET)),
Review Comment:
And `Set.of`.
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -3868,6 +3868,92 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
} finally client.close(time.Duration.ZERO)
}
+ @Test
+ def testListConfigResources(): Unit = {
+ client = createAdminClient
+
+ // Alter group and client metric config to add group and client metric
config resource
+ val clientMetric = "client-metrics"
+ val group = "group"
+ val clientMetricResource = new
ConfigResource(ConfigResource.Type.CLIENT_METRICS, clientMetric)
+ val groupResource = new ConfigResource(ConfigResource.Type.GROUP, group)
+ val alterResult = client.incrementalAlterConfigs(util.Map.of(
+ clientMetricResource,
+ util.Set.of(new AlterConfigOp(new ConfigEntry("interval.ms", "111"),
AlterConfigOp.OpType.SET)),
Review Comment:
In fact, there's a lot of this in this test.
--
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
