mjsax commented on code in PR #14681:
URL: https://github.com/apache/kafka/pull/14681#discussion_r1379562125
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -618,7 +618,7 @@ public ProducerConfig(Map<String, Object> props) {
super(CONFIG, props);
}
- ProducerConfig(Map<?, ?> props, boolean doLog) {
+ protected ProducerConfig(Map<?, ?> props, boolean doLog) {
Review Comment:
> protected is public API for classes where inheritance is allowed.
While technically correct, I don't think that `ProducerConfig` is a class
that is _intended_ to be extended (and we cannot make it final because we want
to extend it internally). -- As a matter of fact, `ProducerConfig` should not
even be instantiated by a user, but it's only public to give access to the
config definitions which are all `public static final String ...` -- all other
methods it has (`configName()` etc) are also just public for practical reason
and should actually be hidden from the user -- it's a very leaky abstraction
from an API POV -- we actually have a similar issue for `StreamsConfig`; even
worse for this case; for which I consider doing a KIP to clean it up:
https://github.com/apache/kafka/pull/14548)
> Pointing to a commit where you did something doesn't quite provide
evidence that it's following the process. ;)
Sure, but I wanted to point out that many people did consider this ok in the
past, so why do we think it was not ok...?
If you are not going to revert this change if we merge it, I would just move
forward with this PR as-is, not doing a KIP (as it's highly questionable)... If
you insist on a KIP, Sophie please just write one and skip the discussion so we
can VOTE it directly -- but it seems totally unnecessary work to me)
And if you know me, I am _always_ in favor of doing thing the right way if
there is the slightest reason for it -- but for this case, I cannot see any
reason for a KIP at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
