Hi,
I think it should be sufficient to just state in the JavaDocs of the new
interface, that the interface is not intended to be implement by users,
and point to the Consumer method that returns it.
To me, the main thing is really, that currently user can call `new` what
makes it easy to do thing wrong. If we change it to an interface, users
cannot call `new`, and if the interface says "do not implement me", it
should hopefully trigger the right response.
In the end, we can only do so much to guide users, and there is some
remaining responsibility to use the software as intended that we cannot
lift of them.
I don't think we would need a runtime check, but again, it's more of an
impl detail, not something the KIP has to cover?
About naming: I agree that `DefaultConsumerGroupMetadata` is not a
default, but it's only an internal class that users should not really
see, so the name does not matter too much, and we could also rename it
at any point. Of course, could also just use `ConsumerGroupMetadataImpl`
instead to avoid the term "default".
Technically, the KIP does not even need to mention
`DefaultConsumerGroupMetadata` at all (or only very briefly). It's not
public API.
-Matthias
On 3/1/25 12:35 PM, Paweł Szymczyk wrote:
Hi Kirk
Many thanks for very good questions!
KT1. I don't think that internal implementation should totally prevent of
passing alternative implementation, but for sure we need to log warning
when producer receives different than default implementaion. I think that
checking internally is the class implementation exactly
DefaultConsumerGroupMetadata is against some basic principles like liskov
substition and if we like to design this mechanism that way, we shouldn't
expose an interface.
KT2. I am also not 100% satisfied with that name, I don't think that there
will be need for future different implementations as it is simple data
struct.
KT3. It will be place in producer/internals package and default java scope
for constructor and whole class.
@Matthias J. Sax <mj...@apache.org> having those KT1 and KT2 in mind,
should we move forward with an interface or maybe ConsumerGroupMetadata has
to be a class, but we have to limit possibility to create an instance
manualy by making the class final and changing constructor visibility to
private or package scope?
czw., 27 lut 2025 o 01:51 Kirk True <k...@kirktrue.pro> napisał(a):
Hi Paweł,
Thanks for the KIP!
My questions:
KT1. What will prevent developers from implementing their own
ConsumerGroupMetadata and passing that to sendOffsetsToTransaction()? I
assume the code will check the incoming object is of type
DefaultConsumerGroupMetadata?
KT2. To me, the use of the adjective "default" in
DefaultConsumerGroupMetadata implies that there could be other
implementations. Is the intention that there could be other implementations
in the future?
KT3. DefaultConsumerGroupMetadata should be defined in an "internals"
package of some sort, right? Will users ever reference the implementation
class name in their code? I'm assuming not.
Thanks!
Kirk
On Tue, Feb 25, 2025, at 8:24 AM, Paweł Szymczyk wrote:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1136%3A+Make+ConsumerGroupMetadata+an+interface
--
Pozdrawiam
Paweł Szymczyk
