Hi Colin, Thanks for taking a look.
I don't think KIP-692, or the discussion of it, really considered the need for the evolution of how result objects are constructed. This KIP explicitly suggests a pattern for dealing with incompatible changes: use static factory methods, so that conflicting erased signatures can be avoided. I wouldn't be opposed to using static factory methods uniformly, deprecating the public access of constructors that are already public. It's not really clear to me that a builder pattern really offers better compatibility guarantees than such static factories. They seem more-or-less equivalent to me (could you illustrate why you think a builder pattern would be better?). But I guess I wouldn't be opposed to using a builder pattern uniformly either. Kind regards, Tom On Wed, Sep 29, 2021 at 6:12 PM Colin McCabe <cmcc...@apache.org> wrote: > Hi Tom, > > Maybe I'm missing something, but I don't see the difference between this > and KIP-692. If the constructors are public APIs, then we must assume that > anyone who needs more values will need to add more constructors. This KIP > spells that out, whereas KIP-692 did not, but I think it was clear in both > KIPs. Maybe there's something I'm overlooking that makes these KIPs > different? > > Looking back at this issue, I wish we had made these Result types into > interfaces rather than concrete classes. There is a fair amount of logic > in some of them. > > I do not really like the idea of maintaining the Result constructors as > public APIs forever. I think if we want to go that direction we should > provide public Builder objects. > > best, > Colin > > > On Wed, Sep 29, 2021, at 02:25, Tom Bentley wrote: > > Hi, > > > > Following Ismael's question in the discussion of KIP-774 (Deprecate > public > > access to Admin client's *Result constructors), I'd like to propose the > > alternative resolution of making all the constructors public. > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-777%3A+Improved+testability+for+Admin+client > > > > This is much closer to what was originally proposed in KIP-692, though > with > > more reasoning about how we can change the constructors in future while > > maintaining compatibility. It also proposes to add > > KafkaFuture.failedFuture() to avoid people needing to use KafkaFutureImpl > > in tests. > > > > Thanks, > > > > Tom > >
