You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Tom Bentley <tb...@redhat.com> on 2021/09/09 14:20:02 UTC

[DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Hi,

I've written a small KIP-774 that proposes to deprecate public access to
the Admin client's *Result constructors:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors

I'd be grateful for any comments you may have.

Kind regards,

Tom

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Tom Bentley <tb...@redhat.com>.
Hi,

I've noticed that ElectLeadersResult is also the only result class to be
declared final, which means that mocking frameworks like Mockito cannot
mock it. So I'd like to also propose to remove that modifier at the same
time as deprecating the public use of the constructors. I've updated the
KIP to mention this.

Since there haven't been any other comments I'll start a vote soon.

Kind regards,

Tom

On Thu, Sep 9, 2021 at 5:01 PM Josep Prat <jo...@aiven.io.invalid>
wrote:

> Hi Tom,
>
> Thanks for your elaborate explanation. I agree with you, it doesn't seem
> like the best option right now. Thanks for updating the KIP with this
> rejected alternative.
>
> Best,
> ———
> Josep Prat
>
> Aiven Deutschland GmbH
>
> Immanuelkirchstraße 26, 10405 Berlin
>
> Amtsgericht Charlottenburg, HRB 209739 B
>
> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
>
> m: +491715557497
>
> w: aiven.io
>
> e: josep.prat@aiven.io
>
> On Thu, Sep 9, 2021, 17:37 Tom Bentley <tb...@redhat.com> wrote:
>
> > Hi Josep,
> >
> > Thanks for the question! I did consider it briefly:
> >
> > * As you mention, it would also be quite a lot more code (interfaces +
> > impls) for relatively small protection.
> > * It wouldn't simplify life for people wanting to mock the Admin client.
> It
> > would break code at runtime for people who (for whatever reason) are
> > already using reflection to instantiate the package-private constructors.
> > * I don't  _think_ it would be binary compatible for clients that were
> just
> > invoking methods on Result instances (specifically, I _think_ the JVM
> > rejects bytecode that uses invokevirtual on an interface, the byte code
> > should be using invokeinterface instead). Maybe that's not a huge problem
> > because people should not be substituting a Kafka 4.x jar for a 3.x one
> at
> > runtime.
> >
> > So on balance I'm inclined to the simpler approach in the KIP.
> >
> > If we _did_ decide to switch to interface in 4.0, deprecating the
> > constructors would be a first step anyway, so it would still make sense
> to
> > deprecate now. The difference would be what happens for 4.0 (or 5.0...):
> > Either changing the access modifier or introducing interfaces. Therefore
> > it's something we could change our minds on before that major release, if
> > there were other motivations for using interfaces.
> >
> > I've added this to the rejected alternatives.
> >
> > Kind regards,
> >
> > Tom
> >
> > On Thu, Sep 9, 2021 at 3:54 PM Josep Prat <jo...@aiven.io.invalid>
> > wrote:
> >
> > > Hi Tom,
> > > Thanks for the KIP,
> > > I have one question. Have you considered converting those classes to
> > > interfaces? This would also solve the problem. I must admit that the
> > change
> > > is way bigger this way, but I would argue it offers a cleaner
> distinction
> > > and it offers more safety against mistakenly making methods public that
> > > shouldn't be.
> > >
> > >
> > > Thanks!
> > > ———
> > > Josep Prat
> > >
> > > Aiven Deutschland GmbH
> > >
> > > Immanuelkirchstraße 26, 10405 Berlin
> > >
> > > Amtsgericht Charlottenburg, HRB 209739 B
> > >
> > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> > >
> > > m: +491715557497
> > >
> > > w: aiven.io
> > >
> > > e: josep.prat@aiven.io
> > >
> > > On Thu, Sep 9, 2021, 16:25 Tom Bentley <tb...@redhat.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > I've written a small KIP-774 that proposes to deprecate public access
> > to
> > > > the Admin client's *Result constructors:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
> > > >
> > > > I'd be grateful for any comments you may have.
> > > >
> > > > Kind regards,
> > > >
> > > > Tom
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Josep Prat <jo...@aiven.io.INVALID>.
Hi Tom,

Thanks for your elaborate explanation. I agree with you, it doesn't seem
like the best option right now. Thanks for updating the KIP with this
rejected alternative.

Best,
———
Josep Prat

Aiven Deutschland GmbH

Immanuelkirchstraße 26, 10405 Berlin

Amtsgericht Charlottenburg, HRB 209739 B

Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen

m: +491715557497

w: aiven.io

e: josep.prat@aiven.io

On Thu, Sep 9, 2021, 17:37 Tom Bentley <tb...@redhat.com> wrote:

> Hi Josep,
>
> Thanks for the question! I did consider it briefly:
>
> * As you mention, it would also be quite a lot more code (interfaces +
> impls) for relatively small protection.
> * It wouldn't simplify life for people wanting to mock the Admin client. It
> would break code at runtime for people who (for whatever reason) are
> already using reflection to instantiate the package-private constructors.
> * I don't  _think_ it would be binary compatible for clients that were just
> invoking methods on Result instances (specifically, I _think_ the JVM
> rejects bytecode that uses invokevirtual on an interface, the byte code
> should be using invokeinterface instead). Maybe that's not a huge problem
> because people should not be substituting a Kafka 4.x jar for a 3.x one at
> runtime.
>
> So on balance I'm inclined to the simpler approach in the KIP.
>
> If we _did_ decide to switch to interface in 4.0, deprecating the
> constructors would be a first step anyway, so it would still make sense to
> deprecate now. The difference would be what happens for 4.0 (or 5.0...):
> Either changing the access modifier or introducing interfaces. Therefore
> it's something we could change our minds on before that major release, if
> there were other motivations for using interfaces.
>
> I've added this to the rejected alternatives.
>
> Kind regards,
>
> Tom
>
> On Thu, Sep 9, 2021 at 3:54 PM Josep Prat <jo...@aiven.io.invalid>
> wrote:
>
> > Hi Tom,
> > Thanks for the KIP,
> > I have one question. Have you considered converting those classes to
> > interfaces? This would also solve the problem. I must admit that the
> change
> > is way bigger this way, but I would argue it offers a cleaner distinction
> > and it offers more safety against mistakenly making methods public that
> > shouldn't be.
> >
> >
> > Thanks!
> > ———
> > Josep Prat
> >
> > Aiven Deutschland GmbH
> >
> > Immanuelkirchstraße 26, 10405 Berlin
> >
> > Amtsgericht Charlottenburg, HRB 209739 B
> >
> > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> >
> > m: +491715557497
> >
> > w: aiven.io
> >
> > e: josep.prat@aiven.io
> >
> > On Thu, Sep 9, 2021, 16:25 Tom Bentley <tb...@redhat.com> wrote:
> >
> > > Hi,
> > >
> > > I've written a small KIP-774 that proposes to deprecate public access
> to
> > > the Admin client's *Result constructors:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
> > >
> > > I'd be grateful for any comments you may have.
> > >
> > > Kind regards,
> > >
> > > Tom
> > >
> >
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Tom Bentley <tb...@redhat.com>.
Hi Josep,

Thanks for the question! I did consider it briefly:

* As you mention, it would also be quite a lot more code (interfaces +
impls) for relatively small protection.
* It wouldn't simplify life for people wanting to mock the Admin client. It
would break code at runtime for people who (for whatever reason) are
already using reflection to instantiate the package-private constructors.
* I don't  _think_ it would be binary compatible for clients that were just
invoking methods on Result instances (specifically, I _think_ the JVM
rejects bytecode that uses invokevirtual on an interface, the byte code
should be using invokeinterface instead). Maybe that's not a huge problem
because people should not be substituting a Kafka 4.x jar for a 3.x one at
runtime.

So on balance I'm inclined to the simpler approach in the KIP.

If we _did_ decide to switch to interface in 4.0, deprecating the
constructors would be a first step anyway, so it would still make sense to
deprecate now. The difference would be what happens for 4.0 (or 5.0...):
Either changing the access modifier or introducing interfaces. Therefore
it's something we could change our minds on before that major release, if
there were other motivations for using interfaces.

I've added this to the rejected alternatives.

Kind regards,

Tom

On Thu, Sep 9, 2021 at 3:54 PM Josep Prat <jo...@aiven.io.invalid>
wrote:

> Hi Tom,
> Thanks for the KIP,
> I have one question. Have you considered converting those classes to
> interfaces? This would also solve the problem. I must admit that the change
> is way bigger this way, but I would argue it offers a cleaner distinction
> and it offers more safety against mistakenly making methods public that
> shouldn't be.
>
>
> Thanks!
> ———
> Josep Prat
>
> Aiven Deutschland GmbH
>
> Immanuelkirchstraße 26, 10405 Berlin
>
> Amtsgericht Charlottenburg, HRB 209739 B
>
> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
>
> m: +491715557497
>
> w: aiven.io
>
> e: josep.prat@aiven.io
>
> On Thu, Sep 9, 2021, 16:25 Tom Bentley <tb...@redhat.com> wrote:
>
> > Hi,
> >
> > I've written a small KIP-774 that proposes to deprecate public access to
> > the Admin client's *Result constructors:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
> >
> > I'd be grateful for any comments you may have.
> >
> > Kind regards,
> >
> > Tom
> >
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Josep Prat <jo...@aiven.io.INVALID>.
Hi Tom,
Thanks for the KIP,
I have one question. Have you considered converting those classes to
interfaces? This would also solve the problem. I must admit that the change
is way bigger this way, but I would argue it offers a cleaner distinction
and it offers more safety against mistakenly making methods public that
shouldn't be.


Thanks!
———
Josep Prat

Aiven Deutschland GmbH

Immanuelkirchstraße 26, 10405 Berlin

Amtsgericht Charlottenburg, HRB 209739 B

Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen

m: +491715557497

w: aiven.io

e: josep.prat@aiven.io

On Thu, Sep 9, 2021, 16:25 Tom Bentley <tb...@redhat.com> wrote:

> Hi,
>
> I've written a small KIP-774 that proposes to deprecate public access to
> the Admin client's *Result constructors:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
>
> I'd be grateful for any comments you may have.
>
> Kind regards,
>
> Tom
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Tom Bentley <tb...@redhat.com>.
>
> I suppose that could be avoided by the use of private constructors and
> public static factory methods (different method names means there's no
> overloading, so no ambiguity in when signatures need to differ only by
> generic type argument). But we'd have to choose names for those static
> methods -- that's easy when you know that DeleteTopicsResult supports
> by-name and by-id variants, but we didn't know that originally, so it's
> likely those methods would end up with poor names. So it's not much of an
> improvement over public constructors, in my opinion.
>

Replying to myself... it's not so bad, because we'd use public constructors
initially and only provide factory methods when we needed to evolve the API
in a way such that overloaded constructors wouldn't work (e.g. due to
erasure ambiguity). Thus, the need to choose names for factory methods only
arises once we know how the API is evolving. For example we'd initially
have a single public constructor, such as
```
public CreateTopicsResult(Map<String, KafkaFuture<Void>> futures) { ... }
```
and then, when we wanted to change the parameter type argument, we'd switch
to factory methods:
```
@Deprecated // use withDescriptions
public CreateTopicsResult(Map<String, KafkaFuture<Void>> futures) { ... }
// Hide the constructor that has the disambiguated signature behind a
static factory method
private CreateTopicsResult(boolean disambiguator, Map<String,
KafkaFuture<TopicMetadataAndConfig>> futures) { ... }
public static CreateTopicsResult withDescriptions(Map<String,
KafkaFuture<TopicDescription>> futures) { ... }
```
Obviously the hidden constructor can have whatever signature we want (that
doesn't collide with the existing public one), so we could use multiple
parameters (as in the DeleteTopicsResult change) if there wasn't a way to
convert one parameter to another.

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Tom Bentley <tb...@redhat.com>.
Hi Ismael,

These are all fair questions. I did a quick review of how these
constructors have changed. There are a few cases where extra constructor
parameters were added, so overloading the constructor would've been a
trivial way to be backwards compatible. The more interesting cases are
those which change generic type arguments:

CreateTopicsResult changed in 5d0052fe00c9071095b872b3b6ab6b1b455c9e20
-    CreateTopicsResult(Map<String, KafkaFuture<Void>> futures) {
+    CreateTopicsResult(Map<String, KafkaFuture<TopicMetadataAndConfig>>
futures) {
We would have needed to have an overloaded constructor with an unused
parameter to disambiguate the signature, and then live with that in the API
even if the original construct were deprecated and removed. Probably the
implementation would have required two, rather than one field for two maps
in order that #values() still worked when the constructor was only given
the old-style futures.

DescribeLogDirsResult changed in 819cd454f9b3078515a86e43a4486794df0a7971
-    DescribeLogDirsResult(Map<Integer, KafkaFuture<Map<String,
LogDirInfo>>> futures) {
+    DescribeLogDirsResult(Map<Integer, KafkaFuture<Map<String,
LogDirDescription>>> futures) {
         this.futures = futures;
     }
Not a source compatible change. We would have needed to have an overloaded
constructor with an unused parameter to disambiguate the signature, and
then live with that. This was part of work to fix the use of an internal
class leaking into the API, rather like the KafkaFutureImpl issue spotted
in time for Kafka 3.0. It had to deprecate methods that returned
LogDirInfo. I think the implementation could have used thenApply to convert
inner Map values from LogDirInfo to LogDirDescription before delegating to
the new constructor.

DescribeTopicsResult changed in 1d22b0d70686aef5689b775ea2ea7610a37f3e8c
+    @Deprecated
     protected DescribeTopicsResult(Map<String,
KafkaFuture<TopicDescription>> futures) {
-        this.futures = futures;
+        this(null, futures);
+    }
+
+    // VisibleForTesting
+    protected DescribeTopicsResult(Map<Uuid,
KafkaFuture<TopicDescription>> topicIdFutures, Map<String,
KafkaFuture<TopicDescription>> nameFutures) {
+        if (topicIdFutures != null && nameFutures != null)
+            throw new IllegalArgumentException("topicIdFutures and
nameFutures cannot both be specified.");
+        if (topicIdFutures == null && nameFutures == null)
+            throw new IllegalArgumentException("topicIdFutures and
nameFutures cannot both be null.");
+        this.topicIdFutures = topicIdFutures;
+        this.nameFutures = nameFutures;
+    }
Part of the topic ids work, clearly care has been taken to preserve
backwards compatibility and this was backwards compatible.

DeleteTopicsResult changed again in
cee2e975d124da0a2bdc0065a3172ce31e036fa0
-    protected DeleteTopicsResult(Map<String, KafkaFuture<Void>> futures) {
+    protected DeleteTopicsResult(Map<Uuid, KafkaFuture<Void>>
topicIdFutures, Map<String, KafkaFuture<Void>> nameFutures) {
Again part of the topic ids work, but this would've been an incompatible
change, though I'm guessing overloading could have solved it.

In summary, I don't think any of these would've been a show-stopper, but
two of them would have necessitated unused constructor parameters to
disambiguate overloaded constructors.

I suppose that could be avoided by the use of private constructors and
public static factory methods (different method names means there's no
overloading, so no ambiguity in when signatures need to differ only by
generic type argument). But we'd have to choose names for those static
methods -- that's easy when you know that DeleteTopicsResult supports
by-name and by-id variants, but we didn't know that originally, so it's
likely those methods would end up with poor names. So it's not much of an
improvement over public constructors, in my opinion.

While we're thinking about the ease of testing our APIs, the absence of a
public API for creating a failed KafkaFuture is an annoyance. Currently to
test an exceptional path people have to delve into KafkaFutureImpl.

Thoughts?

Tom

On Tue, Sep 21, 2021 at 7:51 PM Ismael Juma <is...@juma.me.uk> wrote:

> Hi Tom,
>
> I think these are all fair points. I was actually part of the group that
> decided that constructors should not be public. Since then I've noticed
> that many users find this limiting. I think we should make testability a
> key criteria for our APIs. Requiring a mocking library for data classes
> like this is suboptimal.
>
> To understand the value of the package private constructors, I have some
> questions:
>
> 1. For the cases where we evolved the constructors, how much did we gain?
> 2. Would a deprecation + removal be possible?
> 3. Did we have to deprecate some methods anyway?
>
> Ismael
>
> On Tue, Sep 21, 2021 at 10:07 AM Tom Bentley <tb...@redhat.com> wrote:
>
> > Hi Ismael,
> >
> > I agree that that is a laudable aim, but I couldn't see a good way of
> > achieving that while simultaneously allowing us the ability to evolve the
> > constructor signatures without breaking (or at least having to reason
> about
> > the compatibility impact of) test code which instantiates them. Hence the
> > need to decide whether this is worth addressing, and if so, how.
> >
> > In the KIP I prioritised the ability to evolve the API because I think
> that
> > was the original intent behind package-private, and I think that's a
> valid
> > reason to hide them. If it weren't possible to mock them at all I would
> > weigh the trade-off differently.
> >
> > I wouldn't argue if there was a consensus for making the constructors
> > public. The overall point would be having consistency. Speaking
> personally,
> > I think the constructor issue in KAFKA-13276 snuck past me because I
> didn't
> > notice the public modifier having internalised "all those constructors
> are
> > package-private".
> >
> > Did you have any bright ideas for how to provide more ergonomic mocking
> > without exposing the constructors? Or do you just value the mocking above
> > the evolvability in this case?
> >
> > Kind regards,
> >
> > Tom
> >
> > On Tue, Sep 21, 2021 at 1:21 PM Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Hi Tom,
> > >
> > > You say:
> > >
> > > "While the creation of Admin mocks with package constructors is not
> > > _ergonomic_, it is _possible_. The example code in KIP-692 requires two
> > > line of codes for each result instance."
> > >
> > > Should we not be aiming to make it ergonomic?
> > >
> > > Ismael
> > >
> > > On Thu, Sep 9, 2021 at 7:25 AM Tom Bentley <tb...@redhat.com>
> wrote:
> > >
> > > > Hi,
> > > >
> > > > I've written a small KIP-774 that proposes to deprecate public access
> > to
> > > > the Admin client's *Result constructors:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
> > > >
> > > > I'd be grateful for any comments you may have.
> > > >
> > > > Kind regards,
> > > >
> > > > Tom
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Ismael Juma <is...@juma.me.uk>.
Hi Tom,

I think these are all fair points. I was actually part of the group that
decided that constructors should not be public. Since then I've noticed
that many users find this limiting. I think we should make testability a
key criteria for our APIs. Requiring a mocking library for data classes
like this is suboptimal.

To understand the value of the package private constructors, I have some
questions:

1. For the cases where we evolved the constructors, how much did we gain?
2. Would a deprecation + removal be possible?
3. Did we have to deprecate some methods anyway?

Ismael

On Tue, Sep 21, 2021 at 10:07 AM Tom Bentley <tb...@redhat.com> wrote:

> Hi Ismael,
>
> I agree that that is a laudable aim, but I couldn't see a good way of
> achieving that while simultaneously allowing us the ability to evolve the
> constructor signatures without breaking (or at least having to reason about
> the compatibility impact of) test code which instantiates them. Hence the
> need to decide whether this is worth addressing, and if so, how.
>
> In the KIP I prioritised the ability to evolve the API because I think that
> was the original intent behind package-private, and I think that's a valid
> reason to hide them. If it weren't possible to mock them at all I would
> weigh the trade-off differently.
>
> I wouldn't argue if there was a consensus for making the constructors
> public. The overall point would be having consistency. Speaking personally,
> I think the constructor issue in KAFKA-13276 snuck past me because I didn't
> notice the public modifier having internalised "all those constructors are
> package-private".
>
> Did you have any bright ideas for how to provide more ergonomic mocking
> without exposing the constructors? Or do you just value the mocking above
> the evolvability in this case?
>
> Kind regards,
>
> Tom
>
> On Tue, Sep 21, 2021 at 1:21 PM Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Tom,
> >
> > You say:
> >
> > "While the creation of Admin mocks with package constructors is not
> > _ergonomic_, it is _possible_. The example code in KIP-692 requires two
> > line of codes for each result instance."
> >
> > Should we not be aiming to make it ergonomic?
> >
> > Ismael
> >
> > On Thu, Sep 9, 2021 at 7:25 AM Tom Bentley <tb...@redhat.com> wrote:
> >
> > > Hi,
> > >
> > > I've written a small KIP-774 that proposes to deprecate public access
> to
> > > the Admin client's *Result constructors:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
> > >
> > > I'd be grateful for any comments you may have.
> > >
> > > Kind regards,
> > >
> > > Tom
> > >
> >
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Tom Bentley <tb...@redhat.com>.
Hi Ismael,

I agree that that is a laudable aim, but I couldn't see a good way of
achieving that while simultaneously allowing us the ability to evolve the
constructor signatures without breaking (or at least having to reason about
the compatibility impact of) test code which instantiates them. Hence the
need to decide whether this is worth addressing, and if so, how.

In the KIP I prioritised the ability to evolve the API because I think that
was the original intent behind package-private, and I think that's a valid
reason to hide them. If it weren't possible to mock them at all I would
weigh the trade-off differently.

I wouldn't argue if there was a consensus for making the constructors
public. The overall point would be having consistency. Speaking personally,
I think the constructor issue in KAFKA-13276 snuck past me because I didn't
notice the public modifier having internalised "all those constructors are
package-private".

Did you have any bright ideas for how to provide more ergonomic mocking
without exposing the constructors? Or do you just value the mocking above
the evolvability in this case?

Kind regards,

Tom

On Tue, Sep 21, 2021 at 1:21 PM Ismael Juma <is...@juma.me.uk> wrote:

> Hi Tom,
>
> You say:
>
> "While the creation of Admin mocks with package constructors is not
> _ergonomic_, it is _possible_. The example code in KIP-692 requires two
> line of codes for each result instance."
>
> Should we not be aiming to make it ergonomic?
>
> Ismael
>
> On Thu, Sep 9, 2021 at 7:25 AM Tom Bentley <tb...@redhat.com> wrote:
>
> > Hi,
> >
> > I've written a small KIP-774 that proposes to deprecate public access to
> > the Admin client's *Result constructors:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
> >
> > I'd be grateful for any comments you may have.
> >
> > Kind regards,
> >
> > Tom
> >
>

Re: [DISCUSS] KIP-774: Deprecate public access to Admin client's *Result constructors

Posted by Ismael Juma <is...@juma.me.uk>.
Hi Tom,

You say:

"While the creation of Admin mocks with package constructors is not
_ergonomic_, it is _possible_. The example code in KIP-692 requires two
line of codes for each result instance."

Should we not be aiming to make it ergonomic?

Ismael

On Thu, Sep 9, 2021 at 7:25 AM Tom Bentley <tb...@redhat.com> wrote:

> Hi,
>
> I've written a small KIP-774 that proposes to deprecate public access to
> the Admin client's *Result constructors:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors
>
> I'd be grateful for any comments you may have.
>
> Kind regards,
>
> Tom
>