Thanks for the response Colin, > What specific benefits do we get from transitioning to using an interface rather than an abstract class?
This is covered in the KLIP: "An AdminClient interface has several advantages over an abstract base class, most notably allowing multi-inheritance and the use of dynamic proxies" > If we are serious about doing this, would it be cleaner to just change AdminClient from an abstract class to an interface in Kafka 3.0? It would break binary compatibility, but you have to break binary compatibility in any case to get what you want here (no abstract class). And it would have the advantage of not creating a lot of churn in the codebase as people replaced "AdminClient" with "Admin" all over the place. What do you think? How far off is Kafka 3.0? This is causing us pain right now on a regular basis and, from Ryanne's email above we're not alone. I'm not against making this a change in Kafka 3.0, but only if its imminent. > On a related note, one problem I've seen is that people will subclass AdminClient for testing. Then, every time Kafka adds a new API, we add a new abstract method to the base class, which breaks compilation for them. Their test classes would have been fine simply failing when the new API was called. So perhaps one useful class would be a class that implements the AdminClient API, and throws UnimplementedException for every method. The test classes could subclass this method and never have to worry about new methods getting added again. This is similar to what KSQL does for the other Kafka client APIs, except we use a dynamic proxy, rather than having to hand code the exception throwing. > Another pattern I've seen is people wanting to implement a class which is similar to KafkaAdminClient in every way, except for the behavior of close(). Specifically, people want to implement reference counting in order to reuse AdminClient instances. So they start by implementing essentially a delegating class, which just forwards every method to an underlying AdminClient instance. But this has the same problem that it breaks every time the upstream project adds an API. In order to support this, we could have an official DelegatingAdminClient base class that forwarded every method to an underlying AdminClient instance. Then the client code could override the methods they needed, like close. Again, this would be trivial to implement using dynamic proxies. No need for us to implement any `DelegatingAdminClient`. If this is an interface we empower users to do this for themselves. best, Colin On Mon, 10 Jun 2019 at 21:44, Colin McCabe <cmcc...@apache.org> wrote: > Hi Andy, > > This is a big change, and I don't think there has been a lot of discussion > about the pros and cons. What specific benefits do we get from > transitioning to using an interface rather than an abstract class? > > If we are serious about doing this, would it be cleaner to just change > AdminClient from an abstract class to an interface in Kafka 3.0? It would > break binary compatibility, but you have to break binary compatibility in > any case to get what you want here (no abstract class). And it would have > the advantage of not creating a lot of churn in the codebase as people > replaced "AdminClient" with "Admin" all over the place. What do you think? > > On a related note, one problem I've seen is that people will subclass > AdminClient for testing. Then, every time Kafka adds a new API, we add a > new abstract method to the base class, which breaks compilation for them. > Their test classes would have been fine simply failing when the new API was > called. So perhaps one useful class would be a class that implements the > AdminClient API, and throws UnimplementedException for every method. The > test classes could subclass this method and never have to worry about new > methods getting added again. > > Another pattern I've seen is people wanting to implement a class which is > similar to KafkaAdminClient in every way, except for the behavior of > close(). Specifically, people want to implement reference counting in > order to reuse AdminClient instances. So they start by implementing > essentially a delegating class, which just forwards every method to an > underlying AdminClient instance. But this has the same problem that it > breaks every time the upstream project adds an API. In order to support > this, we could have an official DelegatingAdminClient base class that > forwarded every method to an underlying AdminClient instance. Then the > client code could override the methods they needed, like close. > > best, > Colin > > > On Tue, Jun 4, 2019, at 09:17, Andy Coates wrote: > > Hi folks > > > > As there's been no chatter on this KIP I'm assuming it's non-contentious, > > (or just boring), hence I'd like to call a vote for KIP-476: > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-476%3A+Add+Java+AdminClient+Interface > > > > Thanks, > > > > Andy > > >
