+1 (binding).
C.
On Mon, Jun 24, 2019, at 08:10, Andy Coates wrote:
> Hi all,
>
> KIP updated:
> - No deprecation
> - Factory method back onto Admin interface
>
> I'd like to kick off another round of voting please.
>
> Thanks,
>
> Andy
>
> On Mon, 24 Jun 2019 at 16:03, 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
> >> >>>>>>>>>>>>
> >> >>>>>>>>>>>
> >> >>>>>>>>>>
> >> >>>>>>>>>
> >> >>>>>>>>>
> >> >>>>>>>>
> >> >>>>>>>
> >> >>>>>>
> >> >>>>>
> >> >>>>
> >> >>
> >> >>
> >> >
> >>
> >>
>
