Hi Ryanne, If we don't change the client code, then everywhere will still expect subclasses of `AdminClient`, so the interface will be of no use, i.e. I can't write a class that implements the new interface and pass it to the client code.
Thanks, Andy On Mon, 17 Jun 2019 at 19:01, Ryanne Dolan <ryannedo...@gmail.com> wrote: > Andy, while I agree that the new interface is useful, I'm not convinced > adding an interface requires deprecating AdminClient and changing so much > client code. Why not just add the Admin interface, have AdminClient > implement it, and have done? > > Ryanne > > On Mon, Jun 17, 2019 at 12:09 PM Andy Coates <a...@confluent.io> wrote: > > > Hi all, > > > > I think I've addressed all concerns. Let me know if I've not. Can I call > > another round of votes please? > > > > Thanks, > > > > Andy > > > > On Fri, 14 Jun 2019 at 04:55, Satish Duggana <satish.dugg...@gmail.com> > > wrote: > > > > > Hi Andy, > > > Thanks for the KIP. This is a good change and it gives the user a > better > > > handle on Admin client usage. I agree with the proposal except the new > > > `Admin` interface having all the methods from `AdminClient` abstract > > class. > > > It should be kept clean having only the admin operations as methods > from > > > KafkaClient abstract class but not the factory methods as mentioned in > > the > > > earlier mail. > > > > > > I know about dynamic proxies(which were widely used in RMI/EJB world). > I > > am > > > curious about the usecase using dynamic proxies with Admin client > > > interface. Dynamic proxy can have performance penalty if it is used in > > > critical path. Is that the primary motivation for creating the KIP? > > > > > > Thanks, > > > Satish. > > > > > > On Wed, Jun 12, 2019 at 8:43 PM Andy Coates <a...@confluent.io> wrote: > > > > > > > I'm not married to that part. That was only done to keep it more or > > less > > > > inline with what's already there, (an abstract class that has a > factory > > > > method that returns a subclass.... sounds like the same anti-pattern > > ;)) > > > > > > > > An alternative would to have an `AdminClients` utility class to > create > > > the > > > > admin client. > > > > > > > > On Mon, 10 Jun 2019 at 19:31, Matthias J. Sax <matth...@confluent.io > > > > > > wrote: > > > > > > > > > Hmmm... > > > > > > > > > > So the new interface, returns an instance of a class that > implements > > > the > > > > > interface. This sounds a little bit like an anti-pattern? Shouldn't > > > > > interfaces actually not know anything about classes that implement > > the > > > > > interface? > > > > > > > > > > > > > > > -Matthias > > > > > > > > > > On 6/10/19 11:22 AM, Andy Coates wrote: > > > > > > `AdminClient` would be deprecated purely because it would no > longer > > > > serve > > > > > > any purpose and would be virtually empty, getting all of its > > > > > implementation > > > > > > from the new interfar. It would be nice to remove this from the > API > > > at > > > > > the > > > > > > next major version bump, hence the need to deprecate. > > > > > > > > > > > > `AdminClient.create()` would return what it does today, (so not a > > > > > breaking > > > > > > change). > > > > > > > > > > > > On Tue, 4 Jun 2019 at 22:24, Ryanne Dolan <ryannedo...@gmail.com > > > > > > wrote: > > > > > > > > > > > >>> The existing `AdminClient` will be marked as deprecated. > > > > > >> > > > > > >> What's the reasoning behind this? I'm fine with the other > changes, > > > but > > > > > >> would prefer to keep the existing public API intact if it's not > > > > hurting > > > > > >> anything. > > > > > >> > > > > > >> Also, what will AdminClient.create() return? Would it be a > > breaking > > > > > change? > > > > > >> > > > > > >> Ryanne > > > > > >> > > > > > >> On Tue, Jun 4, 2019, 11:17 AM Andy Coates <a...@confluent.io> > > > 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 > > > > > >>> > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > >
