tombentley commented on a change in pull request #11301:
URL: https://github.com/apache/kafka/pull/11301#discussion_r703274931
##########
File path:
clients/src/main/java/org/apache/kafka/clients/admin/internals/AdminApiFuture.java
##########
@@ -109,7 +110,7 @@ private void completeExceptionally(K key, Throwable t) {
}
private KafkaFutureImpl<V> futureOrThrow(K key) {
- KafkaFutureImpl<V> future = futures.get(key);
+ KafkaFutureImpl<V> future = (KafkaFutureImpl<V>) futures.get(key);
Review comment:
The typecast isn't actually necessary if we're willing to declare
`public Map<K, ? extends KafkaFuture<V>> all()` in `SimpleAdminApiFuture` and
also in a handful of the `*Result` classes which use the
`SimpleAdminApiFuture`. Those `*Result` classes have package access
constructors so it would be compatible to do that. And that's actually the
correct way to solve this. But I think there are other `Result` classes where
that approach, if used more consistently in the other `*Result`, would be
problematic. The precedent was set a long time ago to just use plain `Map<X,
Y>` rather than the more correct `Map<X, ? extends Y>` in the Result classes.
So I guess the typecast is the lesser evil, and I'm OK with a comment.
--
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
