I don't agree with this change. The idea that an interface cannot have a default implementation is outdated in my view. Can someone provide any benefit to introducing a separate class for the factory method?
Ismael On Mon, Jun 17, 2019 at 10:07 AM Andy Coates <a...@confluent.io> wrote: > Hi All, > > I've updated the KIP to move the `create` factory method implementation > into a new `AdminClients` utility class, rather than on the new `Admin` > interface. > > Satish, > > As above, the KIP has been updated to only have the operations on the > `Admin` api. As for the overhead of dynamic proxies... the use of dynamic > proxies is totally up to the users of the library. In KSQL we use dynamic > proxies because the overhead is acceptable and it decouples us from > additions to the client interfaces. Others may decide otherwise for their > project. By making the admin api an interface we're empowering users to > choose the right approach for them. > > This is the primary motivation for the KIP from my point of view. However, > it also brings it inline with the other Kafka clients, and gives users the > freedom to do what they want, rather than requiring the use of an abstract > base class. > > 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 > > > > >>> > > > > >> > > > > > > > > > > > > > > > > > > >
