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
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
>
signature.asc
Description: OpenPGP digital signature
