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