+1 (non-binding)
Thanks.
Ryanne
On Tue, Jun 25, 2019 at 10:21 PM Satish Duggana <satish.dugg...@gmail.com>
wrote:
> +1 (non-binding)
>
> On Wed, Jun 26, 2019 at 8:37 AM Satish Duggana <satish.dugg...@gmail.com>
> wrote:
> >
> > +1 Matthias/Andy.
> > IMHO, interface is about the contract, it should not have/expose any
> > implementation. I am fine with either way as it is more of taste or
> > preference.
> >
> > Agree with Ismael/Colin/Ryanne on not deprecating for good reasons.
> >
> >
> > On Mon, Jun 24, 2019 at 8:33 PM Andy Coates <a...@confluent.io> wrote:
> > >
> > > I agree Matthias.
> > >
> > > (In Scala, such factory methods are on a companion object. As Java
> doesn't
> > > have the concept of a companion object, an equivalent would be a
> utility
> > > class with a similar name...)
> > >
> > > However, I'll update the KIP to include the factory method on the
> interface.
> > >
> > > On Fri, 21 Jun 2019 at 23:40, Matthias J. Sax <matth...@confluent.io>
> wrote:
> > >
> > > > I still think, that an interface does not need to know anything about
> > > > its implementation. But I am also fine if we add a factory method to
> the
> > > > new interface if that is preferred by most people.
> > > >
> > > >
> > > > -Matthias
> > > >
> > > > On 6/21/19 7:10 AM, Ismael Juma wrote:
> > > > > This is even more reason not to deprecate immediately, there is
> very
> > > > little
> > > > > maintenance cost for us. We should be mindful that many of our
> users (eg
> > > > > Spark, Flink, etc.) typically allow users to specify the kafka
> clients
> > > > > version and hence avoid using new classes/interfaces for some
> time. They
> > > > > would get a bunch of warnings they cannot do anything about apart
> from
> > > > > suppressing.
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Fri, Jun 21, 2019 at 4:00 AM Andy Coates <a...@confluent.io>
> wrote:
> > > > >
> > > > >> Hi Ismael,
> > > > >>
> > > > >> I’m happy enough to not deprecate the existing `AdminClient`
> class as
> > > > part
> > > > >> of this change.
> > > > >>
> > > > >> However, note that, the class will likely be empty, i.e. all
> methods and
> > > > >> implementations will be inherited from the interface:
> > > > >>
> > > > >> public abstract class AdminClient implements Admin {
> > > > >> }
> > > > >>
> > > > >> Not marking it as deprecated has the benefit that users won’t see
> any
> > > > >> deprecation warnings on the next release. Conversely, deprecating
> it
> > > > will
> > > > >> mean we can choose to remove this, now pointless class, in the
> future
> > > > if we
> > > > >> choose.
> > > > >>
> > > > >> That’s my thinking for deprecation, but as I’ve said I’m happy
> either
> > > > way.
> > > > >>
> > > > >> Andy
> > > > >>
> > > > >>> On 18 Jun 2019, at 16:09, Ismael Juma <ism...@juma.me.uk> wrote:
> > > > >>>
> > > > >>> 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
> > > > >>>>>>>>>>>>
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>
> > > > >>>>>
> > > > >>>>
> > > > >>
> > > > >>
> > > > >
> > > >
> > > >
>
