Yup I agree, generally speaking the `toString()` output is not recommended
to be relied on programmatically in user's code, but we've observed
convenience-beats-any-other-reasons again and again in development
unfortunately. I think we should still not claiming it is part of the
public APIs that would not be changed anyhow in the future, but just
mentioning it in the wiki for people to be aware is fine.


Guozhang

On Sun, Jun 24, 2018 at 5:01 PM, Matthias J. Sax <matth...@confluent.io>
wrote:

> Thanks for the KIP!
>
> I am don't have any further comments.
>
> For Guozhang's comment: if we mention anything about `toString()`, we
> should make explicit that `toString()` output is still not public
> contract and users should not rely on the output.
>
> Furhtermore, for the actual uses output, I would replace "topic:" by
> "extractor class:" to make the difference obvious.
>
> I am just hoping that people actually to not rely on `toString()` what
> defeats the purpose to the `TopologyDescription` class that was
> introduced to avoid the dependency... (Just a side comment, not really
> related to this KIP proposal itself).
>
>
> If there are no further comments in the next days, feel free to start
> the VOTE and open a PR.
>
>
>
>
> -Matthias
>
> On 6/22/18 6:04 PM, Guozhang Wang wrote:
> > Thanks for writing the KIP!
> >
> > I'm +1 on the proposed changes over all. One minor suggestion: we should
> > also mention that the `Sink#toString` will also be updated, in a way that
> > if `topic()` returns null, use the other call, etc. This is because
> > although we do not explicitly state the following logic as public
> protocols:
> >
> > ```
> >
> > "Sink: " + name + " (topic: " + topic() + ")\n      <-- " +
> > nodeNames(predecessors);
> >
> >
> > ```
> >
> > There are already some users that rely on `topology.describe().toString(
> )`
> > to have runtime checks on the returned string values, so changing this
> > means that their app will break and hence need to make code changes.
> >
> > Guozhang
> >
> > On Wed, Jun 20, 2018 at 7:20 PM, Nishanth Pradeep <nishanth...@gmail.com
> >
> > wrote:
> >
> >> Hello Everyone,
> >>
> >> I have created a new KIP to discuss extending TopologyDescription. You
> can
> >> find it here:
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> 321%3A+Add+method+to+get+TopicNameExtractor+in+TopologyDescription
> >>
> >> Please provide any feedback that you might have.
> >>
> >> Best,
> >> Nishanth Pradeep
> >>
> >
> >
> >
>
>


-- 
-- Guozhang

Reply via email to