I agree with Ryanne, I think we should avoid deprecating AdminClient and causing so much churn for users who don't actually care about this niche use case.
Ismael On Tue, Jun 18, 2019 at 6:43 AM Andy Coates <a...@confluent.io> wrote: > 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 > > > > > > >>> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
