@mj...@apache.org you can see an example in `Rejected Alternatives` section in KIP (ConsumerGroupMetadata newInstance(KafkaConsumer<?,?> kafkaConsumer method).
I agree with you that making the class an inner class of `KafkaConsumer` is quite big change and making ConsumerGroupMetadata an interface is good enough. pt., 28 mar 2025 o 01:18 Matthias J. Sax <mj...@apache.org> napisał(a): > > and providing a static create method that takes a kafka consumer > > object as a parameter or we can make the ConsumerGroupMetada inner not > > static class in KafkaConsumer > > Can you elaborate on the "static create method" -- frankly, it sounds a > little bit clumsy to me? > > I guess we could make the class an inner class of `KafkaConsumer`. > However, this case the "splash radius" of the change is much larger: > > - we need to deprecate the existing class entirely > - we also need to deprecate the `Consumer#groupMetadata()` and add a > new "overload" which would return the new (nested) version of > `ConsumerGroupMetada` (not sure how name this new "overload"...) > - we need to deprecate `Producer#sendOffsetsToTransaction()` and add a > new overload which accepts the new (nested) version of > `ConsumerGroupMetada` > > I think, this splash radius is too large, and not worth the trouble. > > > > Thus, while I agree that from a green field POV in interface is not > ideal (and using a private constructor and `final` class would be > better), we need to work with what we have, and overall to me it still > seems to be the best way forward is using an interface? -- Maybe the > "static create method" can be a viable alternative, but would need to > understand it better before I can form an opinion about it. > > > -Matthias > > > On 3/25/25 11:44 PM, Paweł Szymczyk wrote: > > Hi Kirk > > > > PR with making constructors deprecated since 4.1.0 is ready to merge -> > > https://github.com/apache/kafka/pull/18977. > > > > wt., 25 mar 2025 o 01:24 Kirk True <k...@kirktrue.pro> napisał(a): > > > >> Hi Paweł, > >> > >> We can't change the existing (public) constructors to private in 4.1 as > >> that would break users. Yes, we can argue that users shouldn't be > >> instantiating the ConsumerGroupMetadata in the first place, but as of > 4.0 > >> those constructors are part of the published API. > >> > >> For 4.1, at most we could mark them as deprecated. > >> > >> Thanks, > >> Kirk > >> > >> On Mon, Mar 24, 2025, at 9:24 AM, Paweł Szymczyk wrote: > >>> mj...@apache.org any update on that? Can we release it in 4.1.0? > >>> > >>> pt., 7 mar 2025 o 09:08 Paweł Szymczyk <pawel.szymczy...@gmail.com> > >>> napisał(a): > >>> > >>>> mj...@apache.org the main point is that this class is a regular data > >>>> structure, there is no way to give an alternative implementation which > >> is > >>>> the purpose of interface in Java. We can completely eliminate the > >>>> possibility of creating an instance of this class in any other way > >> than via > >>>> Kafka Consumer by changing the access modifiers of the constructors to > >>>> private and providing a static create method that takes a kafka > >> consumer > >>>> object as a parameter or we can make the ConsumerGroupMetada inner not > >>>> static class in KafkaConsumer, for those two alternative scenarios we > >> have > >>>> to provide different inner class for ConsumerCoordinator and > >>>> AsyncKafkaConsumer and then translate inner into public in Kafka > >> Consumer > >>>> class, like that: > >>>> > >>>> public ConsumerGroupMetadata groupMetadata() { > >>>> return new > >> ConsumerGroupMetadata(delegate.groupMetadata().groupId, > >>>> delegate.groupMetadata().generationId(), > >> delegate.groupMetadata().memberId, > >>>> groupMetadata().groupInstanceId); > >>>> } > >>>> > >>>> śr., 5 mar 2025 o 21:01 Matthias J. Sax <mj...@apache.org> > napisał(a): > >>>> > >>>>> Reversing the question. What is the problem with using an interface? > >> It > >>>>> seem, and interface would provide the cleanest way to separate public > >>>>> and internal things? Or maybe I misunderstand what you propose? > >>>>> > >>>>> We can for sure not have a `private` constructor. > >>>>> > >>>>> We can also not have "package private" constructor, because the class > >> is > >>>>> public, but we instantiate it from multiple different (internal) > >> packages. > >>>>> > >>>>> `protected` could maybe work, but `protected` it technically still > >>>>> public API, so if we need to change the constructor, we would need to > >> do > >>>>> a KIP. > >>>>> > >>>>> > >>>>> > >>>>> -Matthias > >>>>> > >>>>> On 3/5/25 9:22 AM, Ismael Juma wrote: > >>>>>> Hi, > >>>>>> > >>>>>> Thanks for the KIP. Why did we reject the alternative where the > >>>>> constructor > >>>>>> is unavailable to external users? > >>>>>> > >>>>>> Ismael > >>>>>> > >>>>>> On Tue, Feb 25, 2025 at 8:27 AM Paweł Szymczyk < > >>>>> pawel.szymczy...@gmail.com> > >>>>>> wrote: > >>>>>> > >>>>>>> > >>>>>>> > >>>>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1136%3A+Make+ConsumerGroupMetadata+an+interface > >>>>>>> > >>>>>>> -- > >>>>>>> Pozdrawiam > >>>>>>> Paweł Szymczyk > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>> > >>>> -- > >>>> Pozdrawiam > >>>> Paweł Szymczyk > >>>> > >>> > >>> > >>> -- > >>> Pozdrawiam > >>> Paweł Szymczyk > >>> > >> > > > > > > -- Pozdrawiam Paweł Szymczyk
