Thanks Tom, looks reasonable to me. I'll take a closer look soon, but please go ahead with the vote whenever you're ready.
Ismael On Thu, Mar 25, 2021, 8:53 AM Tom Bentley <tbent...@redhat.com> wrote: > Hi, > > I've updated the KIP along the lines of my previous reply to Ismael. If no > one has further comments I will likely start a vote thread next week. > > Kind regards, > > Tom > > On Tue, Mar 23, 2021 at 10:01 AM Tom Bentley <tbent...@redhat.com> wrote: > > > Hi Ryanne, > > > > Thanks for the reply. If there was a consensus for either of the more > > ambitious changes described in my 2nd email then I would agree with you, > > since it's much easier for users to understand and deal with and avoids > > having a situation of a class with half of its API deprecated, the need > to > > find synonyms etc. > > > > TBH though, that 2nd email was an attempt to get people thinking and > > engaging in the discussion. My personal preference is much closer to the > > incremental option described in the KIP, because overall I don't think > > there are nearly enough API mistakes/tech debt in the admin client > overall > > to warrant such a big change. Sure, there are a few deprecated methods > and > > classes, and using CompletionStage or CompletableFuture rather than > > KafkaFuture would require major changes where those could be cleaned up > at > > the same time, but it just doesn't seem worth it when the vast majority > of > > the API is fine. It would force everyone using the Admin client to > > eventually switch, whereas adding an accessor to KafkaFuture achieves the > > prime objective of enabling people who need to to interoperate with 3rd > > party APIs requiring CompletionStage, while requiring nothing of all the > > other users of the API. > > > > So unless there's a chorus of people who disagree and think that such a > > big refactoring really is worthwhile, then I'm inclined to stick with > > something closer to KIP-707. > > > > Many thanks, > > > > Tom > > > > On Fri, Mar 19, 2021 at 4:52 PM Ryanne Dolan <ryannedo...@gmail.com> > > wrote: > > > >> My two cents: keep the same method and class names and just use a > >> different > >> package. Strongly dislike coming up with slightly different names for > >> everything. > >> > >> Ryanne > >> > >> On Tue, Feb 2, 2021, 4:41 AM Tom Bentley <tbent...@redhat.com> wrote: > >> > >> > I've previously discounted the possibility of an "Admin2" client, but > >> > seeing the recent discussions on the thread for KIP-706, I wonder > >> whether > >> > this current proposal in KIP-707 would benefit from a bit more > >> > discussion... I think there are broadly two approaches to evolving the > >> > Admin client API to use CompletionStage directly (rather than what's > >> > currently proposed in KIP-707): > >> > > >> > The simpler option, from a development point of view, would be to > >> introduce > >> > an alternative/parallel set of classes for each of the existing result > >> > classes. E.g. ListTopicsOutcome which was the same as > ListTopicsResult, > >> but > >> > using CompletionStage rather than KafkaFuture. Adding methods to the > >> > existing Admin interface would require coming up with synonym method > >> names > >> > for every API call, and probably half of the API being deprecated (if > >> not > >> > immediately then in the long run). It would be cleaner to have a whole > >> new > >> > interface, let's call it Manager, using the same method names. The > >> existing > >> > Admin client implementation would then wrap a Manager instance, and > the > >> > existing Result classes could have a constructor parameter of their > >> > corresponding Outcome instance which wrapped the CompletionStages with > >> > KafkaFutures. The Options classes would be unchanged. From a users > >> point of > >> > view migrating to the new Manager client would mostly be a matter of > >> > changing class names and adding a `.toCompletionStage()` to those > places > >> > where they were calling KafkaFuture.get()/getNow() (and even this > could > >> be > >> > avoided if we used CompletableFuture rather than CompletionStage in > the > >> > Outcome class APIs). In the long run Admin would be removed and we'd > be > >> > left with the minor annoyance of having a client called Manager in a > >> > package called admin. > >> > > >> > The more involved version would do a similar refactoring, but within a > >> > different package. If we stuck with the Admin and Result class names > the > >> > users experience of migrating their codebase would be limited to > >> changing > >> > import statements and the same additions of `.toCompletionStage()`. On > >> the > >> > implementation side it would force us to duplicate all the Options > >> classes > >> > and also have a way of converting old Options instances to their new > >> > equivalents so that the old Admin implementation could delegate to the > >> new > >> > one. The main benefit of this approach seems to be the slightly easier > >> > experience for people porting their code to the new client. > >> > > >> > In doing either of these much more significant refactorings there > would > >> > also be the opportunity to omit the current Admin API's deprecated > >> methods > >> > and classes from the new API. > >> > > >> > Do we think this is worth biting off in order to have more long term > >> > consistency between the Admin, Producer and consumer APIs? > >> > > >> > Kind regards, > >> > > >> > Tom > >> > > >> > On Fri, Jan 22, 2021 at 3:02 PM Tom Bentley <tbent...@redhat.com> > >> wrote: > >> > > >> > > Hi, > >> > > > >> > > Following a recent discussion on a PR[1], I've written KIP-707 to > >> > > establish what should be done to improve the API of KafkaFuture. > >> > > If you have the time, your comments would be most welcome, as some > of > >> the > >> > > rejected alternatives are not unreasonable. > >> > > > >> > > > >> > > > >> > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-707%3A+The+future+of+KafkaFuture > >> > > > >> > > Many thanks, > >> > > > >> > > Tom > >> > > > >> > > [1]: https://github.com/apache/kafka/pull/9878 > >> > > > >> > > >> > > >
