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 > > > >>> > > > >> > > > > > > > > > > > > >
