rondagostino commented on a change in pull request #9032: URL: https://github.com/apache/kafka/pull/9032#discussion_r469680702
########## File path: clients/src/main/java/org/apache/kafka/clients/admin/DescribeUserScramCredentialsResult.java ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.clients.admin; + +import org.apache.kafka.common.KafkaFuture; +import org.apache.kafka.common.annotation.InterfaceStability; + +import java.util.Map; +import java.util.Objects; + +/** + * The result of the {@link Admin#describeUserScramCredentials()} call. + * + * The API of this class is evolving, see {@link Admin} for details. + */ +@InterfaceStability.Evolving +public class DescribeUserScramCredentialsResult { + private final KafkaFuture<Map<String, UserScramCredentialsDescription>> future; Review comment: Now that I'm working on this, I discovered that there is no other API that can describe or list everything that works this way. Everything that can describe or list everything returns a single future. Every describe or list API that returns a map of keys to futures requires a non-empty list of keys to describe or list. For example: 1. `listTopics()` lists all topics and returns a single `Future`; the `describeTopics()` API returns a map of names to futures but requires a non-empty list of topics to describe. 2. `describeConfigs()` returns a map of resources to futures but requires a non-empty list of resources to describe. 3. `describeLogDirs()` returns a map of broker IDs to futures but requires a non-empty list of brokers to describe. 4. `describeReplicaLogDirs()` returns a map of replicas to futures but requires a non-empty list of replicas to describe. 5. `describeConsumerGroups()` returns a map of consumer groups to futures but requires a non-empty list of consumer groups to describe. 6. `listPartitionReassignments()` allows listing all or a subset of reassignments and returns a single future. 7. `listOffsets()` returns a map of topic-partitions to futures but requires a non-empty list of topic-partitions to describe. 8. `describeClientQuotas()` allows listing all or a subset of quotas and returns a single future. I think if we made this change here we would be off the beaten path. That's not necessarily bad, but what tipped me off to this was the fact that when we list everything we have to create a future for every user that gets returned, and we don't know that list of users when we make the request, so there's really no way to implement it. We could create two separate APIs: one for describing some explicit, non-empty list of users, which would return a map of users to futures, and another one that describes everything, which returns a single future. `listTopics()` vs `describeTopics()` works this way, for example, though the information returned in the two is very different: when listing you just get the names, and when describing you get a lot more. I don't see us distinguishing between listing vs. describing in terms of data -- we are going to send back the same two things (mechanism and iterations) regardless. So we would probably be talking about creating a `describeUserScramCredentials()` API and a `describeAllUserScramCredentials()` API with the first taking a list and returning a map of futures and the second not taking a list and returning a single future. But I'm thinking we should just keep it the way it is -- take a possibly-empty list and return a single future regardles of whether the list was empty or not. Thoughts? ---------------------------------------------------------------- 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
